chi icon indicating copy to clipboard operation
chi copied to clipboard

Inconsistent unescaping of URL parameters

Open klingtnet opened this issue 4 years ago • 1 comments

I discovered some inconsistent unescaping of URL path parameters depending on the content of the parameter value:

  • they are url.PathUnescaped if no / is contained
  • otherwise the parameter value is still path escaped

This was unexpected for me since I could not find any documentation about how chi is handling the unescaping of URL parameters. I included a test case that demonstrates this issue for chi@master but the issue is also present for the latest release version. Here's the test output:

$ go test .
--- FAIL: TestURLParam (0.00s)
    --- FAIL: TestURLParam/escaped_urlparam_containing_a_slash (0.00s)
        go-chi-urlparam_test.go:27:
                Error Trace:    go-chi-urlparam_test.go:27
                                                        server.go:2042
                                                        mux.go:437
                                                        server.go:2042
                                                        mux.go:86
                                                        go-chi-urlparam_test.go:29
                Error:          Not equal:
                                expected: "'/"
                                actual  : "%27%2F"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -'/
                                +%27%2F
                Test:           TestURLParam/escaped_urlparam_containing_a_slash
                Messages:       URLParam
FAIL
FAIL    chi-urlparam-test       0.003s
FAIL

You can find the test files in this zip archive: go-chi-urlparam-test.zip

For convenience this is the content of unit test file:

package main

import (
	"net/http"
	"testing"

	"github.com/go-chi/chi"
	"github.com/stretchr/testify/assert"
)

func TestURLParam(t *testing.T) {
	tCases := []struct {
		name,
		u,
		expected string
	}{
		{"plain urlparam", "http://doesnot.matter/whatever", "whatever"},
		{"escaped urlparam without slash", "http://doesnot.matter/%27", "'"},
		{"escaped urlparam containing a slash", "http://doesnot.matter/%27%2F", "'/"},
	}

	for _, tCase := range tCases {
		t.Run(tCase.name, func(t *testing.T) {
			r := chi.NewRouter()
			r.Get("/{value}", func(w http.ResponseWriter, r *http.Request) {
				assert.Equal(t, tCase.expected, chi.URLParam(r, "value"), "URLParam")
			})
			assert.HTTPSuccess(t, r.ServeHTTP, "GET", tCase.u, nil)
		})
	}
}

klingtnet avatar Dec 29 '20 13:12 klingtnet

I also noticed that chi seems to incorrectly decode / in path parameter (e.g. foo%2Fbar) as actual slashes and tries incorrectly route them, generally leading to a No route found error.

silverwind avatar Aug 02 '22 10:08 silverwind