Kong / ngx_wasm_module

Nginx + WebAssembly
Apache License 2.0
79 stars 7 forks source link

fix(proxy-wasm) only inject shim headers after on_response_headers #565

Closed thibaultcha closed 2 months ago

thibaultcha commented 2 months ago

Due to some Nginx internals (some response headers are added after our module is executed but before the response is sent) we inject "shim headers" into the get_http_response_headers() API. Some heuristics are used to determine whether or not Transfer-Encoding, Connection, or other "last minute" headers will be injected or not.

The issue is that the heuristics used are only valid during response production and after (e.g. r->chunked or r->headers_out.content_length_n may only be relevant during and after response production). If a user uses get_http_response_headers() in request phases, the returned headers may include elements that will not actually be included in the response (e.g. Transfer-Encoding).

We now enforce that shim response headers should only be injected during and after on_http_response_headers.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.58251%. Comparing base (3072221) to head (fa7a66e). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565/graphs/tree.svg?width=650&height=150&src=pr&token=T0PT2Q9IAN&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong)](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) ```diff @@ Coverage Diff @@ ## main #565 +/- ## =================================================== + Coverage 90.57558% 90.58251% +0.00692% =================================================== Files 49 49 Lines 10876 10884 +8 =================================================== + Hits 9851 9859 +8 Misses 1025 1025 ``` | [Files](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | Coverage Δ | | |---|---|---| | [src/common/proxy\_wasm/ngx\_proxy\_wasm\_maps.c](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565?src=pr&el=tree&filepath=src%2Fcommon%2Fproxy_wasm%2Fngx_proxy_wasm_maps.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#diff-c3JjL2NvbW1vbi9wcm94eV93YXNtL25neF9wcm94eV93YXNtX21hcHMuYw==) | `93.47826% <100.00000%> (+0.14493%)` | :arrow_up: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | [Flag](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | `90.32495% <100.00000%> (+0.02706%)` | :arrow_up: | | [valgrind](https://app.codecov.io/gh/Kong/ngx_wasm_module/pull/565/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong) | `81.91737% <100.00000%> (-0.10851%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Kong#carryforward-flags-in-the-pull-request-comment) to find out more.