Skip to content

middleware/store: MemoryKV keeps the caller's key string as its map key, so a request header used as a key changes under the map on epoll and io_uring: a retried Idempotency-Key runs the handler again (1/1 per native engine) #719

Description

@FumingPower3925

Summary

On epoll and io_uring, a retried Idempotency-Key can run the handler a second time instead of replaying the stored response. With the default store it did so in 1 of 1 retry on each native engine, and never on std. The retry is made after the original connection has sent one more request.

The cause is that store.MemoryKV, the default store for idempotency, session and csrf, keeps the caller's key string as its map key. Callers pass request-derived strings, which on the native engines are views of the connection's receive buffer. This is the #485 class, which fixed middleware/static and middleware/ratelimit, at a retention site #485 did not cover. It was found by the audit that #714 called for.

Mechanism

  • idempotency.New takes key := c.Header(keyHeaderLower) (middleware/idempotency/idempotency.go:77 at 9f4d89b) and locks it with cfg.Store.SetNX(ctx, key, ...) (:133). The default store is store.NewMemoryKV (middleware/idempotency/store.go:15).
  • MemoryKV.SetNX, Set and Increment insert with s.items[key] = ... (middleware/store/memory.go:146, :239, :295). The key is not cloned. The update-in-place path (:132) keeps whichever string made the first insert.
  • After the handler returns, the next request on that connection is received into the same buffer. The map key's bytes then change while its hash does not. A lookup with the original key hashes to the entry but compares unequal, so Get reports ErrNotFound and the retry runs the handler again.
  • The same shape exists in middleware/cache/store.go:155 (s.items[key] = n). It is reachable only through a custom Config.KeyGenerator that returns a view (for example c.Path()), because the default key is a fresh concatenation (cache.go:306, :333). The session middleware normally reaches MemoryKV.Set for an existing key, which is the update path, and inserts a view only when the sweep deletes the entry between its Get and its Set.

Reproduction

The evidence test is TestC714EvidenceIdempotencyKeyView. It runs on a snapshot of 9f4d89b and is kept with the #714 evidence, not committed. The route is POST /pay behind idempotency.New(), and the handler counts its runs. Connection A sends key k1, then key k2, with the same request layout. Connection B then retries k1.

engine handler runs after k1, k2, retry k1 (want 2)
std 2
epoll 3
io_uring 3

Docker linux/arm64 (--cpus 4), kernel 7.0.12-linuxkit, memlock 8 MiB, go test -race.

Impact

  • Idempotency-Key replay protection fails for any key whose connection has sent another request since. The non-idempotent handler (a payment, an order) runs again, which is the outcome the middleware exists to prevent.
  • A MemoryKV map whose keys change after insertion also accumulates entries that no lookup can reach until they expire.

Fix direction

Clone the key on insert in MemoryKV (Set when the key is new, SetNX, Increment) and in the cache LRU store's insert, the way #485 cloned at the retention site. The hit and update paths do not keep the argument, so the steady-state cost is zero and there is one copy per new key.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingmiddlewareMiddleware implementation

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions