Skip to content

Commit b5a3ece

Browse files
mromaszewiczflorentchauveauclaude
authored
fix: unescape URL-encoded path params before OpenAPI validation (#43)
gorillamux uses UseEncodedPath(), so FindRoute() returns path parameters in percent-encoded form. openapi3filter expects decoded values, which causes validation of constraints like maxLength and pattern to fail on encoded input (e.g. %2B is 3 chars but decodes to 1 char). Unescape path parameters after route matching and before passing them to openapi3filter. Fixes #17 Supersedes #18 Co-authored-by: Florent CHAUVEAU <florentch@pm.me> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f3f6b0c commit b5a3ece

File tree

3 files changed

+116
-0
lines changed

3 files changed

+116
-0
lines changed

oapi_validate.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"fmt"
1414
"log"
1515
"net/http"
16+
"net/url"
1617
"os"
1718
"strings"
1819

@@ -158,6 +159,15 @@ func ValidateRequestFromContext(ctx echo.Context, router routers.Router, options
158159
}
159160
}
160161

162+
// gorillamux uses UseEncodedPath(), so path parameters are returned in
163+
// their percent-encoded form. Unescape them before passing to
164+
// openapi3filter, which expects decoded values.
165+
for k, v := range pathParams {
166+
if unescaped, err := url.PathUnescape(v); err == nil {
167+
pathParams[k] = unescaped
168+
}
169+
}
170+
161171
validationInput := &openapi3filter.RequestValidationInput{
162172
Request: req,
163173
PathParams: pathParams,

oapi_validate_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,86 @@ func TestOapiRequestValidatorWithPrefix(t *testing.T) {
485485
}
486486
}
487487

488+
func TestEncodedPathParams(t *testing.T) {
489+
spec, err := openapi3.NewLoader().LoadFromData(testSchema)
490+
require.NoError(t, err, "Error initializing OpenAPI spec")
491+
492+
e := echo.New()
493+
494+
options := Options{
495+
SilenceServersWarning: true,
496+
}
497+
498+
e.Use(OapiRequestValidatorWithOptions(spec, &options))
499+
500+
called := false
501+
502+
// Register handlers for parameterized paths. These must be registered
503+
// before any requests are made so echo allocates enough param slots.
504+
e.GET("/resource/maxlength/:param", func(c echo.Context) error {
505+
called = true
506+
return c.NoContent(http.StatusNoContent)
507+
})
508+
e.GET("/resource/pattern/:param", func(c echo.Context) error {
509+
called = true
510+
return c.NoContent(http.StatusNoContent)
511+
})
512+
513+
// Encoded "+" (%2B) — 3 chars encoded, but 1 char decoded.
514+
// maxLength: 1 should pass because validation uses the decoded value.
515+
{
516+
rec := doGet(t, e, "http://test.hostname/resource/maxlength/%2B")
517+
assert.Equal(t, http.StatusNoContent, rec.Code)
518+
assert.True(t, called, "Handler should have been called")
519+
called = false
520+
}
521+
522+
// Unencoded "+" in the same path — should also pass.
523+
{
524+
rec := doGet(t, e, "http://test.hostname/resource/maxlength/+")
525+
assert.Equal(t, http.StatusNoContent, rec.Code)
526+
assert.True(t, called, "Handler should have been called")
527+
called = false
528+
}
529+
530+
// Two-char unencoded value exceeds maxLength: 1 — should be rejected.
531+
{
532+
rec := doGet(t, e, "http://test.hostname/resource/maxlength/ab")
533+
assert.Equal(t, http.StatusBadRequest, rec.Code)
534+
assert.False(t, called, "Handler should not have been called")
535+
}
536+
537+
// Encoded value that decodes to two chars (%2B%2B -> "++") — should be rejected.
538+
{
539+
rec := doGet(t, e, "http://test.hostname/resource/maxlength/%2B%2B")
540+
assert.Equal(t, http.StatusBadRequest, rec.Code)
541+
assert.False(t, called, "Handler should not have been called")
542+
}
543+
544+
// Pattern: ^\+[0-9]+$ — encoded "+1234" as "%2B1234" should pass.
545+
{
546+
rec := doGet(t, e, "http://test.hostname/resource/pattern/%2B1234")
547+
assert.Equal(t, http.StatusNoContent, rec.Code)
548+
assert.True(t, called, "Handler should have been called")
549+
called = false
550+
}
551+
552+
// Unencoded "+1234" should also pass.
553+
{
554+
rec := doGet(t, e, "http://test.hostname/resource/pattern/+1234")
555+
assert.Equal(t, http.StatusNoContent, rec.Code)
556+
assert.True(t, called, "Handler should have been called")
557+
called = false
558+
}
559+
560+
// Value that doesn't match pattern — should be rejected.
561+
{
562+
rec := doGet(t, e, "http://test.hostname/resource/pattern/nope")
563+
assert.Equal(t, http.StatusBadRequest, rec.Code)
564+
assert.False(t, called, "Handler should not have been called")
565+
}
566+
}
567+
488568
func TestGetSkipperFromOptions(t *testing.T) {
489569

490570
options := new(Options)

test_spec.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,32 @@ paths:
3939
properties:
4040
name:
4141
type: string
42+
/resource/maxlength/{param}:
43+
get:
44+
operationId: getMaxLengthResourceParameter
45+
parameters:
46+
- name: param
47+
in: path
48+
required: true
49+
schema:
50+
type: string
51+
maxLength: 1
52+
responses:
53+
'204':
54+
description: success
55+
/resource/pattern/{param}:
56+
get:
57+
operationId: getPatternResourceParameter
58+
parameters:
59+
- name: param
60+
in: path
61+
required: true
62+
schema:
63+
type: string
64+
pattern: '^\+[0-9]+$'
65+
responses:
66+
'204':
67+
description: success
4268
/protected_resource:
4369
get:
4470
operationId: getProtectedResource

0 commit comments

Comments
 (0)