go-chi / go-chi/chi

Inconsistent unescaping of URL parameters

Open
#570 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
Go
Stars
22.9k
Forks
1.4k
Avg merge
5h 17m
Merged PRs (30d)
10

Description

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

- they are `url.PathUnescape`d 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:

```sh
$ 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](https://github.com/go-chi/chi/files/5750881/go-chi-urlparam-test.zip)

For convenience this is the content of unit test file:

```go
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)
})
}
}
```

Contributor guide

Open the contributing guide

Research direction

Start by running the provided TestURLParam case with go test ., using go-chi-urlparam_test.go and its /{value} route as the reproduction. Trace URLParam handling from the router entry point to determine why values containing an encoded slash differ, then add or update regression coverage so the expected parameter value is consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.