From a408200fecf4e6d40d47b9c47c3d86b2cc36dae9 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:39:34 +0300 Subject: [PATCH 1/2] fix(compilers/openapi): stop stamping querystring with a style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- compilers/openapi/conformance_test.go | 30 ++ .../openapi/internal/operation/params.go | 19 +- .../openapi/internal/operation/params_test.go | 56 +++- .../openapi/param-querystring.golden.json | 281 ++++++++++++++++++ .../openapi/param-querystring.yaml | 30 ++ 5 files changed, 409 insertions(+), 7 deletions(-) create mode 100644 testdata/conformance/openapi/param-querystring.golden.json create mode 100644 testdata/conformance/openapi/param-querystring.yaml diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d9..56e3f38 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -180,6 +180,7 @@ func conformanceCases() []conformanceCase { {"tags-grouping", assertTagsGrouping}, {"http-binding", assertHTTPBinding}, {"param-styles", assertParamStyles}, + {"param-querystring", assertParamQuerystring}, {"param-xml-residue", assertParamXMLResidue}, {"param-ref-inheritance", assertParamRefInheritance}, {"header-content-schema", assertHeaderContentSchema}, @@ -1452,6 +1453,35 @@ func assertAllowEmptyValueKept(t *testing.T, op ir.Operation) { assert.JSONEq(t, `true`, string(entry.Value)) } +// assertParamQuerystring pins the 3.2 querystring location, where the whole +// query string binds from the parameter's content: the binding carries that +// media type and neither style nor explode, the two keywords the location +// forbids (GitHub #334). The ordinary query parameter beside it keeps the +// defaults its own location does admit, so what separates them is the location +// rather than the presence of content. +func assertParamQuerystring(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { + report, ok := opByName(doc, "runReport") + require.True(t, ok) + require.Len(t, report.Bindings.HTTP, 1) + require.Len(t, report.Bindings.HTTP[0].ParamBindings, 1) + qs := report.Bindings.HTTP[0].ParamBindings[0] + assert.Equal(t, ir.HTTPLocationQuerystring, qs.Location) + assert.Equal(t, "application/x-www-form-urlencoded", qs.ContentType, + "the media type is the whole of a querystring binding's declared serialization") + assert.Empty(t, qs.Style, "style is not a legal keyword at in: querystring") + assert.Nil(t, qs.Explode, "and explode qualifies a style there is none of") + + summary, ok := opByName(doc, "summarize") + require.True(t, ok) + require.Len(t, summary.Bindings.HTTP, 1) + require.Len(t, summary.Bindings.HTTP[0].ParamBindings, 1) + q := summary.Bindings.HTTP[0].ParamBindings[0] + assert.Equal(t, ir.HTTPLocationQuery, q.Location) + assert.Equal(t, "form", q.Style, "a query parameter still takes its own location's default") + require.NotNil(t, q.Explode) + assert.True(t, *q.Explode) +} + // assertParamRefInheritance pins ir-design §14 at a parameter whose schema is a // $ref: docs, deprecation and default come from the referent when the use site is // silent, and from the use site when it is not. Constraints inherit at neither diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee..d563004 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -307,13 +307,20 @@ func preserveAllowEmptyValue(c lowering.Ctx, param *ir.Parameter, p *soa.Paramet // resolveStyleExplode materializes a parameter's resolved serialization style // and explode flag: an explicit value wins, else the OpenAPI per-location -// default (query/cookie → form/true, path/header → simple/false). The result is -// declared facts, not policy. +// default (query/cookie → form/true, path/header → simple/false, querystring → +// neither). The result is declared facts, not policy. func resolveStyleExplode(p *soa.Parameter, in soa.ParameterIn) (string, *bool) { style := defaultParamStyle(in) if p.Style != nil { style = string(*p.Style) } + // Explode qualifies a style, so a parameter resolving to none takes neither + // the default nor a declared explode — 3.2 forbids that keyword at + // in: querystring too, whose ContentType is the whole of its declared + // serialization (GitHub #334). + if style == "" { + return "", nil + } explode := style == string(soa.SerializationStyleForm) if p.Explode != nil { explode = *p.Explode @@ -322,10 +329,14 @@ func resolveStyleExplode(p *soa.Parameter, in soa.ParameterIn) (string, *bool) { } // defaultParamStyle returns the OpenAPI default serialization style for a -// parameter location. +// parameter location, and "" for one that has none: 3.2 binds in: querystring +// from the parameter's content and forbids style there, so that location has no +// default to fall back on. func defaultParamStyle(in soa.ParameterIn) string { switch in { - case soa.ParameterInQuery, soa.ParameterInCookie, soa.ParameterInQueryString: + case soa.ParameterInQueryString: + return "" + case soa.ParameterInQuery, soa.ParameterInCookie: return string(soa.SerializationStyleForm) default: return string(soa.SerializationStyleSimple) diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index ab7feed..3d8f8a3 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -3,6 +3,7 @@ package operation_test import ( "testing" + "github.com/speakeasy-api/openapi/validation" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -180,19 +181,68 @@ func TestParams_AllLocationsAndStyles(t *testing.T) { assert.True(t, hasDiag(diags, diag.NumericPrecision), "malformed param constraint warns") } +// TestParams_QueryStringLocation pins the whole of a querystring binding's +// declared serialization: the 3.2 location, and the media type from content +// carrying it alone. Style and explode are not legal keywords there, so the +// binding takes neither rather than the query defaults (GitHub #334). func TestParams_QueryStringLocation(t *testing.T) { t.Parallel() spec := pathsSpecVer("3.2.0", ` /q: get: operationId: q parameters: - - {name: qs, in: querystring, schema: {type: string}} + - name: qs + in: querystring + content: + application/x-www-form-urlencoded: + schema: {type: object, properties: {page: {type: string}}} + responses: + "200": {description: ok} +`) + doc, diags := parseFull(t, spec) + requireNoErrorDiags(t, diags) + op := findOp(t, doc, "q") + require.Len(t, op.Bindings.HTTP, 1) + require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1) + + qs := op.Bindings.HTTP[0].ParamBindings[0] + assert.Equal(t, ir.HTTPLocationQuerystring, qs.Location) + assert.Equal(t, "application/x-www-form-urlencoded", qs.ContentType) + assert.Empty(t, qs.Style, "3.2 binds the query string from content and forbids style there") + assert.Nil(t, qs.Explode, "and explode with it: it qualifies a style, and there is none") +} + +// TestParams_QueryStringDeclaredStyleIsKeptAndReported pins the other half of +// the same rule: the compiler stopped inventing a style at that location, it did +// not start erasing one. A document declaring style there is invalid and the +// parser says so; what it declared still lowers, because dropping declared +// content is an emitter's call rather than a compiler's. +func TestParams_QueryStringDeclaredStyleIsKeptAndReported(t *testing.T) { + t.Parallel() + spec := pathsSpecVer("3.2.0", ` /q: + get: + operationId: q + parameters: + - name: qs + in: querystring + style: form + explode: false + content: + application/x-www-form-urlencoded: + schema: {type: object} responses: "200": {description: ok} `) - doc, _ := parseFull(t, spec) + doc, diags := parseFull(t, spec) + assertHasCode(t, diags, diag.Validation+"/"+string(validation.RuleValidationAllowedValues), ir.SeverityError) op := findOp(t, doc, "q") - assert.Equal(t, ir.HTTPLocationQuerystring, op.Bindings.HTTP[0].ParamBindings[0].Location) + require.Len(t, op.Bindings.HTTP, 1) + require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1) + + qs := op.Bindings.HTTP[0].ParamBindings[0] + assert.Equal(t, "form", qs.Style, "the declared style lowers as declared") + require.NotNil(t, qs.Explode) + assert.False(t, *qs.Explode, "and so does the explode qualifying it") } const componentParamRefSpec = `openapi: 3.1.0 diff --git a/testdata/conformance/openapi/param-querystring.golden.json b/testdata/conformance/openapi/param-querystring.golden.json new file mode 100644 index 0000000..f6fcd78 --- /dev/null +++ b/testdata/conformance/openapi/param-querystring.golden.json @@ -0,0 +1,281 @@ +{ + "irVersion": "0.3.0", + "name": "ParamQuerystring", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "ParamQuerystring", + "canonical": "param_querystring" + }, + "docs": {}, + "groups": [ + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1reports/get", + "name": { + "source": "runReport", + "canonical": "run_report" + }, + "docs": {}, + "params": [ + { + "name": { + "source": "criteria", + "canonical": "criteria" + }, + "type": { + "target": "t/anon/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema", + "nullable": false + }, + "required": false, + "docs": {} + } + ], + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "GET", + "uriTemplate": "/reports", + "sharedRoute": false, + "paramBindings": [ + { + "param": "criteria", + "location": "querystring", + "wireName": "criteria", + "allowReserved": false, + "contentType": "application/x-www-form-urlencoded" + } + ], + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get" + } + }, + { + "id": "op/openapi/paths/~1reports~1summary/get", + "name": { + "source": "summarize", + "canonical": "summarize" + }, + "docs": {}, + "params": [ + { + "name": { + "source": "status", + "canonical": "status" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {} + } + ], + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "GET", + "uriTemplate": "/reports/summary", + "sharedRoute": false, + "paramBindings": [ + { + "param": "status", + "location": "query", + "wireName": "status", + "style": "form", + "explode": true, + "allowReserved": false + } + ], + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports~1summary/get" + } + } + ] + } + ], + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "types": { + "t/anon/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema": { + "kind": "model", + "id": "t/anon/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema", + "name": { + "hint": "criteria" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema" + }, + "properties": [ + { + "id": "p/openapi/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema/properties/since", + "name": { + "source": "since", + "canonical": "since" + }, + "wireName": "since", + "type": { + "target": "t/prim/date", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema/properties/since" + } + }, + { + "id": "p/openapi/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema/properties/status", + "name": { + "source": "status", + "canonical": "status" + }, + "wireName": "status", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/parameters/0/content/application~1x-www-form-urlencoded/schema/properties/status" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/prim/date": { + "kind": "primitive", + "id": "t/prim/date", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "date" + }, + "t/prim/string": { + "kind": "primitive", + "id": "t/prim/string", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "string" + } + }, + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "auth": null + } + ], + "sources": [ + { + "format": "openapi@3.2", + "path": "param-querystring.yaml", + "hash": "f0cf6d786e63396444a439407e38ad19f5dbcb65928593e9d8818c5baf01b463" + } + ] +} diff --git a/testdata/conformance/openapi/param-querystring.yaml b/testdata/conformance/openapi/param-querystring.yaml new file mode 100644 index 0000000..0026b95 --- /dev/null +++ b/testdata/conformance/openapi/param-querystring.yaml @@ -0,0 +1,30 @@ +openapi: 3.2.0 +info: {title: ParamQuerystring, version: "1.0.0"} +paths: + # 3.2 binds the whole query string from the parameter's content, so style and + # explode are not legal keywords here and the binding carries neither. + /reports: + get: + operationId: runReport + parameters: + - name: criteria + in: querystring + content: + application/x-www-form-urlencoded: + schema: + type: object + properties: + since: {type: string, format: date} + status: {type: string} + responses: + "200": {description: ok} + # The contrast, in its own operation because a querystring parameter may not + # share one with an in: query parameter: a location that does admit the + # defaults, so the golden shows the difference is the location. + /reports/summary: + get: + operationId: summarize + parameters: + - {name: status, in: query, schema: {type: string}} + responses: + "200": {description: ok} From 5dd7b8f68e7289eee3d51ed2b412cd5dbc067f8f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 07:18:03 +0300 Subject: [PATCH 2/2] fix(compilers/openapi): keep an explode declared without a style 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. --- .../openapi/internal/operation/params.go | 18 ++++++---- .../openapi/internal/operation/params_test.go | 35 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index d563004..24a0366 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -314,17 +314,21 @@ func resolveStyleExplode(p *soa.Parameter, in soa.ParameterIn) (string, *bool) { if p.Style != nil { style = string(*p.Style) } - // Explode qualifies a style, so a parameter resolving to none takes neither - // the default nor a declared explode — 3.2 forbids that keyword at - // in: querystring too, whose ContentType is the whole of its declared - // serialization (GitHub #334). + // A declared explode lowers as declared wherever it is written, for the same + // reason a declared style does: 3.2 forbids both at in: querystring, but + // dropping content the document states is an emitter's call, not a + // compiler's, and this position has an IR field to hold it. + if p.Explode != nil { + explode := *p.Explode + return style, &explode + } + // Absent one, explode qualifies a style, so a position resolving to no style + // gets no default either — a querystring binding's ContentType is the whole + // of its declared serialization (GitHub #334). if style == "" { return "", nil } explode := style == string(soa.SerializationStyleForm) - if p.Explode != nil { - explode = *p.Explode - } return style, &explode } diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index 3d8f8a3..0df0aed 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -245,6 +245,41 @@ func TestParams_QueryStringDeclaredStyleIsKeptAndReported(t *testing.T) { assert.False(t, *qs.Explode, "and so does the explode qualifying it") } +// TestParams_QueryStringDeclaredExplodeAloneIsKept covers the half of that rule +// the case above cannot see, because it declares both keywords: an explode +// written without a style beside it. +// +// Suppressing the invented style must not take a declared explode with it. 3.2 +// forbids explode at this location as it forbids style, and the bundled parser +// refuses neither — so this reaches the compiler, and erasing it would be the +// same silent drop the invented style was, in the other direction. +func TestParams_QueryStringDeclaredExplodeAloneIsKept(t *testing.T) { + t.Parallel() + spec := pathsSpecVer("3.2.0", ` /q: + get: + operationId: q + parameters: + - name: qs + in: querystring + explode: false + content: + application/x-www-form-urlencoded: + schema: {type: object} + responses: + "200": {description: ok} +`) + doc, diags := parseFull(t, spec) + requireNoErrorDiags(t, diags) + op := findOp(t, doc, "q") + require.Len(t, op.Bindings.HTTP, 1) + require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1) + + qs := op.Bindings.HTTP[0].ParamBindings[0] + assert.Empty(t, qs.Style, "no style is invented at this location") + require.NotNil(t, qs.Explode, "but the declared explode is not dropped with it") + assert.False(t, *qs.Explode) +} + const componentParamRefSpec = `openapi: 3.1.0 info: {title: T, version: "1"} paths: