api7 / api7/ngx_http_ffi_client

Honor a caller-supplied Transfer-Encoding once request bodies can stream

Open
#29 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Split out of the #16 review. Part of the lua-resty-http parity effort (#18).

Today

Transfer-Encoding is a reserved request header on both sides of the FFI boundary (forbidden_header() in lib/resty/ngx_http_ffi_client.lua, ngx_http_ffi_client_header_is_reserved() in src/ngx_http_ffi_client_request.c). A caller that sets it has it dropped, silently, and the request goes out framed with Content-Length.

Dropping it is correct for now. This client always frames the request body with a Content-Length, so emitting a caller Transfer-Encoding next to it would put both on the wire, which is the request-smuggling shape. The silence is the part worth fixing first.

What lua-resty-http does

Transfer-Encoding is load-bearing there, and it is how a caller asks for a streamed request body:

  • transfer_encoding_is_chunked(headers) (resty/http.lua:793) decides the framing.
  • When chunked, Content-Length is dropped, citing RFC 7230 3.3.3 (:796-799) - the inverse of what this module does.
  • A body that is a function is then written chunk by chunk by _send_body. Without a chunked encoding or an explicit length, that same function body is rejected with Request body is a function but a length or chunked encoding is not specified (:806-807).

So the header alone is meaningless in isolation: it exists to select a request-body framing this module cannot produce yet.

Why it is blocked

There is no streamed request body here. send_request takes rp->body as one ngx_str_t and build_request emits it behind a computed Content-Length. A function or iterator body is the request-side reader deferred in #12, and its absence is already why t/010-stream-conformance.t skips lua-resty-http's TEST 6 / 7 / 8 / 10.

Acceptance

Two steps, in order. The first stands on its own and does not wait for #12.

  1. Reject instead of dropping. A caller-supplied Transfer-Encoding returns nil, "<reason>" rather than being discarded, so a request never quietly differs from what was asked for. Same treatment on the C side, which validates independently of Lua.
  2. Honor it once a streamed request body exists (needs #12). Transfer-Encoding: chunked selects chunked request framing, Content-Length is dropped rather than sent alongside it, and a function body without either is refused the way lua-resty-http refuses it. Any combination that would emit both Content-Length and Transfer-Encoding stays refused.

Test::Nginx cases for the rejection, and for chunked request framing once step 2 lands. Green on both parser backends.

Related

Host, Connection and Content-Length are reserved by the same two functions and break parity in their own ways. They are tracked separately since each needs a different answer; this issue covers Transfer-Encoding only.

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 forbidden_header() in lib/resty/ngx_http_ffi_client.lua and ngx_http_ffi_client_header_is_reserved() in src/ngx_http_ffi_client_request.c, then inspect t/010-stream-conformance.t. First, make caller-supplied Transfer-Encoding return an error on both parser backends. Once streamed request bodies from #12 exist, verify chunked framing, Content-Length removal, and rejection of unsafe combinations with Test::Nginx cases.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.