GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||
GeezerVison is showing github.com — Quote or validate every spec string that reaches generated Go source (2.8.x backport) by mromaszewicz · Pull Request #2626 · oapi-codegen/oapi-codegen Conversation
Member
SummaryBackport of #2625 to
After these, CompatibilityNo generated file changes: Testing
🤖 Generated with Claude Code
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
…(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 <[email protected]>
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 <[email protected]> `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) <[email protected]>
|
There was an error while loading. Please reload this page.
[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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| _, err := strconv.ParseFloat(s, 64) | ||
| return err == nil |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.