Skip to content

fix(router): keep the query string when normalize_uri_like_servlet is on - #13982

Open
AlinsRan wants to merge 1 commit into
apache:masterfrom
AlinsRan:fix/servlet-uri-query-string
Open

AlinsRan wants to merge 1 commit into
apache:masterfrom
AlinsRan:fix/servlet-uri-query-string

Conversation

@AlinsRan

Copy link
Copy Markdown
Contributor

Description

With apisix.normalize_uri_like_servlet: true, every proxied request loses its query string: the upstream receives the path only.

_M.http_access_phase() sets $upstream_uri to the original path so that servlet upstreams still see the parameters after ;:

api_ctx.var.upstream_uri = core.utils.uri_safe_encode(uri)

uri is ngx.var.uri, which never carries the query string. Assigning $upstream_uri at all is what causes the loss: with the variable left empty, proxy_pass $upstream_scheme://apisix_backend$upstream_uri falls back to forwarding the client's request line verbatim, query string included. A non-empty value turns that fallback off, and nothing puts the query string back.

This PR re-appends $is_args$args to the forwarded path, the same way proxy-rewrite already does when it rewrites the path itself, so the two paths that set $upstream_uri now produce the same shape. That also fixes proxy-mirror, which treats a non-empty $upstream_uri as already containing the query string.

An encoded question mark stays percent-encoded in the forwarded path, as before — uri_safe_encode() escapes ?, so the appended ? is the only unencoded one.

Which issue(s) this PR fixes:

N/A

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible

Documentation is unchanged: the query string reaching the upstream is the behavior the option was always meant to have and is not described anywhere as being dropped, so there is nothing to correct in docs/.

Tests: t/router/servlet-upstream-uri.t gains a case asserting the upstream sees both the servlet parameters and the query string. It fails on master (upstream receives /anything/sub;jsessionid=x, query gone) and passes with this change. The existing case covering %3F staying percent-encoded is unaffected.

With `normalize_uri_like_servlet` enabled, the access phase pins
$upstream_uri to the original path so upstreams still receive the
parameters after ';'. Assigning $upstream_uri at all switches off
nginx's "empty proxy_pass URI" passthrough, and that passthrough is
what otherwise hands the client's request line -- query string
included -- to the upstream, so every proxied request lost its query
string.

Re-append $is_args$args to the forwarded path, the same way
proxy-rewrite already does when it rewrites the path itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants