Skip to content

Commit

Permalink
fix(clustering): hybrid mode should accept special characters used in…
Browse files Browse the repository at this point in the history
… forward proxy password (#13457)

Hybrid mode connections not working if the forward proxy password contains
special character(`#`). Even after the `proxy_server` is url encoded, it still
doesn't work and reports a "407 Proxy Authentication Required" error.

This fix tries to address this issue. But, keep in mind that even with this fix,
`proxy_server` still needs to be url-encoded.

(cherry picked from commit 2c4b98b)
  • Loading branch information
ms2008 committed Aug 14, 2024
1 parent f6d4e8a commit 4939987
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Fixed an issue where hybrid mode not working if the forward proxy password contains special character(#). Note that the `proxy_server` configuration parameter still needs to be url-encoded.
type: bugfix
scope: Clustering
4 changes: 2 additions & 2 deletions kong.conf.default
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@
# stack traces to help improve Kong.


#proxy_server = # Proxy server defined as a URL. Kong will only use this
# option if a component is explicitly configured
#proxy_server = # Proxy server defined as an encoded URL. Kong will only
# use this option if a component is explicitly configured
# to use a proxy.


Expand Down
7 changes: 4 additions & 3 deletions kong/clustering/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ local type = type
local table_insert = table.insert
local table_concat = table.concat
local encode_base64 = ngx.encode_base64
local unescape_uri = ngx.unescape_uri
local worker_id = ngx.worker.id
local fmt = string.format

Expand Down Expand Up @@ -50,14 +51,14 @@ local function parse_proxy_url(proxy_server)
-- the connection details is statically rendered in nginx template

else -- http
ret.proxy_url = fmt("%s://%s:%s", parsed.scheme, parsed.host, parsed.port or 443)
ret.proxy_url = fmt("%s://%s:%s", parsed.scheme, unescape_uri(parsed.host), parsed.port or 443)
ret.scheme = parsed.scheme
ret.host = parsed.host
ret.host = unescape_uri(parsed.host)
ret.port = parsed.port
end

if parsed.user and parsed.password then
ret.proxy_authorization = "Basic " .. encode_base64(parsed.user .. ":" .. parsed.password)
ret.proxy_authorization = "Basic " .. encode_base64(unescape_uri(parsed.user) .. ":" .. unescape_uri(parsed.password))
end
end

Expand Down
24 changes: 24 additions & 0 deletions spec/01-unit/03-conf_loader_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,18 @@ describe("Configuration loader", function()
assert.is_nil(errors)
assert.is_table(conf)

local conf, _, errors = conf_loader(nil, {
proxy_server = "http://😉.tld",
})
assert.is_nil(errors)
assert.is_table(conf)

local conf, _, errors = conf_loader(nil, {
proxy_server = "http://%F0%9F%98%89.tld",
})
assert.is_nil(errors)
assert.is_table(conf)

local conf, _, errors = conf_loader(nil, {
proxy_server = "://localhost:2333",
})
Expand All @@ -1255,6 +1267,18 @@ describe("Configuration loader", function()
})
assert.contains("fragments, query strings or parameters are meaningless in proxy configuration", errors)
assert.is_nil(conf)

local conf, _, errors = conf_loader(nil, {
proxy_server = "http://user:password%23@localhost:2333",
})
assert.is_nil(errors)
assert.is_table(conf)

local conf, _, errors = conf_loader(nil, {
proxy_server = "http://user:password#@localhost:2333",
})
assert.contains("fragments, query strings or parameters are meaningless in proxy configuration", errors)
assert.is_nil(conf)
end)

it("doesn't allow cluster_use_proxy on CP but allows on DP", function()
Expand Down
6 changes: 3 additions & 3 deletions spec/02-integration/09-hybrid_mode/10-forward-proxy_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ local fixtures = {
content_by_lua_block {
require("spec.fixtures.forward-proxy-server").connect({
basic_auth = ngx.encode_base64("test:konghq"),
basic_auth = ngx.encode_base64("test:konghq#"),
})
}
}
Expand All @@ -56,15 +56,15 @@ local proxy_configs = {
proxy_server_ssl_verify = "off",
},
["https off auth on"] = {
proxy_server = "http://test:konghq@127.0.0.1:16796",
proxy_server = "http://test:konghq%23@127.0.0.1:16796",
proxy_server_ssl_verify = "off",
},
["https on auth off"] = {
proxy_server = "https://127.0.0.1:16799",
proxy_server_ssl_verify = "off",
},
["https on auth on"] = {
proxy_server = "https://test:konghq@127.0.0.1:16798",
proxy_server = "https://test:konghq%23@127.0.0.1:16798",
proxy_server_ssl_verify = "off",
},
["https on auth off verify on"] = {
Expand Down

0 comments on commit 4939987

Please sign in to comment.