Skip to content

fix: Path parameters are not validated properly when they are encoded #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions oapi_validate.go
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import (
"fmt"
"log"
"net/http"
"net/url"
"os"
"strings"

@@ -117,6 +118,10 @@ func ValidateRequestFromContext(ctx echo.Context, router routers.Router, options
if err != nil {
switch e := err.(type) {
case *routers.RouteError:
// The path may exist on the server, but the method is not allowed.
if errors.Is(e, routers.ErrMethodNotAllowed) {
return echo.NewHTTPError(http.StatusMethodNotAllowed, e.Reason)
}
// We've got a bad request, the path requested doesn't match
// either server, or path, or something.
return echo.NewHTTPError(http.StatusNotFound, e.Reason)
@@ -128,6 +133,14 @@ func ValidateRequestFromContext(ctx echo.Context, router routers.Router, options
}
}

// because gorillamux.NewRouter() uses mux.NewRouter().UseEncodedPath()
// we need to unescape the path parameters for openapi3filter
for k, v := range pathParams {
if v, err := url.PathUnescape(v); err == nil {
pathParams[k] = v
}
}

validationInput := &openapi3filter.RequestValidationInput{
Request: req,
PathParams: pathParams,
49 changes: 49 additions & 0 deletions oapi_validate_test.go
Original file line number Diff line number Diff line change
@@ -129,6 +129,18 @@ func TestOapiRequestValidator(t *testing.T) {
called = true
return nil
})
// add a Handler for an encoded path parameter
// this needs to be installed before calling the first doGet
// because of echo internals (maxParam)
e.GET("/resource/maxlength/:encoded", func(c echo.Context) error {
called = true
return c.NoContent(http.StatusNoContent)
})
e.GET("/resource/pattern/:encoded", func(c echo.Context) error {
called = true
return c.NoContent(http.StatusNoContent)
})

// Let's send the request to the wrong server, this should return 404
{
rec := doGet(t, e, "http://not.deepmap.ai/resource")
@@ -231,6 +243,43 @@ func TestOapiRequestValidator(t *testing.T) {
assert.False(t, called, "Handler should not have been called")
called = false
}

// Let's send a request with an encoded parameter
// It should pass validation even though the parameter is encoded
// to 3 chars and the parameter is limited to maxLength: 1
{
rec := doGet(t, e, "http://deepmap.ai/resource/maxlength/%2B")
assert.Equal(t, http.StatusNoContent, rec.Code)
assert.True(t, called, "Handler should have been called")
called = false
}

// Let's send a request with an unencoded parameter
// It should pass as well
{
rec := doGet(t, e, "http://deepmap.ai/resource/maxlength/+")
assert.Equal(t, http.StatusNoContent, rec.Code)
assert.True(t, called, "Handler should have been called")
called = false
}

// Let's send a request with an encoded parameter
// It should pass validation
{
rec := doGet(t, e, "http://deepmap.ai/resource/pattern/%2B1234")
assert.Equal(t, http.StatusNoContent, rec.Code)
assert.True(t, called, "Handler should have been called")
called = false
}

// Let's send a request with an unencoded parameter
// It should pass as well
{
rec := doGet(t, e, "http://deepmap.ai/resource/pattern/+1234")
assert.Equal(t, http.StatusNoContent, rec.Code)
assert.True(t, called, "Handler should have been called")
called = false
}
}

func TestOapiRequestValidatorWithOptionsMultiError(t *testing.T) {
38 changes: 32 additions & 6 deletions test_spec.yaml
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ paths:
minimum: 10
maximum: 100
responses:
'200':
"200":
description: success
content:
application/json:
@@ -29,7 +29,7 @@ paths:
post:
operationId: createResource
responses:
'204':
"204":
description: No content
requestBody:
required: true
@@ -39,14 +39,40 @@ paths:
properties:
name:
type: string
/resource/maxlength/{param}:
get:
operationId: getMaxLengthResourceParameter
parameters:
- name: param
in: path
required: true
schema:
type: string
maxLength: 1
responses:
"204":
description: success
/resource/pattern/{param}:
get:
operationId: getPatternResourceParameter
parameters:
- name: param
in: path
required: true
schema:
type: string
pattern: '^\+[1-9]+$'
responses:
"204":
description: success
/protected_resource:
get:
operationId: getProtectedResource
security:
- BearerAuth:
- someScope
responses:
'204':
"204":
description: no content
/protected_resource2:
get:
@@ -55,7 +81,7 @@ paths:
- BearerAuth:
- otherScope
responses:
'204':
"204":
description: no content
/protected_resource_401:
get:
@@ -64,7 +90,7 @@ paths:
- BearerAuth:
- unauthorized
responses:
'401':
"401":
description: no content
/multiparamresource:
get:
@@ -85,7 +111,7 @@ paths:
minimum: 10
maximum: 100
responses:
'200':
"200":
description: success
content:
application/json: