Skip to content

fix(compilers/openapi): stop stamping querystring with a style - #364

Open
OmarAlJarrah wants to merge 2 commits into
mainfrom
fix/openapi-querystring-no-style
Open

fix(compilers/openapi): stop stamping querystring with a style#364
OmarAlJarrah wants to merge 2 commits into
mainfrom
fix/openapi-querystring-no-style

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

defaultParamStyle put in: querystring in the same arm as query and cookie, so an
OpenAPI 3.2 parameter that binds the whole query string came out of the compiler carrying
style: "form" and explode: true. Neither keyword is legal there: 3.2 binds the query
string from the parameter's content and forbids style at that location, and the bundled
parser refuses the declaration outright. The IR was therefore recording a serialization fact
the source cannot state, and an emitter reading style: form would serialize the parameter
as a form-exploded field rather than as the media type contentType already names.

The location now resolves to no style, and a parameter that resolves to none takes no
explode either — explode qualifies a style, and there is none to qualify. resolveStyleExplode's
doc comment named only two of the three cases in its table; it now states the third.

A style declared at that location anyway still lowers as declared, beside the error
diagnostic the parser already raises for it: the compiler stops inventing a style there, it
does not start erasing one. That is the same posture the reserved-header parameter takes —
dropping declared content is an emitter's call rather than a compiler's.

Before and after, compiling a parameter declared as

- name: whole
  in: querystring
  content:
    application/x-www-form-urlencoded:
      schema: {type: object}
   "param": "whole",
   "location": "querystring",
   "wireName": "whole",
-  "style": "form",
-  "explode": true,
   "allowReserved": false,
   "contentType": "application/x-www-form-urlencoded"

Both compiles report no diagnostics at all, so nothing flagged the invented style before.
Query parameters in the same document are untouched: they keep form/true.

Out of scope, and stated here because the change makes it reachable: 3.2 also forbids
explode and allowReserved at in: querystring, and the parser validates neither. A
document that declares explode there without a style now loses it with the style it was
qualifying, and a declared allowReserved still lowers onto the binding as it did before.

Test plan

  • testdata/conformance/openapi/param-querystring.yaml joins the conformance corpus with the
    contrast the golden needs: a querystring binding, and an ordinary query parameter in its own
    operation (3.2 forbids the two sharing one) that keeps the defaults its own location does
    admit. assertParamQuerystring reads both, so the golden shows the difference is the
    location rather than the presence of content.
  • TestParams_QueryStringLocation previously asserted only the location, from a fixture that
    declared schema — which 3.2 forbids there too — and discarded its diagnostics. It now
    compiles a valid content-bound parameter, requires no error diagnostics, and pins the whole
    binding: location, media type, no style, no explode.
  • TestParams_QueryStringDeclaredStyleIsKeptAndReported pins the other half: a declared style
    survives and the validation error is reported.

Both assertions bite. Restoring the old arm reddens them:

--- FAIL: TestConformance/param-querystring
    Error:      Should be empty, but was form
    Messages:   style is not a legal keyword at in: querystring
    Error:      Expected nil, but got: (*bool)(0xcf51f02d928)
    Messages:   and explode qualifies a style there is none of

--- FAIL: TestParams_QueryStringLocation
    Error:      Should be empty, but was form
    Messages:   3.2 binds the query string from content and forbids style there
    Error:      Expected nil, but got: (*bool)(0x1ed0c6c19605)
    Messages:   and explode with it: it qualifies a style, and there is none

The golden moves with them: compiling the new corpus spec with the old arm restored differs
from the committed golden by exactly the two keys above. Deleting a property from the fixture
reddens the golden comparison too, so the snapshot is reading the spec rather than agreeing
with itself.

Gate: gofmt clean, go vet ./... clean, golangci-lint run reports 0 issues, go build ./...
succeeds, ./scripts/check-coverage.sh passes at 100% (4945 statements).

Closes #334

Follow-up in this branch: a declared explode is kept

Self-review caught a regression the first commit introduced. Suppressing the
invented style took a declared explode with it, because
resolveStyleExplode returned early whenever the resolved style was empty:

# in: querystring, explode: false, no style declared
main @ dbf0054  -> {"location":"querystring","style":"form","explode":false,...}
first commit    -> {"location":"querystring",...}          # explode gone, no diagnostic
now             -> {"location":"querystring","explode":false,...}

That was the same silent drop the invented style was, in the other direction,
and it contradicted the rule the style half already states: 3.2 forbids both
keywords at this location, the bundled parser refuses neither, so both reach the
compiler — and dropping content a document states is an emitter's call rather
than a compiler's. A declared explode now lowers as declared wherever it is
written; only the default is suppressed where there is no style to qualify.

TestParams_QueryStringDeclaredStyleIsKeptAndReported could not catch this: it
declares style and explode together, so the early return was never reached.
TestParams_QueryStringDeclaredExplodeAloneIsKept covers the gap, and restoring
the exact pre-fix body of resolveStyleExplode fails that test and only that
test.

defaultParamStyle put in: querystring in the same arm as query and
cookie, so a 3.2 parameter binding the whole query string came out of the
compiler carrying style: "form" and explode: true. Neither keyword is
legal at that location: 3.2 binds the query string from the parameter's
content and forbids style there, and the bundled parser refuses the
declaration outright. The IR therefore recorded a serialization fact the
source cannot state, and an emitter reading style: form would serialize
the parameter as a form-exploded field instead of as the media type
ContentType already names.

The location now has no default style, and a parameter that resolves to
none takes no explode either — explode qualifies a style, and there is
none to qualify. A style declared at that location anyway still lowers as
declared, beside the parser's error diagnostic: the compiler stops
inventing a style there, it does not start erasing one.

param-querystring joins the conformance corpus with the contrast the
golden needs — a querystring binding beside an ordinary query parameter,
which keeps the defaults its own location does admit.
Suppressing the invented querystring style took a declared explode with
it: resolveStyleExplode returned early whenever the resolved style was
empty, so `in: querystring` with `explode: false` and no style beside it
reached the IR with no explode at all, and nothing said so.

That is the same silent drop the invented style was, in the other
direction, and it contradicts the rule the style half already follows --
3.2 forbids both keywords at that location and the bundled parser
refuses neither, so both reach the compiler, and dropping content a
document states is an emitter's call rather than a compiler's. A
declared explode now lowers as declared wherever it is written; only the
default is suppressed where there is no style to qualify.

The existing test could not see this: it declares style and explode
together, so the early return it would have tripped was never reached.
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.

openapi: querystring parameters are stamped with a style the spec forbids

1 participant