Make sure to escape user strings by mromaszewicz · Pull Request #2433 · oapi-codegen/oapi-codegen

mromaszewicz · 2026-07-06T01:19:32Z

Spec-derived paths and string enum values were interpolated into generated Go source without escaping, so a value containing a quote could produce malformed output. Route paths in the server registration templates and string enum constants are now emitted through strconv.Quote (the existing toGoString helper) so they are always valid, properly-escaped Go string literals. Generated output is unchanged for normal specs.

Spec-derived paths and string enum values were interpolated into generated Go source without escaping, so a value containing a quote could produce malformed output. Route paths in the server registration templates and string enum constants are now emitted through strconv.Quote (the existing toGoString helper) so they are always valid, properly-escaped Go string literals. Generated output is unchanged for normal specs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

greptile-apps · 2026-07-06T01:23:33Z

Greptile Summary

This PR fixes a code-generation injection bug where spec-derived strings (route paths and string enum values) were embedded into generated Go source inside double-quoted literals without escaping, so a value containing " or \ would produce malformed output. The fix threads all affected values through the existing toGoString helper (strconv.Quote) and is applied consistently across all eight router backends and the shared constants.tmpl.

All router backends updated (chi, echo, echo/v5, fiber, gin, gorilla, iris, stdhttp) — the swaggerUriToXxxUri | toGoString pipeline is consistently applied across every registration template.
constants.tmpl correctly conditioned: toGoString is only applied when ValueWrapper is non-empty (string enums), leaving numeric and boolean enum literals unchanged.
Remaining gap: the options.OperationMiddlewares[\"{{.MiddlewareKey}}\"] expression in both echo templates still embeds the raw spec OperationId without escaping — the same category of injection, same fix needed.

Confidence Score: 4/5

Safe to merge for normal specs; the MiddlewareKey gap in the echo templates is pre-existing and low-probability, but worth addressing while the fix is fresh.

The core fix is correct and comprehensive across all backends and constants. The only gap is that the echo (and echo/v5) templates still embed the raw spec OperationId in a map-key literal without going through toGoString — the exact same category of injection the PR set out to eliminate. No regression test was added to guard against future regressions on paths or enum values with special characters.

pkg/codegen/templates/echo/echo-register.tmpl and pkg/codegen/templates/echo/v5/echo-register.tmpl — the MiddlewareKey map-key literal is the remaining unescaped user string.

Important Files Changed

Filename	Overview
pkg/codegen/templates/chi/chi-handler.tmpl	Path is now passed through toGoString (strconv.Quote), preventing injection via spec-derived path strings.
pkg/codegen/templates/constants.tmpl	String enum values now use toGoString instead of raw ValueWrapper interpolation; correctly conditioned on ValueWrapper non-emptiness so non-string enums are unaffected.
pkg/codegen/templates/echo/echo-register.tmpl	Path correctly escaped via toGoString, but MiddlewareKey is still interpolated raw and could break generated code if the spec OperationId contains a quote character.
pkg/codegen/templates/echo/v5/echo-register.tmpl	Same change as echo/echo-register.tmpl; MiddlewareKey gap applies equally here.
pkg/codegen/templates/fiber/fiber-handler.tmpl	Path escaped via toGoString; no other unescaped user strings present.
pkg/codegen/templates/gin/gin-register.tmpl	Path escaped via toGoString; change is minimal and correct.
pkg/codegen/templates/gorilla/gorilla-register.tmpl	Path escaped via toGoString; change is minimal and correct.
pkg/codegen/templates/iris/iris-handler.tmpl	Path escaped via toGoString; change is minimal and correct.
pkg/codegen/templates/stdhttp/std-http-handler.tmpl	Path escaped via toGoString; the string concatenation pattern for ServeMux is preserved correctly.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
pkg/codegen/templates/echo/echo-register.tmpl:50
**Unescaped `MiddlewareKey` in map literal**

This PR fixes injection for `.Path` but the `{{.MiddlewareKey}}` value in `options.OperationMiddlewares["{{.MiddlewareKey}}"]` is still interpolated without escaping. `MiddlewareKey()` returns the raw `SpecOperationId` from the spec verbatim (see `operations.go:385`), so an operation ID containing a double-quote — e.g. `get"resource"` — would produce malformed Go output. The same issue exists in `echo/v5/echo-register.tmpl`. Replacing with `{{.MiddlewareKey | toGoString}}` would be consistent with the rest of this fix.

```suggestion
{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...)
```

### Issue 2 of 3
pkg/codegen/templates/echo/v5/echo-register.tmpl:50
Same unescaped `MiddlewareKey` issue as in `echo/echo-register.tmpl` — the raw spec `OperationId` is interpolated directly into a map-key string literal without going through `toGoString`.

```suggestion
{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...)
```

### Issue 3 of 3
pkg/codegen/templates/constants.tmpl:12
**No regression test for special-character enum values or paths**

Per the repo's contribution guidelines, bug fixes should include a regression test in `internal/test/`. Neither a new fixture spec (with a path or string enum value containing `"` or `\`) nor a corresponding `*.gen.go` was added. Without this, a future template change could silently re-introduce the injection bug and CI would not catch it.

_{Reviews (1): Last reviewed commit: "Make sure to escape user strings" | Re-trigger Greptile}

greptile-apps · 2026-07-06T01:23:37Z

    }
 {{end}}
-{{range .}}router.{{.Method}}(options.BaseURL + "{{.Path | swaggerUriToEchoUri}}", wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)
+{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)


Unescaped MiddlewareKey in map literal

This PR fixes injection for .Path but the {{.MiddlewareKey}} value in options.OperationMiddlewares["{{.MiddlewareKey}}"] is still interpolated without escaping. MiddlewareKey() returns the raw SpecOperationId from the spec verbatim (see operations.go:385), so an operation ID containing a double-quote — e.g. get"resource" — would produce malformed Go output. The same issue exists in echo/v5/echo-register.tmpl. Replacing with {{.MiddlewareKey | toGoString}} would be consistent with the rest of this fix.

Suggested change

{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)

{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...)

Prompt To Fix With AI

This is a comment left during a code review. Path: pkg/codegen/templates/echo/echo-register.tmpl Line: 50 Comment: **Unescaped `MiddlewareKey` in map literal** This PR fixes injection for `.Path` but the `{{.MiddlewareKey}}` value in `options.OperationMiddlewares["{{.MiddlewareKey}}"]` is still interpolated without escaping. `MiddlewareKey()` returns the raw `SpecOperationId` from the spec verbatim (see `operations.go:385`), so an operation ID containing a double-quote — e.g. `get"resource"` — would produce malformed Go output. The same issue exists in `echo/v5/echo-register.tmpl`. Replacing with `{{.MiddlewareKey | toGoString}}` would be consistent with the rest of this fix. ```suggestion {{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...) ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-07-06T01:23:38Z

    }
 {{end}}
-{{range .}}router.{{.Method}}(options.BaseURL + "{{.Path | swaggerUriToEchoUri}}", wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)
+{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)


Same unescaped MiddlewareKey issue as in echo/echo-register.tmpl — the raw spec OperationId is interpolated directly into a map-key string literal without going through toGoString.

Suggested change

{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)

{{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...)

Prompt To Fix With AI

This is a comment left during a code review. Path: pkg/codegen/templates/echo/v5/echo-register.tmpl Line: 50 Comment: Same unescaped `MiddlewareKey` issue as in `echo/echo-register.tmpl` — the raw spec `OperationId` is interpolated directly into a map-key string literal without going through `toGoString`. ```suggestion {{range .}}router.{{.Method}}(options.BaseURL + {{.Path | swaggerUriToEchoUri | toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey | toGoString}}]...) ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-07-06T01:23:39Z

 const (
 {{range $name, $value := $Enum.GetValues}}
-  {{$name}} {{$Enum.TypeName}} = {{$Enum.ValueWrapper}}{{$value}}{{$Enum.ValueWrapper -}}
+  {{$name}} {{$Enum.TypeName}} = {{if $Enum.ValueWrapper}}{{$value | toGoString}}{{else}}{{$value}}{{end -}}


No regression test for special-character enum values or paths

Per the repo's contribution guidelines, bug fixes should include a regression test in internal/test/. Neither a new fixture spec (with a path or string enum value containing " or \) nor a corresponding *.gen.go was added. Without this, a future template change could silently re-introduce the injection bug and CI would not catch it.

Prompt To Fix With AI

This is a comment left during a code review. Path: pkg/codegen/templates/constants.tmpl Line: 12 Comment: **No regression test for special-character enum values or paths** Per the repo's contribution guidelines, bug fixes should include a regression test in `internal/test/`. Neither a new fixture spec (with a path or string enum value containing `"` or `\`) nor a corresponding `*.gen.go` was added. Without this, a future template change could silently re-introduce the injection bug and CI would not catch it. How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Spec-derived paths and string enum values were interpolated into generated Go source without escaping, so a value containing a quote could produce malformed output. Route paths in the server registration templates and string enum constants are now emitted through strconv.Quote (the existing toGoString helper) so they are always valid, properly-escaped Go string literals. Generated output is unchanged for normal specs. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Closes: #2180 The reported miscompilation — a string enum value like N\A emitted verbatim into a double-quoted Go literal — was already fixed by #2433, which routes string enum constants through strconv.Quote. That change shipped without an end-to-end test for this scenario, so pin it with a test generating from the issue's schema and asserting the escaped constant and formattable output. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

mromaszewicz requested a review from a team as a code owner July 6, 2026 01:19

mromaszewicz added the bug Something isn't working label Jul 6, 2026

mromaszewicz merged commit d5e1d02 into main Jul 6, 2026
29 checks passed

mromaszewicz deleted the fix/string-escaping branch July 6, 2026 01:22

greptile-apps Bot reviewed Jul 6, 2026

View reviewed changes

This was referenced Jul 12, 2026

Enum values with backslash (escape character) produce invalid client code #2180

Closed

Add regression test for enum values containing backslashes #2454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Make sure to escape user strings#2433

Make sure to escape user strings#2433
mromaszewicz merged 1 commit into
mainfrom
fix/string-escaping

mromaszewicz commented Jul 6, 2026

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 6, 2026

Important Files Changed

Uh oh!

greptile-apps Bot Jul 6, 2026

Uh oh!

greptile-apps Bot Jul 6, 2026

Uh oh!

greptile-apps Bot Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	{{range .}}router.{{.Method}}(options.BaseURL + {{.Path \| swaggerUriToEchoUri \| toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares["{{.MiddlewareKey}}"]...)
	{{range .}}router.{{.Method}}(options.BaseURL + {{.Path \| swaggerUriToEchoUri \| toGoString}}, wrapper.{{.HandlerName}}, options.OperationMiddlewares[{{.MiddlewareKey \| toGoString}}]...)

Uh oh!

Uh oh!

Conversation

mromaszewicz commented Jul 6, 2026

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 6, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

greptile-apps Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant