julienschmidt / httprouter

A high performance HTTP request router that scales well
https://pkg.go.dev/github.com/julienschmidt/httprouter
BSD 3-Clause "New" or "Revised" License
16.64k stars 1.47k forks source link

bug: should not redirect if target location not exists #311

Open jxskiss opened 4 years ago

jxskiss commented 4 years ago

I was running a python service and a go service before it, the go service handles some endpoints and proxy requests which not found to the python service. And I ran into a situation which redirect to death.

The following code will reproduce the bug when accessing "http://127.0.0.1:8080/users/profile/".

package main

import (
    "github.com/julienschmidt/httprouter"
    "net/http"
)

func main() {
    r := httprouter.New()
    r.GET("/users/:user_id/somedata/", dummyHandler)
    r.NotFound = proxyHandler()

    http.ListenAndServe(":8080", r)
}

// mimics the backend python service
func proxyHandler() http.Handler {
    r := httprouter.New()
    r.GET("/users/profile/", dummyHandler)
    return r
}

func dummyHandler(_ http.ResponseWriter, _ *http.Request, _ httprouter.Params) {}
jxskiss commented 4 years ago

It redirects between "/users/profile/" and "/users/profile" cyclically, which is not expected. Since neither "/users/profile/" and "/users/profile" is registered, I think it should not redirect "/users/profile/" to "/users/profile", but should go to NotFound instead.

jxskiss commented 4 years ago

@julienschmidt Hi, sorry to bother, would you please look into this?

duskwuff commented 4 years ago

Here's a reduced test case:

package main

import (
    "github.com/julienschmidt/httprouter"
    "net/http"
)

func main() {
    r := &httprouter.Router{}
    r.RedirectTrailingSlash = true
    r.GET("/a/:id/b/", dummyHandler)
    http.ListenAndServe("localhost:9000", r)
}

func dummyHandler(_ http.ResponseWriter, _ *http.Request, _ httprouter.Params) {}

So long as RedirectTrailingSlash is enabled, a request to GET /a/x/ will be redirected to /a/x, despite there being no handler registered for that path. (This is specific to requests under /a/; requests under other paths will not be redirected similarly.)