RawPathRouting

[imkira]

Thanks in advance for your awesome router.

golang's http package does (by default) percent-decode the URI path in requests, which cases problems during routing if the requested path contains %2f in named/catch-all parameters.
For instance, if I define the route GET /dirs/:dir/files/:file, and make a request like GET /dirs/dir1%2fdir2/files/file instead of having that route triggered, the NotFound handler will be executed because httprouter uses req.URL.Path which happens to expand to /dirs/dir1/dir2/files/file.

The following thread mentions the importance of this detail.
https://code.google.com/p/go/issues/detail?id=2782

This patch adds the option RawPathRouting to Router. It is disabled by default because it is rarely necessary and because it forces the user to decode parameters manually if enabled. There are already other routers doing this, namely httptreemux

Would it be possible to merge this into your repo?

[julienschmidt]

Why did you change everything to newRequest instead of http.NewRequest, even for unaffected tests?

[imkira]

The reason I did this for the new tests is because parsing of http requests is internally handled differently in net/http when requests come from a server.

https://github.com/golang/go/blob/master/src/net/http/server.go#L613
https://github.com/golang/go/blob/master/src/net/http/request.go#L592

When I looked at net/http's code, I noticed that public function ReadRequest builds a url.Request structure from an io.Reader, parsing the request as-is (in raw form) and the request is never altered until being delivered to the http function handler. Making a request using http.NewRequest does not handle requests in raw form so one would also need to call url.ParseRequestURI and fix other fields to be coherent. I thought that would be difficult to maintain and to keep up with golang updates. The only external requirement now is using http.ReadRequest so it shouldn't be difficult to maintain.
The other alternative to all of this would be to fully mock http listeners, transports, etc so that the request would go through all steps of the http stack from client to server and then from server to http handler. I didn't do this because that would be a lot more work and because it would better fit in a PR for that purpose and not this one.

The reason I also changed all old tests to use newRequest is to make all tests homogeneous and to be able to specify requests in raw form (not parsed).

[samsalisbury]

Any reason this hasn't been merged yet @julienschmidt? Has this functionality been included in some other way?