You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/03/13 14:39:31 UTC
[GitHub] [apisix] wilson-1024 opened a new pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
wilson-1024 opened a new pull request #6600:
URL: https://github.com/apache/apisix/pull/6600
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
support for having port in host header when pass_host = node and port is not standard
resolve #6366
### Pre-submission checklist:
<!--
Please follow the PR manners:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
6. Use "request review" to notify the reviewer once you have resolved the review
7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
-->
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068679342
@wilson-1024
There are other issues in the test file. Would be better to apply my patch 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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r829700452
##########
File path: apisix/balancer.lua
##########
@@ -176,6 +177,17 @@ local function set_balancer_opts(route, ctx)
end
+local function parse_server_for_upstream_host(picked_server, ctx)
+ local upstream_scheme = ctx.upstream_scheme
+ local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
Review comment:
Yes.Thank you for your careful instruction
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068733323
> I think this can be used as a check for retry apisix/apisix/balancer.lua line 362
>
> ```
> core.log.info("proxy request to ", server.host, ":", server.port)
> ```
The log here doesn't include the real Host header. That's why I use debug log.
Simply committing the patch is enough. The openresty in CI has debug log enabled.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068707055
Is there `--with-debug` in your `nginx -V`? Debug log is not enabled by default.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068754992
@spacewander
Thank you for your careful instruction. When i enable the nginx debugging option,the local test succeeds.
```
FLUSH_ETCD=1 prove -I. -I../test-nginx/ -I../test-nginx/lib -r t/node/upstream.t
t/node/upstream.t .. ok
All tests successful.
Files=1, Tests=71, 17 wallclock secs ( 0.03 usr 0.01 sys + 2.16 cusr 1.22 csys = 3.42 CPU)
Result: PASS
```
log file
```
[debug] 7218#2263361: *23 http proxy header:
"GET /uri HTTP/1.1
Host: 127.0.0.1:1979
X-Real-IP: 127.0.0.1
X-Forwarded-For: 127.0.0.1
X-Forwarded-Proto: http
X-Forwarded-Host: localhost
X-Forwarded-Port: 1984
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r827562623
##########
File path: t/node/upstream.t
##########
@@ -510,3 +510,125 @@ GET /uri
qr/host: localhost/
--- error_log
proxy request to 127.0.0.1:1980
+
+
+
+=== TEST 21: check that including port in host header is supported when pass_host = node and port is not standard
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1979": 1,
+ "localhost:1980": 1000
+ },
+ "type": "roundrobin",
+ "pass_host": "node"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "methods": ["GET"],
+ "upstream_id": "1",
+ "uri": "/uri"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- skip_nginx: 5: < 1.19.0
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: hit route
+--- request
+GET /uri
+--- skip_nginx: 5: < 1.19.0
+--- response_body
Review comment:
we can use:
```
--- response_body eval
qr/host: localhost:1980/
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r829700452
##########
File path: apisix/balancer.lua
##########
@@ -176,6 +177,17 @@ local function set_balancer_opts(route, ctx)
end
+local function parse_server_for_upstream_host(picked_server, ctx)
+ local upstream_scheme = ctx.upstream_scheme
+ local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
Review comment:
Yes.Thank you for your careful instruction
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r827925082
##########
File path: apisix/init.lua
##########
@@ -260,8 +260,8 @@ local function set_upstream_host(api_ctx, picked_server)
if nodes_count == 1 then
local node = up_conf.nodes[1]
api_ctx.var.upstream_host = node.domain or node.host
Review comment:
Sorry, I have been busy with work and have no time to reply.ok, I'll deal with it later.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r826586478
##########
File path: t/node/upstream.t
##########
@@ -510,3 +510,54 @@ GET /uri
qr/host: localhost/
--- error_log
proxy request to 127.0.0.1:1980
+
+
+
+=== TEST 21: check that including port in host header is supported when pass_host = node and port is not standard
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/server_upstream_host"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1979": 1,
+ "127.0.0.1:1980": 1000
+ },
+ "type": "roundrobin",
+ "pass_host": "node"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: hit route
+--- request
+GET /hello
+--- response_body
+127.0.0.1:1980
+--- no_error_log
+[error]
Review comment:
Thank you.I have solved the problem mentioned,please review it again.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r829669250
##########
File path: apisix/balancer.lua
##########
@@ -176,6 +177,17 @@ local function set_balancer_opts(route, ctx)
end
+local function parse_server_for_upstream_host(picked_server, ctx)
+ local upstream_scheme = ctx.upstream_scheme
+ local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
Review comment:
```suggestion
local function parse_server_for_upstream_host(picked_server, upstream_scheme)
local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
```
is better?
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander edited a comment on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander edited a comment on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068661619
@wilson-1024
You can apply:
```
diff --git t/node/upstream.t t/node/upstream.t
index feee95bd..a799e2fd 100644
--- t/node/upstream.t
+++ t/node/upstream.t
@@ -565,10 +565,8 @@ passed
--- request
GET /uri
--- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
+--- response_body eval
+qr/host: localhost:1980/
--- no_error_log
[error]
@@ -614,7 +612,6 @@ x-real-ip: 127.0.0.1
}
--- request
GET /t
---- skip_nginx: 5: < 1.19.0
--- response_body
passed
--- no_error_log
@@ -623,12 +620,8 @@ passed
=== TEST 24: hit route
+--- log_level: debug
--- request
GET /uri
---- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
--- error_log
-proxy request to 127.0.0.1:1980
+Host: 127.0.0.1:1979
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r827905475
##########
File path: apisix/init.lua
##########
@@ -260,8 +260,8 @@ local function set_upstream_host(api_ctx, picked_server)
if nodes_count == 1 then
local node = up_conf.nodes[1]
api_ctx.var.upstream_host = node.domain or node.host
Review comment:
We should also handle this path (nodes_count == 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6600:
URL: https://github.com/apache/apisix/pull/6600
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #6600:
URL: https://github.com/apache/apisix/pull/6600
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068704349
@spacewander I applied this patch directly, but my local test fails.Please help me.
result：
```
FLUSH_ETCD=1 prove -I. -I../test-nginx/ -I../test-nginx/lib -r t/node/upstream.t
t/node/upstream.t .. 66/?
# Failed test 't/node/upstream.t TEST 24: hit route - pattern "Host: 127.0.0.1:1979" should match a line in error.log (req 0)'
# at /Users/zenghuilin/github/apisix/../test-nginx/lib/Test/Nginx/Socket.pm line 1284.
# Looks like you failed 1 test of 71.
t/node/upstream.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/71 subtests
Test Summary Report
-------------------
t/node/upstream.t (Wstat: 256 Tests: 71 Failed: 1)
Failed test: 71
Non-zero exit status: 1
Files=1, Tests=71, 12 wallclock secs ( 0.03 usr 0.00 sys + 1.52 cusr 0.89 csys = 2.44 CPU)
Result: FAIL
```
file diff
```diff
diff --git a/t/node/upstream.t b/t/node/upstream.t
index feee95bd..c73b8391 100644
--- a/t/node/upstream.t
+++ b/t/node/upstream.t
@@ -565,10 +565,8 @@ passed
--- request
GET /uri
--- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
+--- response_body eval
+qr/host: localhost:1980/
--- no_error_log
[error]
@@ -614,7 +612,6 @@ x-real-ip: 127.0.0.1
}
--- request
GET /t
---- skip_nginx: 5: < 1.19.0
--- response_body
passed
--- no_error_log
@@ -623,12 +620,10 @@ passed
=== TEST 24: hit route
+--- log_level: debug
--- request
GET /uri
---- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
+--- response_body eval
+qr/host: localhost:1980/
--- error_log
-proxy request to 127.0.0.1:1980
+Host: 127.0.0.1:1979
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068708144
I think this can be used as a check for retry
apisix/apisix/balancer.lua line 362
```
core.log.info("proxy request to ", server.host, ":", server.port)
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r829669250
##########
File path: apisix/balancer.lua
##########
@@ -176,6 +177,17 @@ local function set_balancer_opts(route, ctx)
end
+local function parse_server_for_upstream_host(picked_server, ctx)
+ local upstream_scheme = ctx.upstream_scheme
+ local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
Review comment:
```suggestion
local function parse_server_for_upstream_host(picked_server, upstream_scheme)
local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
```
is better?
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r825600062
##########
File path: t/cli/test_standalone.sh
##########
@@ -65,3 +65,139 @@ if [ ! $code -eq 200 ]; then
fi
echo "passed: resolve variables in apisix.yaml conf success"
+
Review comment:
This file is not suitable to add this test.
Please add it in node/upstream.t like https://github.com/apache/apisix/blob/a490e0a2570d916921b4a4e50f3c308291ba7dd9/t/node/upstream.t#L400
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r826108621
##########
File path: t/cli/test_standalone.sh
##########
@@ -65,3 +65,139 @@ if [ ! $code -eq 200 ]; then
fi
echo "passed: resolve variables in apisix.yaml conf success"
+
Review comment:
Fixed. Please review it again.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander edited a comment on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander edited a comment on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068661619
@wilson-1024
You can apply:
```diff
diff --git t/node/upstream.t t/node/upstream.t
index feee95bd..a799e2fd 100644
--- t/node/upstream.t
+++ t/node/upstream.t
@@ -565,10 +565,8 @@ passed
--- request
GET /uri
--- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
+--- response_body eval
+qr/host: localhost:1980/
--- no_error_log
[error]
@@ -614,7 +612,6 @@ x-real-ip: 127.0.0.1
}
--- request
GET /t
---- skip_nginx: 5: < 1.19.0
--- response_body
passed
--- no_error_log
@@ -623,12 +620,8 @@ passed
=== TEST 24: hit route
+--- log_level: debug
--- request
GET /uri
---- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
--- error_log
-proxy request to 127.0.0.1:1980
+Host: 127.0.0.1:1979
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#discussion_r826504136
##########
File path: t/lib/server.lua
##########
@@ -531,6 +531,11 @@ function _M.plugin_proxy_rewrite_resp_header()
ngx.say(s)
end
+function _M.server_upstream_host()
Review comment:
There is no need to add a new backend. `/uri` already contains the host: https://github.com/apache/apisix/blob/530fb4d62c990ec23026f43f70b878e157d441bc/t/node/upstream.t#L510
##########
File path: t/node/upstream.t
##########
@@ -510,3 +510,54 @@ GET /uri
qr/host: localhost/
--- error_log
proxy request to 127.0.0.1:1980
+
+
+
+=== TEST 21: check that including port in host header is supported when pass_host = node and port is not standard
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/server_upstream_host"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1979": 1,
+ "127.0.0.1:1980": 1000
+ },
+ "type": "roundrobin",
+ "pass_host": "node"
+ },
+ "uri": "/hello"
Review comment:
We can use `"/uri"` directly, as the test I pointed out before
##########
File path: t/node/upstream.t
##########
@@ -510,3 +510,54 @@ GET /uri
qr/host: localhost/
--- error_log
proxy request to 127.0.0.1:1980
+
+
+
+=== TEST 21: check that including port in host header is supported when pass_host = node and port is not standard
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/server_upstream_host"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1979": 1,
+ "127.0.0.1:1980": 1000
+ },
+ "type": "roundrobin",
+ "pass_host": "node"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: hit route
+--- request
+GET /hello
+--- response_body
+127.0.0.1:1980
+--- no_error_log
+[error]
Review comment:
Also need to cover the case with retry: https://github.com/apache/apisix/blob/530fb4d62c990ec23026f43f70b878e157d441bc/t/node/upstream.t#L458
```
"nodes": {
"127.0.0.1:1979": 1000,
"localhost:1980": 1
}
```
We use 1979, a nonexistent port to trigger the retry.
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068710372
I don't have --with-debug in my nginx -V
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068661619
@wilson-1024
You can apply:
```
diff --git t/node/upstream.t t/node/upstream.t
index feee95bd..12b7123d 100644
--- t/node/upstream.t
+++ t/node/upstream.t
@@ -565,10 +565,8 @@ passed
--- request
GET /uri
--- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
+--- response_body eval
+qr/host: localhost:1980/
--- no_error_log
[error]
@@ -623,12 +621,9 @@ passed
=== TEST 24: hit route
+--- log_level: debug
--- request
GET /uri
--- skip_nginx: 5: < 1.19.0
---- response_body
-uri: /uri
-host: localhost:1980
-x-real-ip: 127.0.0.1
--- error_log
-proxy request to 127.0.0.1:1980
+Host: 127.0.0.1:1979
```
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 removed a comment on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 removed a comment on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068710203
I don't have --with-debug in your nginx -V
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068710203
I don't have --with-debug in your nginx -V
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] lijing-21 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
lijing-21 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068992692
Hi @wilson-1024 , thank you for your contribution!
Here is the Contributor T-shirt form[1], if you're interested, kindly take a look :)
[1] https://github.com/apache/apisix/blob/master/CONTRIBUTING.md#contributor-t-shirt
--
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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1068684062
@spacewander Sorry, I didn't read your comment carefully just now.I am modifying to apply your patch 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-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1071010228
There is an error in the output of CI.
```
! Installing the dependencies failed: Module 'Test::Base' is not installed, Module 'Text::Diff' is not installed
! Bailing out the installation for Test-Nginx-0.29.
2 distributions installed
Error: Process completed with exit code 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] wilson-1024 commented on pull request #6600: feat: support for having port in host header when pass_host = node and port is not standard
Posted by GitBox <gi...@apache.org>.
wilson-1024 commented on pull request #6600:
URL: https://github.com/apache/apisix/pull/6600#issuecomment-1071010228
There is an error in the output of CI.
```
! Installing the dependencies failed: Module 'Test::Base' is not installed, Module 'Text::Diff' is not installed
! Bailing out the installation for Test-Nginx-0.29.
2 distributions installed
Error: Process completed with exit code 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org