Hi, On Sun, Apr 7, 2019 at 10:09 PM Andreas Henriksson <andr...@fatal.se> wrote: > > Greetings from the Gothenburg BSP. > > This is the output I get when reproducing this issue: > > ---8<----->8----- > > --- FAIL: TestEncodeNecessaryEscapesAll (0.00s) > purell_test.go:761: Got error parse http://host/ > > > � net/url: invalid control character in URL > FAIL > exit status 1 > FAIL _/tmp/golang-github-puerkitobio-purell-1.1.0 0.003s > > ---8<----->8----- > > > net/url Parse now explicitly check that you don't pass in > "invalid" (non-encoded) urls and rejects them: > https://sources.debian.org/src/golang-1.11/1.11.6-1/src/net/url/url.go/?hl=498#L498 > > The NormalizeUrlString helper function starts out by passing the url > string to url.Parse: > https://sources.debian.org/src/golang-github-puerkitobio-purell/1.1.0-1/purell.go/#L153 > > The TestDecodeUnnecessaryEscapesAll function creates an url with control > characters and passes it to NormalizeURLString to try to get it > normalized/escaped. > > I'd say the design of the NormalizeUrlString function is broken and thus > it's not obvious to me how to fix it. I'd say that if you have a > non-normalized string you want encoded, you should pass it in as > something that doesn't need to be parsed. ie. use NormalizeUrl helper > function, which takes an Url type. > > Removing the NormalizeUrlString function however would be an API/ABI > break as it's publicly exported (and used by Hugo and Kubernetes)... > > Maybe open-coding some homebrew url parsing to fall back on could be > done to keep the function around..... Sounds bad to think you know > better than net/url how to parse an url though. > > I'm attaching a patch which "fixes" this issue, but really it's most likely > a stupid and wrong things to do to solve this. It's just designed to > make the testsuite pass. As mentioned, I think the NormalizeURLString > function is incorrectly designed and there's no way to fix it (so should > be deprecated in favour of NormalizeURL). > Thus intentionally *not* tagging patch, as this is rather a "proof of > concept". > > Regards, > Andreas Henriksson
Thanks for your great work! However it's already fixed by upstream, https://github.com/PuerkitoBio/purell/pull/29 If someone is going to upload the new version, please take upstream patch instead :/ -- Shengjing Zhu
