Skip to content

Commit 730c94f

Browse files
mromaszewiczclaude
mromaszewicz
and
claude
committed
sanitize param names for http.ServeMux
Go's net/http ServeMux requires wildcard segment names to be valid Go identifiers. OpenAPI specs can use path parameter names containing dashes (e.g. "addressing-identifier"), which causes a panic when registering routes with ServeMux. Fix by sanitizing parameter names in the stdhttp code path: - SwaggerUriToStdHttpUri now sanitizes param names via SanitizeGoIdentity so route patterns use valid Go identifiers (e.g. {addressing_identifier}) - stdhttp middleware template uses new SanitizedParamName for r.PathValue() calls to match the sanitized route pattern, while keeping the original ParamName for error messages - Add SanitizedParamName() method to ParameterDefinition for use by templates that need the sanitized form Add server-specific test directory with per-router integration tests exercising dashed path parameter names. Right now, only stdhttp has a test in this directory, but we'll do router specific tests in there in the future. Fixes #2278 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a13ef41 commit 730c94f

File tree

9 files changed

+284
-10
lines changed

9 files changed

+284
-10
lines changed

‎internal/test/server-specific/spec.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
openapi: "3.0.0"
2+
info:
3+
version: 1.0.0
4+
title: Server-specific tests
5+
paths:
6+
# The dashed path parameter name "addressing-identifier" is not a valid Go
7+
# identifier. This exercises GitHub issue #2278: stdhttp's ServeMux requires
8+
# wildcard names to be valid Go identifiers.
9+
/resources/{addressing-identifier}:
10+
get:
11+
operationId: getResource
12+
parameters:
13+
- name: addressing-identifier
14+
in: path
15+
required: true
16+
schema:
17+
type: string
18+
responses:
19+
"200":
20+
description: OK
21+
content:
22+
text/plain:
23+
schema:
24+
type: string

‎internal/test/server-specific/stdhttp/config.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# yaml-language-server: $schema=../../../../configuration-schema.json
2+
package: stdhttp
3+
generate:
4+
std-http-server: true
5+
models: true
6+
output: server.gen.go

‎internal/test/server-specific/stdhttp/doc.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package stdhttp
2+
3+
//go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen --config=config.yaml ../spec.yaml

‎internal/test/server-specific/stdhttp/server.gen.go‎

Lines changed: 179 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/test/server-specific/stdhttp/server_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//go:build go1.22
2+
3+
package stdhttp
4+
5+
import (
6+
"fmt"
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
type testServer struct {
15+
receivedParam string
16+
}
17+
18+
func (s *testServer) GetResource(w http.ResponseWriter, r *http.Request, addressingIdentifier string) {
19+
s.receivedParam = addressingIdentifier
20+
_, _ = fmt.Fprint(w, addressingIdentifier)
21+
}
22+
23+
func TestDashedPathParam(t *testing.T) {
24+
server := &testServer{}
25+
handler := Handler(server)
26+
27+
req := httptest.NewRequest(http.MethodGet, "/resources/my-value", nil)
28+
rec := httptest.NewRecorder()
29+
handler.ServeHTTP(rec, req)
30+
31+
assert.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d; body: %s", rec.Code, rec.Body.String())
32+
assert.Equal(t, "my-value", server.receivedParam, "path parameter was not correctly extracted")
33+
}

‎pkg/codegen/operations.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,14 @@ func (pd *ParameterDefinition) SchemaFormat() string {
151151
return ""
152152
}
153153

154+
// SanitizedParamName returns the parameter name sanitized to be a valid Go
155+
// identifier. This is needed for routers like net/http's ServeMux where path
156+
// wildcards (e.g. {name}) must be valid Go identifiers. For the original
157+
// OpenAPI parameter name (e.g. for error messages or JSON tags), use ParamName.
158+
func (pd ParameterDefinition) SanitizedParamName() string {
159+
return SanitizeGoIdentifier(pd.ParamName)
160+
}
161+
154162
func (pd ParameterDefinition) GoVariableName() string {
155163
name := LowercaseFirstCharacters(pd.GoName())
156164
if IsGoKeyword(name) {

‎pkg/codegen/templates/stdhttp/std-http-middleware.tmpl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,17 @@ func (siw *ServerInterfaceWrapper) {{$opid}}(w http.ResponseWriter, r *http.Requ
1919
var {{$varName := .GoVariableName}}{{$varName}} {{.TypeDef}}
2020

2121
{{if .IsPassThrough}}
22-
{{$varName}} = r.PathValue("{{.ParamName}}")
22+
{{$varName}} = r.PathValue("{{.SanitizedParamName}}")
2323
{{end}}
2424
{{if .IsJson}}
25-
err = json.Unmarshal([]byte(r.PathValue("{{.ParamName}}")), &{{$varName}})
25+
err = json.Unmarshal([]byte(r.PathValue("{{.SanitizedParamName}}")), &{{$varName}})
2626
if err != nil {
2727
siw.ErrorHandlerFunc(w, r, &UnmarshalingParamError{ParamName: "{{.ParamName}}", Err: err})
2828
return
2929
}
3030
{{end}}
3131
{{if .IsStyled}}
32-
err = runtime.BindStyledParameterWithOptions("{{.Style}}", "{{.ParamName}}", r.PathValue("{{.ParamName}}"), &{{$varName}}, runtime.BindStyledParameterOptions{ParamLocation: runtime.ParamLocationPath, Explode: {{.Explode}}, Required: {{.Required}}, Type: "{{.SchemaType}}", Format: "{{.SchemaFormat}}"})
32+
err = runtime.BindStyledParameterWithOptions("{{.Style}}", "{{.ParamName}}", r.PathValue("{{.SanitizedParamName}}"), &{{$varName}}, runtime.BindStyledParameterOptions{ParamLocation: runtime.ParamLocationPath, Explode: {{.Explode}}, Required: {{.Required}}, Type: "{{.SchemaType}}", Format: "{{.SchemaFormat}}"})
3333
if err != nil {
3434
siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "{{.ParamName}}", Err: err})
3535
return

‎pkg/codegen/utils.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -638,8 +638,9 @@ func SwaggerUriToGorillaUri(uri string) string {
638638
}
639639

640640
// SwaggerUriToStdHttpUri converts a swagger style path URI with parameters to a
641-
// Chi compatible path URI. We need to replace all Swagger parameters with
642-
// "{param}". Valid input parameters are:
641+
// net/http ServeMux compatible path URI. Parameter names are sanitized to be
642+
// valid Go identifiers, as required by ServeMux wildcard segments. Valid input
643+
// parameters are:
643644
//
644645
// {param}
645646
// {param*}
@@ -656,7 +657,10 @@ func SwaggerUriToStdHttpUri(uri string) string {
656657
return "/{$}"
657658
}
658659

659-
return pathParamRE.ReplaceAllString(uri, "{$1}")
660+
return pathParamRE.ReplaceAllStringFunc(uri, func(match string) string {
661+
sub := pathParamRE.FindStringSubmatch(match)
662+
return "{" + SanitizeGoIdentifier(sub[1]) + "}"
663+
})
660664
}
661665

662666
// OrderedParamsFromUri returns the argument names, in order, in a given URI string, so for
@@ -755,9 +759,12 @@ func IsValidGoIdentity(str string) bool {
755759
return !IsPredeclaredGoIdentifier(str)
756760
}
757761

758-
// SanitizeGoIdentity deletes and replaces the illegal runes in the given
759-
// string to use the string as a valid identity.
760-
func SanitizeGoIdentity(str string) string {
762+
// SanitizeGoIdentifier replaces illegal runes in the given string so that
763+
// it is a valid Go identifier. Unlike SanitizeGoIdentity, it does not
764+
// prefix reserved keywords or predeclared identifiers. This is useful for
765+
// contexts where the name must be a valid identifier but is not used as a
766+
// Go symbol (e.g. net/http ServeMux wildcard names).
767+
func SanitizeGoIdentifier(str string) string {
761768
sanitized := []rune(str)
762769

763770
for i, c := range sanitized {
@@ -768,7 +775,14 @@ func SanitizeGoIdentity(str string) string {
768775
}
769776
}
770777

771-
str = string(sanitized)
778+
return string(sanitized)
779+
}
780+
781+
// SanitizeGoIdentity deletes and replaces the illegal runes in the given
782+
// string to use the string as a valid identity. It also prefixes reserved
783+
// keywords and predeclared identifiers with an underscore.
784+
func SanitizeGoIdentity(str string) string {
785+
str = SanitizeGoIdentifier(str)
772786

773787
if IsGoKeyword(str) || IsPredeclaredGoIdentifier(str) {
774788
str = "_" + str

‎pkg/codegen/utils_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,13 @@ func TestSwaggerUriToStdHttpUriUri(t *testing.T) {
454454
assert.Equal(t, "/path/{arg}/foo", SwaggerUriToStdHttpUri("/path/{;arg*}/foo"))
455455
assert.Equal(t, "/path/{arg}/foo", SwaggerUriToStdHttpUri("/path/{?arg}/foo"))
456456
assert.Equal(t, "/path/{arg}/foo", SwaggerUriToStdHttpUri("/path/{?arg*}/foo"))
457+
458+
// Parameter names that are not valid Go identifiers must be sanitized (issue #2278)
459+
assert.Equal(t, "/path/{addressing_identifier}", SwaggerUriToStdHttpUri("/path/{addressing-identifier}"))
460+
assert.Equal(t, "/path/{my_param}/{other_param}", SwaggerUriToStdHttpUri("/path/{my-param}/{other-param}"))
461+
462+
// Go keywords are valid ServeMux wildcard names and should not be prefixed
463+
assert.Equal(t, "/path/{type}", SwaggerUriToStdHttpUri("/path/{type}"))
457464
}
458465

459466
func TestOrderedParamsFromUri(t *testing.T) {

0 commit comments

Comments
 (0)