Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: fix, test, document requesting and serving %2F in paths #4860

Closed
bradfitz opened this issue Feb 20, 2013 · 5 comments
Closed

net/http: fix, test, document requesting and serving %2F in paths #4860

bradfitz opened this issue Feb 20, 2013 · 5 comments
Milestone

Comments

@bradfitz
Copy link
Contributor

$ cat slash.go
package main

import (
        "log"
        "net/http"
        "net/http/httptest"
)

func main() {
        ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                log.Printf("Got request for %q", r.RequestURI)
        }))
        defer ts.Close()
        req, _ := http.NewRequest("GET", ts.URL, nil)

        // Try setting Opaque to just a path.

        req.URL.Opaque = "/%2F/%2F/%2F/"
        log.Printf("1. Request URL String = %q", req.URL)
        _, err := http.DefaultClient.Do(req)
        if err != nil {
                log.Fatalf("Do: %v", err)
        }

        println("")

        // Try setting Opaque to just //host/path, so it'll String-ify
        // to a valid full URL (as required for App Engine's urlfetch)

        req.URL.Opaque = "//why-hello-there/%2F/%2F/%2F/"
        log.Printf("2. Request URL String = %q", req.URL)
        _, err = http.DefaultClient.Do(req)
        if err != nil {
                log.Fatalf("Do: %v", err)
        }
}


$ go run slash.go
1. Request URL String = "http:/%2F/%2F/%2F/"
Got request for "/%2F/%2F/%2F/"
                                                                                                                      
2. Request URL String = "http://why-hello-there/%2F/%2F/%2F/";
Got request for "//why-hello-there/%2F/%2F/%2F/"


The 1st works on for client & server with Go's *http.Transport.  (but stringifies
poorly, which means it doesn't work on App Engine).

The 2nd stringifies correctly (so it'll work with App Engine's urlfetch), but it doesn't
send the correct HTTP request with *http.Transport (it should include the scheme, if the
Opaque starts with "//")
@bradfitz
Copy link
Contributor Author

Comment 1:

Maybe we should also change (note: I'm not saying "fix") App Engine's urlfetch.go to
stringify the URL differently if Opaque is set with no Host, getting the Host from
req.Host)
Currently urlfetch.go is just:
...
       Url:                           proto.String(req.URL.String()),
...

@bradfitz
Copy link
Contributor Author

Comment 2:

Sent out https://golang.org/cl/7369045/ to update (*net.URL).RequestURI to do
the sane thing with the URL.Opaque begins with "//", in which case it generates an HTTP
request line like:
GET http://host/path HTTP/1.1
Rather than the invalid:
GET //host/path HTTP/1.1

@bradfitz
Copy link
Contributor Author

Comment 3:

This issue was updated by revision fb21bca.

R=adg, rsc, campoy
CC=golang-dev
https://golang.org/cl/7369045

@bradfitz
Copy link
Contributor Author

Comment 4:

Now implemented and tested.  Docs are in https://golang.org/cl/7375047

@bradfitz
Copy link
Contributor Author

Comment 5:

This issue was closed by revision 0462aad.

Status changed to Fixed.

@bradfitz bradfitz self-assigned this Feb 21, 2013
@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1maybe label Apr 14, 2015
rsc added a commit that referenced this issue Jun 22, 2015
Historically we have declined to try to provide real support for URLs
that contain %2F in the path, but they seem to be popping up more
often, especially in (arguably ill-considered) REST APIs that shoehorn
entire paths into individual path elements.

The obvious thing to do is to introduce a URL.RawPath field that
records the original encoding of Path and then consult it during
URL.String and URL.RequestURI. The problem with the obvious thing
is that it breaks backward compatibility: if someone parses a URL
into u, modifies u.Path, and calls u.String, they expect the result
to use the modified u.Path and not the original raw encoding.

Split the difference by treating u.RawPath as a hint: the observation
is that there are many valid encodings of u.Path. If u.RawPath is one
of them, use it. Otherwise compute the encoding of u.Path as before.

If a client does not use RawPath, the only change will be that String
selects a different valid encoding sometimes (the original passed
to Parse).

This ensures that, for example, HTTP requests use the exact
encoding passed to http.Get (or http.NewRequest, etc).

Also add new URL.EscapedPath method for access to the actual
escaped path. Clients should use EscapedPath instead of
reading RawPath directly.

All the old workarounds remain valid.

Fixes #5777.
Might help #9859.
Fixes #7356.
Fixes #8767.
Fixes #8292.
Fixes #8450.
Fixes #4860.
Fixes #10887.
Fixes #3659.
Fixes #8248.
Fixes #6658.
Reduces need for #2782.

Change-Id: I77b88f14631883a7d74b72d1cf19b0073d4f5473
Reviewed-on: https://go-review.googlesource.com/11302
Reviewed-by: Brad Fitzpatrick <[email protected]>
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants