api7 / api7/ngx_http_ffi_client
body_reader must be bound to its response generation
Nobody has claimed this yet.
- 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 onclose/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/closeerrors; the happy path is unchanged. - Green on both parser backends.
Part of #18.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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