Repository navigation
Quote or validate every spec string that reaches generated Go source (2.8.x backport) - #2626
Merged
mromaszewicz merged 3 commits intoOct 24, 2026
Conversation
…(2.8.x)
Hand port of the main change to the 2.8.x templates and code: the two
initiator templates and both Echo register templates carry the sinks that
main has in one file each, strict-fiber-v3.tmpl has a raw content-type
literal that main's unified Fiber template already quotes, and the
discriminator literal helper does not exist here, so that item is dropped.
- webhook-initiator.tmpl and callback-initiator.tmpl emitted the request
body's content type as "{{.ContentType}}", and strict-fiber-v3.tmpl did
the same in its multi-body content-type match; they now go through
toGoString like the client's does. MIME parameters may legitimately
contain quoted strings, which also used to produce code that did not
compile.
- echo/echo-register.tmpl and echo/v5/echo-register.tmpl emitted the raw
operationId as the OperationMiddlewares key inside literal quotes; they
now go through toGoString. ValidateSpec additionally rejects control
characters in an operationId.
- A non-string enum is emitted unquoted, and its values were whatever the
spec wrote. GenerateEnums now requires each such value to be a number or
boolean literal and reports the offending value otherwise.
- toStringArray joined security scopes with `","`: a scope ending in a
backslash escaped the quote the joiner supplied and the next scope became
code. Each element is now quoted on its own.
- Response keys are copied into type names and, for the client, into
`rsp.StatusCode == <key>`; they were never checked. ValidateSpec now
requires a three-digit code, a range such as 2XX, or default.
- Component names (schemas, parameters, request bodies, responses, headers,
callbacks, security schemes) reach single-line doc comments verbatim, so
a newline ended the comment; ValidateSpec rejects control characters in
them. Quotes stay allowed, since the names become identifiers only after
sanitization.
- A response header's style is copied into a string literal by the strict
templates like a parameter's; it is now checked the same way.
- The server-urls generator uses x-go-name, variable names, undeclared
{placeholder} names and enum values as identifiers without sanitizing
them. A new ValidateServerURLs, run only when server URLs are generated
so other specs are unaffected, requires each to be a Go identifier (enum
values: identifier characters, since they are appended to the type name).
Generated fixtures are unchanged: every valid value renders byte-for-byte
as before. Each item has a regression test; the end-to-end ones generate
from a crafted document and check that the payload stays inside a string
literal or is rejected with a clear error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback on oapi-codegen#2625. ValidateServerURLs checked every declared variable, but the generator only emits code for variables whose {name} placeholder appears in the URL, so a server with an unused hyphenated variable, or an unused enum value such as us-east, was refused although it generated fine before. Limit the name and enum-value checks to UsedVariables, keeping the separate check for undeclared placeholders, and cover the unused case in the test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`checkExtensions` in `validate_spec.go` validated `x-go-name`, `x-go-type-name`, `x-go-type`, and `x-oapi-codegen-extra-tags`, but had no case for `x-go-type-import`. This adds validation for both fields of that extension: - `name`: must be empty, `"."` (dot-import), or a valid Go identifier; handled by a new `checkGoImportName` method - `path`: must not contain control characters; handled by a new `checkGoImportPath` method (the path is emitted with `%q` so control characters would be escaped anyway, but we reject obviously malformed inputs early) Also adds `validate_spec_test.go` with tests covering valid inputs, invalid `name` and `path` values, case-insensitive key matching (mirroring `ParseGoImportExtension`), and regression coverage for the pre-existing extension checks. Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
 [Medium impact] The changes affect OpenAPI-to-Go generation across its supported output paths. The PR appears safe to merge, with a non-blocking gap in the new enum check. Findings
Fix with agent prompt### Issue 1
pkg/codegen/codegen.go:1567-1568
`strconv.ParseFloat` accepts `NaN` and `+Inf`, even though they are not Go number literals. An integer schema with `enum: ['NaN']` and `x-enum-varnames: ['First']` passes this guard. The template then emits `First Level = NaN`, which refers to an undefined name and does not compile.
Check Go literal syntax rather than float parsing, and add regression tests for these special values.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThis backport quotes spec strings in generated Go and adds checks for values emitted without quotes.
Reviews (1) · Last reviewed commit: "Add missing case to validation (#2493)" · Reviewed by Greptile |
Comment on lines
+1567
to
+1568
| _, err := strconv.ParseFloat(s, 64) | ||
| return err == nil |
Contributor
There was a problem hiding this comment.
strconv.ParseFloat accepts NaN and +Inf, even though they are not Go number literals. An integer schema with enum: ['NaN'] and x-enum-varnames: ['First'] passes this guard. The template then emits First Level = NaN, which refers to an undefined name and does not compile.
Check Go literal syntax rather than float parsing, and add regression tests for these special values.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/codegen.go
Line: 1567-1568
Comment:
**Bad enums pass the check**
`strconv.ParseFloat` accepts `NaN` and `+Inf`, even though they are not Go number literals. An integer schema with `enum: ['NaN']` and `x-enum-varnames: ['First']` passes this guard. The template then emits `First Level = NaN`, which refers to an undefined name and does not compile.
Check Go literal syntax rather than float parsing, and add regression tests for these special values.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport of #2625 to
release/v2.8.x, applied by hand because the templates had diverged, plus the one earlier validation fix this release line was missing. Three commits, meant to be squashed:webhook-initiator.tmplandcallback-initiator.tmpl(main has a singleinitiator.tmpl).OperationMiddlewareskey is quoted in both the Echo v4 and the Echo v5 register templates (main shares one).strict-fiber-v3.tmplhad a raw"{{.ContentType}}"in its content-type prefix check that main's unified Fiber template already quotes; it is quoted here too.Discriminator.literalitem is dropped: that function does not exist on 2.8.x.toStringArray, response key / component name / operationId / header style validation,ValidateServerURLs) is the same code, and the newvalidate_spec_test.goand end-to-end tests come along.x-go-type-importname and path validation, cherry-picked. It had never reached this release line, and on 2.8.x the import name is still emitted unquoted into the import block, so it belongs in the same patch release.After these,
pkg/codegen/validate_spec.goand its test file are byte-identical tomain.Compatibility
No generated file changes:
GOTOOLCHAIN=go1.26.10 make generateproduces no diff, so every valid value renders byte-for-byte as before. The new validation only rejects documents that already produced code that did not compile, or code the document author had no business producing.ValidateServerURLsruns only when server URLs are generated and only looks at variables the URL actually uses.Testing
go test ./pkg/codegen/ ./cmd/...and lint pass locally.🤖 Generated with Claude Code