Skip to content

Give nginx back the query string on routed requests - #1163

Merged
mastacontrola merged 1 commit into
working-1.6from
fix-nginx-querystring
Aug 18, 2026
Merged

Give nginx back the query string on routed requests#1163
mastacontrola merged 1 commit into
working-1.6from
fix-nginx-querystring

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

The bug

Anything that is not a real file is served through an internal rewrite to
api/index.php. Apache does that with [QSA,L] and carries the query string.
nginx did it with a try_files fallback naming a plain URI — and a plain URI
hands the router an empty query string.

So on nginx, every routed endpoint that reads a query parameter silently saw
nothing. Measured on the lab server before the fix:

PROBE id=0 qs='' fi=NULL get=NULL

Route::queryParam() has been working around this since #529 by re-parsing
REQUEST_URI, which is why the API's expand / start / length still worked.
Nothing outside Route could — the method was protected.

What it cost

The whole OIDC plugin, on every nginx install. Its login button links to
/ext/oidc/start?provider=3; OIDCFlow read the id with
filter_input(INPUT_GET, 'provider'), got null, and refused a configured,
enabled provider with "Unknown identity provider". callback() reads state,
code and error the same way, so the flow could not have completed either.

The fix — two halves, both needed

Half Fixes Doesn't fix
installer appends $is_args$args to all three nginx vhost writers every server that re-runs the installer an existing vhost — nothing rewrites one
Route::queryParam() keeps its REQUEST_URI fallback and becomes public every server that doesn't

Making it public is the point of the second half: a plugin registering a handler
on this same router hits exactly the same wall, and the working answer should be
reachable rather than re-invented. The OIDC plugin's ?provider= is how this was
found; the companion fix is FOGProject/fog-plugins#14.

Apache is untouched — it was always correct, and the test now pins that too.

Verification

Live on the lab server. Before the vhost fix, /fog/ext/oidc/start?provider=3
302'd straight back to the login page with "Unknown identity provider". After:

location: https://127.0.0.1:8443/realms/fog/protocol/openid-connect/auth?response_type=code
  &client_id=fog-web&redirect_uri=https%3A%2F%2F10.255.20.1%2Ffog%2Fext%2Foidc%2Fcallback
  &scope=openid+profile+email&state=…&nonce=…&code_challenge=…&code_challenge_method=S256

and a full sign-in against a real Keycloak now completes end to end.

tests/routed-query-string.test.php counts the fallbacks rather than matching
one, so a fourth vhost writer added without the suffix fails here — same for the
three Apache rewrites and their QSA. Five mutations verified, all caught.
sh tests/run-all.sh → 51 passed, 0 failed.

Downstream

None. No route class list change, no schema change, no OpenAPI change.

🤖 Generated with Claude Code

https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR

A request for anything that is not a real file is rewritten to
api/index.php. Apache does that with [QSA,L] and carries the query
string. nginx did it with a try_files fallback naming a plain URI, and a
plain URI hands the router an EMPTY query string -- so on nginx every
routed endpoint that reads a query parameter silently saw nothing.

Route::queryParam() has been working around this since GH-529: it
re-parses REQUEST_URI when $_GET is empty, which is why the API's
expand, start and length still worked. Nothing outside Route could,
because queryParam() was protected.

That is the whole of the OIDC plugin on nginx. Its login button links to
/ext/oidc/start?provider=3; OIDCFlow reads the id with
filter_input(INPUT_GET, 'provider'), gets null, and refuses a configured
and enabled provider with "Unknown identity provider". Its callback
reads state, code and error the same way, so the flow could not have
completed either. Measured on the lab server: QUERY_STRING arrived as
'' and the id as 0.

Two halves, both needed. The installer appends $is_args$args to all
three nginx vhost writers, which fixes it at the source -- but only for
a server that re-runs the installer, and an existing vhost is not
rewritten by anything. So queryParam() keeps its REQUEST_URI fallback
and becomes public, because a plugin registering a handler on this
router hits exactly the same wall and should not have to re-invent the
answer. Apache is untouched; it was always correct.

tests/routed-query-string.test.php counts the fallbacks rather than
matching one, so a fourth vhost writer added without the suffix fails
here. Five mutations verified, all caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
@mastacontrola
mastacontrola merged commit a69dd22 into working-1.6 Aug 18, 2026
3 checks passed
@mastacontrola
mastacontrola deleted the fix-nginx-querystring branch August 18, 2026 00:05
mastacontrola added a commit that referenced this pull request Aug 18, 2026
Carries FOGProject/fog-plugins#14: the OIDC flow reads its query
parameters through Route::queryParam(), which #1163 made public. Without
this bump the fix cannot reach a server -- fetch-plugins.sh downloads
whatever this constant names.


Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant