valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.66k stars 1.75k forks source link

Query string values imporperly handled (URI encoding forced for = character) #1611

Closed misiek08 closed 1 year ago

misiek08 commented 1 year ago

Hello! Today I encountered interesting problem. I've got = sign in query string value. I know I could trim the character from some base64 strings, but remote proxy (not managed by me) requires them. I try to proxy those args and fasthttp is treating values as args.

Smallest test case I could produce for fasthttp:

package main

import (
    "testing"

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

func TestFastHTTPURLEncoding(t *testing.T) {
    input := "/some/file?arg1=value1=&arg2=value2"
    uri := fasthttp.AcquireURI()
    defer fasthttp.ReleaseURI(uri)

    err := uri.Parse(nil, []byte(input))
    assert.NoError(t, err)

    assert.Equal(t, input, string(uri.RequestURI()))
}

func TestFastHTTPURLEncodingBUG(t *testing.T) {
    input := "/some/file?arg1=value1=&arg2=value2"
    uri := fasthttp.AcquireURI()
    defer fasthttp.ReleaseURI(uri)

    err := uri.Parse(nil, []byte(input))
    assert.NoError(t, err)

    assert.NotNil(t, uri.QueryArgs()) // without this line it works good, u.parseQueryArgs called here messing with values

    assert.Equal(t, input, string(uri.RequestURI()))
}

func TestGoURLParsing(t *testing.T) {
    input := "/some/file?arg1=value1=&arg2=value2"

    parsedURL, err := url.Parse(input)
    assert.NoError(t, err)

    assert.Equal(t, input, parsedURL.String())
}

Result:

=== RUN   TestFastHTTPURLEncodingBUG
    fasthttp_bug_test.go:32:
                Error Trace:    fasthttp_bug_test.go:32
                Error:          Not equal:
                                expected: "/some/file?arg1=value1=&arg2=value2"
                                actual  : "/some/file?arg1=value1%3D&arg2=value2"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -/some/file?arg1=value1=&arg2=value2
                                +/some/file?arg1=value1%3D&arg2=value2
                Test:           TestFastHTTPURLEncodingBUG
--- FAIL: TestFastHTTPURLEncodingBUG (0.00s)
FAIL

TestFastHTTPURLEncoding - correct behavior TestFastHTTPURLEncodingBUG - incorrect behavior.

Same test for net/url called TestGoURLParsing passing.

To keep "clean" issue format:

Expected

At least = character is not being encoded in query args values.

Actual behavior

Query args values are being encoded and at least = is being replaced with %3D.

erikdubbelboer commented 1 year ago

I'm afraid that this is intended behavior. When you call uri.QueryArgs() it returns an object you can use to modify the query params. When you then call uri.RequestURI() it re-builds the query params and re-encodes all the values. When re-encoding the values it changes = in the value to %3D as is expected (net/url also does this).

misiek08 commented 1 year ago

Wow, I tried adding params and URL.Encode() is encoding them too. Thanks and I'm sorry I didn't go through net/url deep enough. So we will need to force workaround on backend for this :(

Thanks!