Repository navigation
net/url: Parse+String does not round trip magnet URLs #20054
Description
I received a private email with a report that Go inserts // in magnet URLs.
That appears to mean https://en.wikipedia.org/wiki/Magnet_URI_scheme.
Round-tripping the example from that wikipedia page:
package main
import (
"fmt"
"net/url"
)
func main() {
fmt.Println(url.Parse(`magnet:?xt=urn:btih:c12fe1c06bba254a9dc9f519b335aa7c1367a88a&dn`))
}
prints
magnet://?xt=urn:btih:c12fe1c06bba254a9dc9f519b335aa7c1367a88a&dn <nil>
https://play.golang.org/p/OQKMvDyTD-
It does seem like the leading // should not be there. However, I don't know how much this matters, nor how invasive this would be to fix, nor whether the // insertion is correct for any other schemes (in particular, http and https).
/cc @bradfitz
Activity
bradfitz commentedon Apr 20, 2017
We don't special case
mailto:so it'd be nice to not special casemagnet:.But perhaps we could tweak our heuristics for
URL.String's Opaque-vs-://forms.rsc commentedon Apr 20, 2017
Hmm, so
mailto:rsc?subject=hiround-trips butmailto:?subject=hiturns intomailto://?subject=hi.dsnet commentedon Apr 20, 2017
Part of the problem is that we parse the Opaque field out as the empty string. When we try to round trip, we notice that the Opaque field is empty and assume it's a hierarchical URL, which it is not. Perhaps, we should add a
ForceOpaque boolfield?rsc commentedon Apr 20, 2017
I would rather not add a new bool.
If
u.User == nil && u.Host=="" && u.Path=="", can we print that part as an empty string instead of as//?johnnyluo commentedon Jul 15, 2017
I am interested to work on this one, can someone assign it to me? so other people won't waste time on this
gopherbot commentedon Jul 16, 2017
CL https://golang.org/cl/49050 mentions this issue.
odeke-em commentedon Jul 19, 2017
Moving this to Go1.10 as per the CL update and given how deep we are in the freeze Go1.9.
johnnyluo commentedon Aug 29, 2017
@dsnet @odeke-em could you guys please help to run trybot on CL https://golang.org/cl/49050, the previous trybot failed , however it is nothing to do with my change.
odeke-em commentedon Aug 29, 2017
Thanks for the ping @johnnyluo, I've ran the trybot.