feat: support for having port in host header when pass_host = node and port is not standard (#6600)

Co-authored-by: spacewander <spacewanderlzx@gmail.com>

apisix/balancer.lua

@@ -18,6 +18,7 @@ local require           = require
 local balancer          = require("ngx.balancer")
 local core              = require("apisix.core")
 local priority_balancer = require("apisix.balancer.priority")
+local apisix_upstream   = require("apisix.upstream")
 local ipairs            = ipairs
 local is_http           = ngx.config.subsystem == "http"
 local enable_keepalive = balancer.enable_keepalive and is_http
@@ -176,6 +177,16 @@ local function set_balancer_opts(route, ctx)
 end
 
 
+local function parse_server_for_upstream_host(picked_server, upstream_scheme)
+    local standard_port = apisix_upstream.scheme_to_port[upstream_scheme]
+    local host = picked_server.domain or picked_server.host
+    if upstream_scheme and (not standard_port or standard_port ~= picked_server.port) then
+        host = host .. ":" .. picked_server.port
+    end
+    return host
+end
+
+
 -- pick_server will be called:
 -- 1. in the access phase so that we can set headers according to the picked server
 -- 2. each time we need to retry upstream
@@ -189,6 +200,7 @@ local function pick_server(route, ctx)
         local node = up_conf.nodes[1]
         ctx.balancer_ip = node.host
         ctx.balancer_port = node.port
+        node.upstream_host = parse_server_for_upstream_host(node, ctx.upstream_scheme)
         return node
     end
 
@@ -250,6 +262,7 @@ local function pick_server(route, ctx)
     ctx.balancer_ip = res.host
     ctx.balancer_port = res.port
     ctx.server_picker = server_picker
+    res.upstream_host = parse_server_for_upstream_host(res, ctx.upstream_scheme)
 
     return res
 end
@@ -331,7 +344,7 @@ function _M.run(route, ctx, plugin_funcs)
         local header_changed
         local pass_host = ctx.pass_host
         if pass_host == "node" and balancer.recreate_request then
-            local host = server.domain or server.host
+            local host = server.upstream_host
             if host ~= ctx.var.upstream_host then
                 -- retried node has a different host
                 ctx.var.upstream_host = host

apisix/init.lua

@@ -257,11 +257,8 @@ local function set_upstream_host(api_ctx, picked_server)
     end
 
     local nodes_count = up_conf.nodes and #up_conf.nodes or 0
-    if nodes_count == 1 then
+    if nodes_count == 1 or ngx_balancer.recreate_request then
-        local node = up_conf.nodes[1]
+        api_ctx.var.upstream_host = picked_server.upstream_host
-        api_ctx.var.upstream_host = node.domain or node.host
-    elseif picked_server.domain and ngx_balancer.recreate_request then
-        api_ctx.var.upstream_host = picked_server.domain
     end
 end
 

apisix/upstream.lua

@@ -165,68 +165,69 @@ local function set_upstream_scheme(ctx, upstream)
 end
 
 
-local fill_node_info
+local scheme_to_port = {
-do
+    http = 80,
-    local scheme_to_port = {
+    https = 443,
-        http = 80,
+    grpc = 80,
-        https = 443,
+    grpcs = 443,
-        grpc = 80,
+}
-        grpcs = 443,
-    }
 
-    function fill_node_info(up_conf, scheme, is_stream)
-        local nodes = up_conf.nodes
-        if up_conf.nodes_ref == nodes then
-            -- filled
-            return true
-        end
 
-        local need_filled = false
+_M.scheme_to_port = scheme_to_port
-        for _, n in ipairs(nodes) do
-            if not is_stream and not n.port then
-                if up_conf.scheme ~= scheme then
-                    return nil, "Can't detect upstream's scheme. " ..
-                                "You should either specify a port in the node " ..
-                                "or specify the upstream.scheme explicitly"
-                end
 
-                need_filled = true
-            end
 
-            if not n.priority then
+local function fill_node_info(up_conf, scheme, is_stream)
-                need_filled = true
+    local nodes = up_conf.nodes
-            end
+    if up_conf.nodes_ref == nodes then
-        end
+        -- filled
-
-        up_conf.original_nodes = nodes
-
-        if not need_filled then
-            up_conf.nodes_ref = nodes
-            return true
-        end
-
-        local filled_nodes = core.table.new(#nodes, 0)
-        for i, n in ipairs(nodes) do
-            if not n.port or not n.priority then
-                filled_nodes[i] = core.table.clone(n)
-
-                if not is_stream and not n.port then
-                    filled_nodes[i].port = scheme_to_port[scheme]
-                end
-
-                -- fix priority for non-array nodes and nodes from service discovery
-                if not n.priority then
-                    filled_nodes[i].priority = 0
-                end
-            else
-                filled_nodes[i] = n
-            end
-        end
-
-        up_conf.nodes_ref = filled_nodes
-        up_conf.nodes = filled_nodes
         return true
     end
+
+    local need_filled = false
+    for _, n in ipairs(nodes) do
+        if not is_stream and not n.port then
+            if up_conf.scheme ~= scheme then
+                return nil, "Can't detect upstream's scheme. " ..
+                            "You should either specify a port in the node " ..
+                            "or specify the upstream.scheme explicitly"
+            end
+
+            need_filled = true
+        end
+
+        if not n.priority then
+            need_filled = true
+        end
+    end
+
+    up_conf.original_nodes = nodes
+
+    if not need_filled then
+        up_conf.nodes_ref = nodes
+        return true
+    end
+
+    local filled_nodes = core.table.new(#nodes, 0)
+    for i, n in ipairs(nodes) do
+        if not n.port or not n.priority then
+            filled_nodes[i] = core.table.clone(n)
+
+            if not is_stream and not n.port then
+                filled_nodes[i].port = scheme_to_port[scheme]
+            end
+
+            -- fix priority for non-array nodes and nodes from service discovery
+            if not n.priority then
+                filled_nodes[i].priority = 0
+            end
+        else
+            filled_nodes[i] = n
+        end
+    end
+
+    up_conf.nodes_ref = filled_nodes
+    up_conf.nodes = filled_nodes
+    return true
 end
 
 

t/cli/test_access_log.sh

@@ -209,14 +209,14 @@ sleep 4
 tail -n 2 logs/access.log > output.log
 
 # APISIX
-if ! grep '"https://localhost" -' output.log; then
+if ! grep '"https://localhost:9180" -' output.log; then
     echo "failed: should find upstream scheme"
     cat output.log
     exit 1
 fi
 
 # admin
-if ! grep '"http://localhost" localhost' output.log; then
+if ! grep '"http://localhost:9180" localhost' output.log; then
     echo "failed: should find upstream scheme"
     cat output.log
     exit 1

t/node/https-proxy.t

@@ -258,5 +258,5 @@ host: www.sni.com
 Receive SNI: localhost
 --- response_body
 uri: /uri
-host: localhost
+host: localhost:1983
 x-real-ip: 127.0.0.1

t/node/upstream.t

@@ -510,3 +510,117 @@ 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": {
+                        "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 22: hit route
+--- request
+GET /uri
+--- response_body eval
+qr/host: localhost:1980/
+--- no_error_log
+[error]
+
+
+
+=== TEST 23: check that including port in host header is supported when retrying and 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": 1000,
+                        "localhost:1980": 1
+                    },
+                    "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)
+        }
+    }
+--- skip_nginx: 5: < 1.19.0
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 24: hit route
+--- log_level: debug
+--- skip_nginx: 5: < 1.19.0
+--- request
+GET /uri
+--- error_log
+Host: 127.0.0.1:1979