Skip to content

Commit

Permalink
fix(pdk): make kong.service.request.clear_query_arg encode spaces as …
Browse files Browse the repository at this point in the history
…%20 instead of + (#14277)

### Summary

The `+` encoding is more correct (in query strings), but it seems to cause some
problems with some users. As a short-term solution I propose that we just convert
`+` to `%20`. This is more in-lines what Nginx and OpenResty seems to be doing as
well.

Signed-off-by: Aapo Talvensaari <[email protected]>
  • Loading branch information
bungle authored Feb 14, 2025
1 parent 712efe9 commit 479274e
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
message: |
**kong.service.request.clear_query_arg**: Changed the encoding of spaces in query arguments from `+`
to `%20` as a short-term solution to an issue that some users are reporting. While the `+` is correct
encoding of space in querystrings, Kong uses `%20` in many other APIs (inherited from Nginx / OpenResty)
type: breaking_change
scope: Plugin
7 changes: 6 additions & 1 deletion kong/pdk/service/request.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ local table_concat = table.concat
local type = type
local string_find = string.find
local string_sub = string.sub
local string_gsub = string.gsub
local string_byte = string.byte
local string_lower = string.lower
local normalize_multi_header = checks.normalize_multi_header
Expand Down Expand Up @@ -295,7 +296,11 @@ local function new(self)

local args = ngx_var.args
if args and args ~= "" then
ngx_var.args = search_remove(args, name)
args = search_remove(args, name)
if string_find(args, "+", nil, true) then
args = string_gsub(args, "+", "%%20")
end
ngx_var.args = args
end
end

Expand Down
36 changes: 36 additions & 0 deletions t/01-pdk/06-service-request/14-clear_query_arg.t
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,39 @@ GET /t?a=0&d=1&a=2&c=3&a=4&b=5&a=6&d=7&a=8
query: d=1&c=3&b=5&d=7
--- no_error_log
[error]
=== TEST 8: service.request.clear_query_arg() uses %20 as space instead of +
--- http_config eval
qq{
$t::Util::HttpConfig
server {
listen unix:$ENV{TEST_NGINX_NXSOCK}/nginx.sock;
location /t {
content_by_lua_block {
ngx.say("query: " .. tostring(ngx.var.args))
}
}
}
}
--- config
location = /t {
access_by_lua_block {
local PDK = require "kong.pdk"
local pdk = PDK.new()
pdk.service.request.clear_query_arg("a")
}
proxy_pass http://unix:/$TEST_NGINX_NXSOCK/nginx.sock;
}
--- request
GET /t?a=0&c+d=1+2&a=2&c%20d=3%20&a=4&b+d=5&%20a+=6+%20+&+%20+d=+%20+7%20++&a=8
--- response_body
query: c%20d=1%202&c%20d=3%20&b%20d=5&%20a%20=6%20%20%20&%20%20%20d=%20%20%207%20%20%20
--- no_error_log
[error]

1 comment on commit 479274e

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong-dev:479274e1376ccd40fe16b0a42328c6624d7e1d4b
Artifacts available https://github.com/Kong/kong/actions/runs/13338199607

Please sign in to comment.