http: add QueryUnescape in HTTPPool.ServeHTTP #91
Conversation
|
This is missing a bug (or description of the problem in the commit message), and is missing a test. And does this change break behavior of existing clients? Can this be incrementally rolled out safely? I think if we're changing the "protocol" like this, we should make it explicit and put something in the URL from the client to indicate that the peer "server" should unescape like this, whether that something is a new path component or header or query parameter. |
|
I find this change breaks behavior of existing clients. For example, a key "+myKey+" sent in raw format will be converted to " myKey ". How about adding a query parameter |
|
Sounds good. Or |
lorneli
force-pushed
the
fix-servehttp
branch
from
0f3cb5d to
7b7eb1d
Compare
|
I add |
http.go
Outdated
| @@ -138,19 +138,45 @@ func (p *HTTPPool) PickPeer(key string) (ProtoGetter, bool) { | |||
| return nil, false | |||
| } | |||
|
|
|||
| func (p *HTTPPool) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| // Parse request. | |||
| func (p *HTTPPool) parseRequest(r *http.Request) (string, string, bool) { | |||
There was a problem hiding this comment.
Use named result parameters here, otherwise (string, string, bool) is super confusing, especially without docs.
There was a problem hiding this comment.
@bradfitz Modified. Thanks for your review.
HTTPPool will inverse escaped path if query parameter contains "escaped=true". Add keys with char to be escaped in test.
lorneli
force-pushed
the
fix-servehttp
branch
from
7b7eb1d to
e652685
Compare
Use url.QueryUnescape to inverse escaped group and key in path.