buaazp / fasthttprouter

A high performance fasthttp request router that scales well
http://godoc.org/github.com/buaazp/fasthttprouter
BSD 3-Clause "New" or "Revised" License
872 stars 91 forks source link

Fix query string on trailing slash redirect #30

Closed bbrodriges closed 7 years ago

bbrodriges commented 7 years ago

This commit fixes query string lost on trailing slash redirect.

Example: handler - router.POST("/v1/info/", myHandler) request - mysite.com/v1/info?id=1 expected - HTTP 301 /v1/info/?id=1 got - HTTP 301 /v1/info/

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.708% when pulling b931c21fc5856860ef88ef081c5db4f20bde25d0 on bbrodriges:master into ade4e2031af3aed7fffd241084aad80a58faf421 on buaazp:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.708% when pulling b931c21fc5856860ef88ef081c5db4f20bde25d0 on bbrodriges:master into ade4e2031af3aed7fffd241084aad80a58faf421 on buaazp:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.708% when pulling b931c21fc5856860ef88ef081c5db4f20bde25d0 on bbrodriges:master into ade4e2031af3aed7fffd241084aad80a58faf421 on buaazp:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 99.708% when pulling b931c21fc5856860ef88ef081c5db4f20bde25d0 on bbrodriges:master into ade4e2031af3aed7fffd241084aad80a58faf421 on buaazp:master.

buaazp commented 7 years ago

Hi @bbrodriges , late for reply, can you show me the router codes so I can reproduce this issue?

solarfly73 commented 7 years ago

TestPathClean fails when I add these new unit tests:

    // Params
    {"abc/?foo=bar", "/abc/?foo=bar"},
    {"abc?foo=bar", "/abc?foo=bar"},
    {"abc//?foo=bar", "/abc/?foo=bar"},
    {"abc//./?foo=bar", "/abc/?foo=bar"},
    {"/abc/.?foo", "/abc/?foo"},

If you look in path_test.go there's a test {"/abc/.", "/abc/"} which passes, but when I add params where the URI ends with a period, the Clean function doesn't strip the dot and keep the params. Thoughts? Maybe my test cases are incorrect if the params are being stripped off somewhere else before CleanPath would ever be called, but I don't see a unit test anywhere for parameter handling.

paduvi commented 7 years ago

why this still not merged?

Logger.Debugln(string(ctx.QueryArgs().QueryString()))

"/recommend?itemid=20170914141825212&alg=1" -> "itemid=20170914141825212&alg=1" "/recommend/?itemid=20170914141825212&alg=1" -> ""

buaazp commented 7 years ago

Merged. Busy with work these days.