api7 / api7/ngx_http_ffi_client

body_reader must be bound to its response generation

Open
#26 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug P2
Dominant language
C
Stars
2
Forks
0
PR merge metrics
No merged PRs in 30d

Description

P2 follow-up from the review of #22 (https://github.com/api7/ngx_http_ffi_client/pull/22#issuecomment-5088042507).

res.body_reader is not bound to the response it came from, so a reader held past its response can consume a later response's body.

Detail

make_body_reader() (lib/resty/ngx_http_ffi_client.lua:632-654) closes over the client object and a local done flag, then calls read_chunk(self) — which reads from whatever the op's current state is. Nothing ties the reader to the response generation that produced it.

So on a reused connection:

local res1 = httpc:request{ path = "/a" }   -- body not read
-- ... res1.body_reader kept around ...
local res2 = httpc:request{ path = "/b" }
res1.body_reader()                          -- reads /b's body

The same hazard exists for a reader kept across set_keepalive() / close() + connect(), and for a preread (prefetch) response whose reader is called afterwards.

Proposal

  • Give the op a monotonically increasing response generation, bumped on each request (and on close / set_keepalive).
  • Capture the generation in the reader; on mismatch return a clear error (response already consumed / stale body reader) instead of reading bytes.
  • Enforce it on the C side too, so an FFI caller cannot bypass the Lua check.

Acceptance

  • Test::Nginx cases: a stale reader from response N returns an error after response N+1 starts; a reader used after set_keepalive/close errors; the happy path is unchanged.
  • Green on both parser backends.

Part of #18.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with make_body_reader() in lib/resty/ngx_http_ffi_client.lua:632-654 and trace read_chunk(self), then locate the corresponding C-side reader state. Add generation checks so stale readers return an error after a new request, set_keepalive, or close. Verify the Test::Nginx cases for stale and happy-path readers, and run them on both parser backends.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, lua
Domain
api, networking, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.