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/url: parseHost doesn't valid ports or contents after IPv6 literals #11208

Closed
dvyukov opened this issue Jun 13, 2015 · 5 comments
Closed

net/url: parseHost doesn't valid ports or contents after IPv6 literals #11208

dvyukov opened this issue Jun 13, 2015 · 5 comments
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Jun 13, 2015

URL parsing allows %-ending after [], which allows weird things like:

package main

import (
    "bufio"
    "bytes"
    "net/http"
    "os"
)

func main() {
    data := []byte("CONNECT []%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a HTTP/1.0\n\n")
    r, err := http.ReadRequest(bufio.NewReader(bytes.NewReader(data)))
    if err != nil {
        panic(err)
    }
    r.WriteProxy(os.Stdout)
}
CONNECT [] HTTP/1.1
MyHeader: 123

 HTTP/1.1
Host: [] HTTP/1.1
MyHeader: 123


User-Agent: Go 1.1

%-ending must be allowed only inside of []

go version devel +a1fe3b5 Sat Jun 13 04:33:26 2015 +0000 linux/amd64

@ianlancetaylor ianlancetaylor added this to the Go1.5Maybe milestone Jun 14, 2015
@bradfitz
Copy link
Contributor

What is special about []? You can have %xx in GET /foo?req=%41 too.

Or is the bug here that we write out bogus newlines in Request.Write? That looks wrong.

@dvyukov
Copy link
Member Author

dvyukov commented Jun 24, 2015

Sorry, this is about host. % is not otherwise allowed in host, but after [] it is.

@bradfitz bradfitz changed the title net/http: allows %-encoding after [] net/url: parseHost doesn't valid ports or contents after IPv6 literals Jun 24, 2015
@bradfitz
Copy link
Contributor

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/11414 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/13253 mentions this issue.

rsc added a commit that referenced this issue Aug 6, 2015
Go 1.4 and earlier accepted mysql://x@y(z:123)/foo
and I don't see any compelling reason to break that.

The CL during Go 1.5 that broke this syntax was
trying to fix #11208 and was probably too aggressive.
I added a test case for #11208 to make sure that stays
fixed.

Relaxing the check did not re-break #11208 nor did
it cause any existing test to fail. I added a test for the
mysql://x@y(z:123)/foo syntax being preserved.

Fixes #12023.

Change-Id: I659d39f18c85111697732ad24b757169d69284fc
Reviewed-on: https://go-review.googlesource.com/13253
Reviewed-by: Andrew Gerrand <[email protected]>
Reviewed-by: Mikio Hara <[email protected]>
@golang golang locked and limited conversation to collaborators Aug 5, 2016
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

5 participants