go-chi / chi

lightweight, idiomatic and composable router for building Go HTTP services
https://go-chi.io
MIT License
18.5k stars 986 forks source link

Inconsistent unescaping of URL parameters #570

Open klingtnet opened 3 years ago

klingtnet commented 3 years ago

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

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)
        })
    }
}
silverwind commented 2 years ago

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.