Make sure to escape user strings (#2433)#2434
Conversation
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 SummaryThis PR fixes a code-injection bug where spec-derived path strings and string enum values were interpolated raw into Go source, allowing a value containing a double-quote to produce malformed generated code. All seven router-backend templates and
Confidence Score: 4/5Safe to merge — the template changes are mechanically correct, generated output is byte-for-byte identical for normal (no-special-char) specs, and all backends are updated consistently. The fix is straightforward and low-risk, but there is no new regression test in
|
| Filename | Overview |
|---|---|
| pkg/codegen/templates/constants.tmpl | String enum values now use toGoString when ValueWrapper is set; logic is correct but no regression test exists for enum values containing quote characters. |
| pkg/codegen/templates/chi/chi-handler.tmpl | Route path now passed through toGoString (strconv.Quote) instead of bare interpolation inside quotes — correct fix. |
| pkg/codegen/templates/echo/echo-register.tmpl | Path interpolation fixed with toGoString; analogous to the chi change. |
| pkg/codegen/templates/echo/v5/echo-register.tmpl | Echo v5 template updated identically to Echo v4 — correct parity. |
| pkg/codegen/templates/fiber/fiber-handler.tmpl | Path interpolation fixed with toGoString; analogous to the other backend fixes. |
| pkg/codegen/templates/gin/gin-register.tmpl | Path interpolation fixed with toGoString; analogous to the other backend fixes. |
| pkg/codegen/templates/gorilla/gorilla-register.tmpl | Path interpolation fixed with toGoString; analogous to the other backend fixes. |
| pkg/codegen/templates/iris/iris-handler.tmpl | Path interpolation fixed with toGoString; analogous to the other backend fixes. |
| pkg/codegen/templates/stdhttp/std-http-handler.tmpl | Path interpolation fixed with toGoString; generated output is identical for paths without special characters because strconv.Quote produces the same quoted literal. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pkg/codegen/templates/constants.tmpl:12
**Missing regression test for the injection fix**
The project convention (as codified in the review rules and via `internal/test/issues/`) is that bug fixes include a regression test keyed to the issue number. This PR has no new entry under `internal/test/issues/` covering a string enum value that contains a quote (e.g. `fo"o`) or a route path with a backslash or quote. Without such a fixture, CI's `make generate` guard cannot detect a future regression that re-introduces the unescaped interpolation, and there is no `_test.go` exercising the boundary condition.
Reviews (1): Last reviewed commit: "Make sure to escape user strings (#2433)" | Re-trigger Greptile
| 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 -}} |
There was a problem hiding this comment.
The project convention (as codified in the review rules and via internal/test/issues/) is that bug fixes include a regression test keyed to the issue number. This PR has no new entry under internal/test/issues/ covering a string enum value that contains a quote (e.g. fo"o) or a route path with a backslash or quote. Without such a fixture, CI's make generate guard cannot detect a future regression that re-introduces the unescaped interpolation, and there is no _test.go exercising the boundary condition.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/templates/constants.tmpl
Line: 12
Comment:
**Missing regression test for the injection fix**
The project convention (as codified in the review rules and via `internal/test/issues/`) is that bug fixes include a regression test keyed to the issue number. This PR has no new entry under `internal/test/issues/` covering a string enum value that contains a quote (e.g. `fo"o`) or a route path with a backslash or quote. Without such a fixture, CI's `make generate` guard cannot detect a future regression that re-introduces the unescaped interpolation, and there is no `_test.go` exercising the boundary condition.
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.