BindStyledParameterWithOptions double-decodes path parameters

[reinkrul]

When a path parameter contains percent-encoded characters (in the original value), BindStyledParameterWithOptions causes the original percent characters to be decoded as if they were URL path encoded. This either causes the original value to be lost or a HTTP error 400 in case the characer can't be decoded. E.g., if a path parameter were a (fictional) discount ("/checkout/" + url.PathEscape("15%off")), a HTTP error 400 occurs. Or url.PathEscape("discount%20") would be received in the server interface as "discount " (decoded to a space).

This is caused by BindStyledParameterWithOptions which uses the path parameters from Echo (didn't test other HTTP servers) to unescape the parameters, which Echo already did. It does not occur with query parameters.

Example

OpenAPI spec:

openapi: "3.0.1"
paths:
 /test/{param1}:
 parameters:
 - name: param1
 in: path
 required: true
 schema:
 type: string
 get:
 operationId: test
 responses:
 204:
 description: good

Unit test:

package issueXX_test

import (
	"context"
	issueXX "github.com/deepmap/oapi-codegen/v2/internal/test/issues/issue-XX"
	"github.com/labstack/echo/v4"
	"github.com/stretchr/testify/require"
	"net/http"
	"net/http/httptest"
	"net/url"
	"sync/atomic"
	"testing"

	"github.com/stretchr/testify/assert"
)

type serverImplementation struct {
	param1 atomic.Value
}

var _ issueXX.StrictServerInterface = &serverImplementation{}

func (s *serverImplementation) Test(_ context.Context, request issueXX.TestRequestObject) (issueXX.TestResponseObject, error) {
	s.param1.Store(request.Param1)
	return issueXX.Test204Response{}, nil
}

func TestIssueXX(t *testing.T) {
	impl := &serverImplementation{}
	e := echo.New()
	issueXX.RegisterHandlers(e, issueXX.NewStrictHandler(impl, nil))
	svr := httptest.NewServer(e)
	defer svr.Close()

	const expected = "discount%20"
	httpResponse, err := http.Get(svr.URL + "/test/" + url.PathEscape(expected))
	require.NoError(t, err)
	require.Equal(t, http.StatusNoContent, httpResponse.StatusCode)
	assert.Equal(t, expected, impl.param1.Load(), "path unescape incorrect")
}

Test output:

 	Error: 	Not equal: 
 	 	expected: "discount%20"
 	 	actual : "discount "

Solution

BindStyledParameterWithOptions should not unescape the value (at least for Echo).

[roman-khimov]

Seems to be the same for std-http.

[reinkrul]

Any news on this? Is this something you consider a bug as well? I'd be happy to fix it, if we can align on it.

[josepereiradash0]

We're experiencing the same issue with Gin.

Path parameters containing literal % characters such as test%20with%20%25%20in%20name are being double-decoded, once by Gin's router via c.Param(), and again by runtime.BindStyledParameterWithOptions, causing the error.

error unescaping query parameter 'xxx': invalid URL escape "% i"

The client is correctly encoding the URL, but the generated code attempts to decode an already-decoded value. This makes it impossible to support path parameters with percent signs in their decoded form.

[mromaszewicz]

This is kind of tricky, because we use a single BindStyledParameterWithOptions implementations for all the routers, and some escape it for you automatically (echo), leading to a double escape, and others like gin actually let you configure it. I will add another option to this function, which flags that a value needs escaping, and only conditionally escape it.

[mromaszewicz]

You're going to have to wait for oapi-codegen v2.8.0 for this to work end-to-end. I'm going to fix oapi-codegen/oapi-codegen#2455.

v2.8.0 is 1-2 weeks away, and I'll have this fix committed in the next day or two, so you can pin to our latest commit to try it out once it's done.