AlinsRan commented on issue #13753:
URL: https://github.com/apache/apisix/issues/13753#issuecomment-5276049619

   https://github.com/apache/apisix/pull/13803 fixes the two approaches this 
issue asks for. Measured against `master`, same route, upstream echoing what it 
received:
   
   | | `master` | with #13803 |
   |---|---|---|
   | `proxy-rewrite` `headers.remove: ["X-Forwarded-Host"]` | 
`x-forwarded-host: localhost` | header not sent |
   | `proxy-rewrite` `headers.set: {"X-Forwarded-Host": 
"my-upstream.example.com"}` | `x-forwarded-host: localhost` | 
`x-forwarded-host: my-upstream.example.com` |
   
   **Why it did not work.** `location /` carried `proxy_set_header 
X-Forwarded-Host $var_x_forwarded_host;`, and `$var_x_forwarded_host` was 
populated from `set_upstream_x_forwarded_headers` in `apisix/init.lua`. That 
function only assigned the variable when `ctx.var.http_x_forwarded_host` was 
non-nil, so `headers.remove` — which sets the header to nil — left the variable 
at its `set $var_x_forwarded_host $host;` default and the original value was 
sent regardless. The plugin genuinely removed the header from `r->headers_in`; 
`proxy_set_header` then put a value back.
   
   #13803 removes `proxy_set_header X-Forwarded-Proto/Host/Port` altogether. 
`r->headers_in` is already correct by the time the request is proxied, and 
`proxy_pass` forwards it as it stands, so whatever a plugin leaves there is 
what the upstream receives. (`X-Forwarded-For` keeps its `proxy_set_header`, 
because only `$proxy_add_x_forwarded_for` appends the connection address.)
   
   **One caveat if you are using a workaround today.** Approaches 5 and 6 in 
your report — `ngx.var.var_x_forwarded_host = ` from a `before_proxy` plugin — 
do work on `master` in my test, and they stop working with #13803: 
`$var_x_forwarded_host` becomes a `map` output that no longer feeds 
`proxy_set_header`, so the assignment is a silent no-op. 
`$var_x_forwarded_proto` ceases to exist. If you have that workaround in place, 
switch to `core.request.set_header(ctx, "X-Forwarded-Host", ...)` or 
`proxy-rewrite`, which is the supported path once the PR lands.


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