shreemaan-abhishek opened a new pull request, #13839:
URL: https://github.com/apache/apisix/pull/13839

   ### Description
   
   `data-mask` masks configured request headers from its log-phase handler by 
calling `core.request.set_header()`, which calls `ngx.req.set_header()`. That 
function returns early without touching the request header list whenever 
`r->headers_out.status` is already `400` 
([`ngx_http_lua_headers_in.c`](https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_headers_in.c#L766)):
   
   ```c
   if (r->headers_out.status == 400 || r->headers_in.headers.last == NULL) {
       /* must be a 400 Bad Request */
       return NGX_OK;
   }
   ```
   
   The check was added in 2016 to avoid a segfault when writing headers on a 
request nginx itself rejected as malformed. It reads `headers_out.status`, 
which is unset in the rewrite/access phases where `set_header()` is normally 
called, but in the log phase holds whatever status the upstream returned. A 
well-formed request answered with `400` is therefore indistinguishable from a 
rejected one, and the masking silently does nothing while returning success.
   
   The result: with `data-mask` and a logger plugin on the same route, the 
configured header was recorded unmasked whenever the response status was `400`. 
Every other status masked correctly.
   
   Both routes a logger takes to the request headers were affected:
   
   - the default log entry, built from `ngx.req.get_headers()` in `log-util`
   - a custom `log_format` referencing `$http_*`, since `modify_header()` 
invalidates the `ctx.var` cache and the read falls through to the native header
   
   **How:** the plugin now keeps the masked headers on `ctx` and `log-util` 
reads them when present. The `core.request.set_header()` call is kept, so 
nothing changes for any other status and other plugins still observe the masked 
value through `core.request.header()`.
   
   #### Which issue(s) this PR fixes:
   
   N/A
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [x] I have updated the documentation to reflect this change (no doc change 
needed: no configuration or documented behavior changes, the documented 
behavior now holds for all response statuses)
   - [x] I have verified that this change is backward compatible
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to