AlinsRan commented on PR #13803:
URL: https://github.com/apache/apisix/pull/13803#issuecomment-5261330667

   Good catch, and the asymmetry was an oversight rather than a decision — 
thank you.
   
   X-Forwarded-For is the one header this change *clears* rather than 
overwrites, so it is the one where the original is actually lost, and it was 
the one whose `set` I removed as unused. Restored in 612a826d, together with 
documentation for all five next to `trusted_addresses`:
   
   ```nginx
   set $apisix_orig_xf_proto  $http_x_forwarded_proto;
   set $apisix_orig_xf_host   $http_x_forwarded_host;
   set $apisix_orig_xf_port   $http_x_forwarded_port;
   set $apisix_orig_forwarded $http_forwarded;
   set $apisix_orig_xf_for    $http_x_forwarded_for;
   ```
   
   Readable from a log format, or from Lua as `ctx.var.apisix_orig_xf_for`.
   
   On retaining `ctx.var.original_x_forwarded_for` under its old name: I went 
with the equivalent variable instead. The old field was written from Lua on 
every request on the default path — no `trusted_addresses` configured, nothing 
trusted — which is the work this change exists to remove. The config-level 
variable costs one rewrite-phase assignment in C and is symmetric with the 
other four. Migration for an out-of-tree plugin is 
`ctx.var.original_x_forwarded_for` → `ctx.var.apisix_orig_xf_for`, and the 
`conf/config.yaml.example` note names it as the replacement. Happy to restore 
the Lua field as well if you would rather not have any rename at all.
   
   One thing worth recording, since it looks like it should be a problem and is 
not: reading `$http_x_forwarded_for` in the rewrite phase indexes the variable, 
so it keeps the client's raw value for the rest of the request. That does not 
weaken the sanitization — `$proxy_add_x_forwarded_for` builds its value from 
`r->headers_in.x_forwarded_for` directly rather than from the variable, so an 
untrusted peer's chain still does not reach the upstream.
   
   TEST 17 covers the regression you asked for, pinning both halves at once: 
with `trusted_addresses: 10.0.0.0/8` and a request carrying `X-Forwarded-For: 
9.9.9.9, 8.8.8.8` from `127.0.0.1`, a plugin in the access phase — after 
sanitization — reads `9.9.9.9, 8.8.8.8` from `ctx.var.apisix_orig_xf_for`, 
while the upstream receives `x-forwarded-for: 127.0.0.1`.


-- 
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