lua: add downstreamRequestHeaders() accessor for response path#44594
Open
derekargueta wants to merge 21 commits into
Open
lua: add downstreamRequestHeaders() accessor for response path#44594derekargueta wants to merge 21 commits into
derekargueta wants to merge 21 commits into
Conversation
Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Member
Author
|
/retest |
derekargueta
commented
Apr 24, 2026
…t-headers-in-response
wbpcode
reviewed
Apr 27, 2026
Member
wbpcode
left a comment
wbpcode
left a comment
There was a problem hiding this comment.
LGTM. Thanks for this contribution. Could you merge main and resolve the conflict? Thanks.
Member
|
/wait |
…t-headers-in-response Signed-off-by: Derek Argueta <derekargueta@gmail.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
…to response path Rename the Lua filter's requestHeaders() accessor to downstreamRequestHeaders() to make the semantics explicit: it returns the *downstream* request headers and is only meaningful on the response path. A runtime guard enforces this: calling downstreamRequestHeaders() from envoy_on_request raises a Lua script error. The is_response_ flag on StreamHandleWrapper is threaded through Filter::doHeaders(), which passes false for the decode path and true for the encode path. On the C++ side, FilterCallbacks::requestHeaders() is renamed to downstreamRequestHeaders() to match. The docs are restructured to distinguish the request handle API from the response handle API (a superset), with downstreamRequestHeaders() documented exclusively in the response section. Signed-off-by: Derek Argueta <derekargueta@gmail.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
…assertions Adds RequestHeadersInResponseCached to verify the downstream_request_headers_wrapper_ is reused across multiple Lua calls within a single response handler, enforced via Times(1) on the requestHeaders() mock. Also updates DownstreamRequestHeadersNotAvailableInRequest and RespondInResponsePath to expect the Lua nil-method error produced by the two-type handle design rather than the old custom luaL_error strings. Signed-off-by: Derek Argueta <dereka@gmail.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
…t-headers-in-response Signed-off-by: Derek Argueta <dereka@gmail.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <dereka@gmail.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Member
|
seems the CI failure is real... /wait |
Check the cached wrapper before calling into the C++ callbacks layer so the second Lua call to downstreamRequestHeaders() returns the cached HeaderMapWrapper without re-invoking requestHeaders(). Previously the cache check came after the callbacks call, causing the mock to fire twice and h2 to receive a nil wrapper. Also rephrase the FORWARD_LUA_FUNCTION comment to drop "redeclaring" (failed spell check) and apply clang-format to the Cached test block. Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
…ponse Signed-off-by: Derek Argueta <derek.argueta@airbnb.com> # Conflicts: # changelogs/current.yaml
derekargueta
changed the title
- DecoderCallbacks::downstreamRequestHeaders() now returns nullopt instead of the live request headers; the method is unreachable from Lua on the request path (not in RequestStreamHandleWrapper's exportedFunctions) but returning real headers was a latent footgun. - Add sync comments to both onMarkDead() overrides flagging that they must be kept in step with each other and with StreamHandleWrapperBase fields. - Add two missing unit tests: RequestHeadersInResponseAfterYield: verifies that the cached wrapper is invalidated across a coroutine yield (body() suspend/resume) and that requestHeaders() is called again on the resumed coroutine. RequestHeadersInResponseWithBody: verifies that the accessor works correctly when encodeHeaders is called with end_stream=false. Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Clarify that nil is returned when the response is generated before downstream request headers have been fully received, e.g. request header timeout, stream idle timeout before headers arrive, or an immediate protocol-level rejection during header parsing. Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Response handle API must use the same underline character (-) as the other top-level API sections (Stream handle API, Header object API) rather than ~~~. downstreamRequestHeaders() moves to ~~~ accordingly. The previous ~~~/^^^^^^ nesting caused Sphinx to see an inconsistent title level when it encountered Header object API's - underline. Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Use ^^^^^ for the method entry, matching every other method heading in the document. The previous ~~~ reused the H3 level already assigned to Request/Response handle API sections, confusing Sphinx when it tried to render Header object API and its ^^^^^-underlined entries. Signed-off-by: Derek Argueta <dargueta@users.noreply.github.com> Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
Request handle API and Response handle API were using a tilde (~~~) heading level that didn't exist anywhere else in lua_filter.rst. Introducing that H3 level caused all subsequent ^^^^^ method entries (which Sphinx had been treating as H3) to shift to H4, breaking every H2 -> ^^^^^ transition in the document (Header object API, Buffer API, etc.) that had been working fine on main. Fix: use --- for Request handle API (matching all other major sections), and replace the Response handle API heading with a .. note:: block so the anchor is preserved but no new heading level is introduced. Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
The previous commit replaced the heading with a note block to avoid RST title level issues. Since the section heading is useful for navigation and the title level fix (removing ~~~) is sufficient on its own, restore the heading using --- consistent with all other major sections in the file. Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
The comment incorrectly claimed Times(2) was the load-bearing assertion. requestHeaders() is only called once since downstreamRequestHeaders() is invoked after the yield, not before it. Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
wbpcode
reviewed
May 14, 2026
Member
There was a problem hiding this comment.
Thanks for the contribution. Seems this bring huge difference rather than only a new method.
I have two questions:
- should that request headers be read only? I guess we still could let the envoy_on_response to update the request headers? The updating still makes sense because it could affect logging/tracing and so on?
- could we just call it
requestHeaders()(remove thedownstreamprefix) and also provide this method atenvoy_on_request. Then the PR will be much more clean?
/wait-any
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: lua: add
downstreamRequestHeaders()accessor for response pathAdditional Description: Add support for reading the original request headers from
envoy_on_responseLua path. This is useful for response-time logic that depends on any values from the request.Risk Level: low
Testing: included - unit & integration
Docs Changes: updated Lua docs to specify "request API" vs "response API"
Release Notes: added a changelog entry
Platform Specific Features: N/A
Fixes #4613