kong.service.request.set_raw_query() doesn't work reliably

[shanexguo]

Is there an existing issue for this?

• I have searched the existing issues

Kong version ($ kong version)

3.0+

Current Behavior

When we tried to use the Kong API to manipulate the incoming HTTP request query parameters, e.g. in the plugin "access" function, like below:

local orignal_args = kong.request.get_raw_query()
... [append, remove, modify the query parameters as a string] 
kong.service.request.set_raw_query(qparams_str)g API to modify query parameters string

We noticed that the set_raw_query() method doesn't work. I've found an existing bug report on the same issue, see the link below:
#9492

We don't like calling ngx.var.args to achieve the same goal, as it invokes yet another API. Plus, we've found that even this API doesn't work if the original HTTP request doesn't have any query parameters.

Our conclusion, after experimenting with both API, is that they don't work reliably for all situations. We want it to work consistently as specified below:

Expected Behavior

1. We use Kong API to read the existing query parameters from the incoming request. It should return either nil or "" or empty table if there is none
2. We then manipulate the existing query parameters, either as a string or a LUA table, by appending, removing or modifying query parameter
3. We call Kong API to set the query parameters

The above 3 steps should always work in the Kong plugin access phase.

Steps To Reproduce

1. Create a kong 3.0 custom plugin
2. In its access function, try to use Kong API to retrieve the raw query parameters
3. Try to append a new query parameter, remove an existing parameter, and modify an existing parameter
4. Call Kong API set_raw_qeury() to set the updated query parameter string
5. Observe if the service/upstream received the request with the updated query parameters

Anything else?

No response

[StarlightIbuki]

We have planed an investigation into this issue.

[samugi]

Hi @shanexguo I am working on this to find a long term solution. It appears that the problem occurs (as per my older investigation in the issue you have linked) when the nginx variable ngx.var.args is accessed before being modified via ngx.req.set_uri_args().

In order to unblock you and also understand whether this would fit your use case, could you test the following code?

  -- fetch uri and raw args
  local uri  = kong.request.get_path_with_query()

  -- extract args
  local q    = string.find(uri, "?", 2, true)
  local args = q and string.sub(uri, q + 1, -1) or ""

  -- process args
  args = args .. "&foo=bar"

  -- set new args
  kong.service.request.set_raw_query(args)

[samugi]

Just an update on my investigation and a summary of what appears to be happening:

• get_raw_query() returns: ngx.var.args
• set_raw_query() invokes: ngx.req.set_uri_args()
• We patch the ngx.var metatable to improve performance of get and set on ngx.var.* variables.

The problem is that after args has been indexed once, if ngx.req.set_uri_args() is called, it modifies the value of args without accessing the metatable, i.e. without using our patched index.

Later, when we access ngx.var.args to populate upstream_uri, we read through our patched index (which remained outdated) and find the inconsistent value. This means that after any access to ngx.var.args , any subsequent call to ngx.req.set_uri_args() will be ineffective.

ngx.req.set_uri_args("foo=bar") --> args is not yet indexed --> this works
a = ngx.var.args                --> does a get(), indexes the variable
ngx.req.set_uri_args("upd=val") --> this is ineffective
print(ngx.var.args)             --> prints "foo=bar"

Fixing this in the pdk should be easy enough, I will follow up with some update soon

[shanexguo]

Many thanks for the update. It would be awesome if we can call the PDK API to both read, update, and set the query parameters. For now, we are using various workarounds in our plugin code to avoid this issue. These bind-aid type of codes are pretty ugly :) Thanks,

…

On Tue, Jan 24, 2023 at 10:32 AM Samuele Illuminati < ***@***.***> wrote: Just an update on my investigation and a summary of what appears to be happening: - get_raw_query() returns: ngx.var.args - set_raw_query() invokes: ngx.req.set_uri_args() - We patch the ngx.var metatable <https://github.com/Kong/lua-kong-nginx-module/blob/master/lualib/resty/kong/var.lua#L153-L187> to improve performance of get and set on ngx.var.* variables. The problem is that after args has been indexed once, if ngx.req.set_uri_args() is called, it modifies the value of args without accessing the metatable, i.e. without using our patched index. Later, when we access ngx.var.args to populate upstream_uri <https://github.com/Kong/kong/blob/e95b730952cf2bcd77dcc442a04b497a6ab49ffa/kong/runloop/handler.lua#L1310>, we read through our patched index (which remained outdated) and find the inconsistent value. This means that after any access to ngx.var.args , any subsequent call to ngx.req.set_uri_args() will be ineffective. ngx.req.set_uri_args("foo=bar") --> args is not yet indexed --> this works a = ngx.var.args --> does a get(), indexes the variable ngx.req.set_uri_args("upd=val") --> this is ineffective print(ngx.var.args) --> prints "foo=bar" Fixing this in the pdk should be easy enough, I will follow up with some update soon — Reply to this email directly, view it on GitHub <#10080 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXINGJCCYEELYIPQDR666PLWT7YYPANCNFSM6AAAAAATTE62RM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[samugi]

totally agree @shanexguo , the fix is in progress, what I provided was just a temporary workaround. Glad you are not blocked by this.

[shanexguo]

Which Kong release version will this fix be included? Thanks

…

On Mon, Jan 30, 2023, 01:50 Datong Sun ***@***.***> wrote: Closed #10080 <#10080> as completed via #10181 <#10181>. — Reply to this email directly, view it on GitHub <#10080 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXINGJG76TFQ5F5VVVRQEBTWU5QEFANCNFSM6AAAAAATTE62RM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[locao]

This fix will be part of Kong 3.2.0.