[GitHub] [apisix] spacewander commented on a diff in pull request #8275: refactor: remove if statement in nginx.conf
spacewander commented on code in PR #8275: URL: https://github.com/apache/apisix/pull/8275#discussion_r1064296492 ## apisix/init.lua: ## @@ -267,6 +267,11 @@ local function set_upstream_headers(api_ctx, picked_server) api_ctx.var.var_x_forwarded_host = x_forwarded_host end +local x_forwarded_for = api_ctx.var.proxy_add_x_forwarded_for +if x_forwarded_for then +api_ctx.var.var_x_forwarded_for = x_forwarded_for Review Comment: Yes. It seems there are no builtin replacements for the others. -- 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: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8275: refactor: remove if statement in nginx.conf
spacewander commented on code in PR #8275: URL: https://github.com/apache/apisix/pull/8275#discussion_r1063198605 ## apisix/init.lua: ## @@ -267,6 +267,11 @@ local function set_upstream_headers(api_ctx, picked_server) api_ctx.var.var_x_forwarded_host = x_forwarded_host end +local x_forwarded_for = api_ctx.var.proxy_add_x_forwarded_for +if x_forwarded_for then +api_ctx.var.var_x_forwarded_for = x_forwarded_for Review Comment: Maybe we can set `proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for` directly? -- 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: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8275: refactor: remove if statement in nginx.conf
spacewander commented on code in PR #8275: URL: https://github.com/apache/apisix/pull/8275#discussion_r1062128762 ## apisix/core/ctx.lua: ## @@ -201,6 +201,7 @@ do var_x_forwarded_proto = true, var_x_forwarded_port = true, var_x_forwarded_host = true, +var_x_forwarded_for = true, Review Comment: Bad indent ## apisix/init.lua: ## @@ -267,6 +267,17 @@ local function set_upstream_headers(api_ctx, picked_server) api_ctx.var.var_x_forwarded_host = x_forwarded_host end +local x_forwarded_for = api_ctx.var.http_x_forwarded_for Review Comment: Would be better to use http://nginx.org/en/docs/http/ngx_http_proxy_module.html#var_proxy_add_x_forwarded_for directly? -- 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: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8275: refactor: remove if statement in nginx.conf
spacewander commented on code in PR #8275: URL: https://github.com/apache/apisix/pull/8275#discussion_r1035561242 ## t/plugin/proxy-rewrite3.t: ## @@ -455,3 +455,99 @@ passed GET /echo HTTP/1.1 --- response_headers test: test_in_set + + + +=== TEST 21: set route (test if X-Forwarded-Port can be set before proxy) Review Comment: This PR contains https://github.com/apache/apisix/pull/8404. We can deal with it after #8404 is merged. -- 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: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
