code review 5975050: net/url: parse url with @ symbol in username/password c...

net/url: parse url with @ symbol in username/password correctly.
Fixes issue 3439

[smwca, 2012-03-31 02:53:15]

This fixes http://code.google.com/p/go/issues/detail?id=3439

I added two tests which would fail if the changes are not implemented.

[snaury, 2012-03-31 09:38:57]

Also, please add "Fixes issue 3439." as the last line of commit description and
then please use "hg mail" to request a review.

http://codereview.appspot.com/5975050/diff/3003/src/pkg/net/url/url.go
File src/pkg/net/url/url.go (right):

http://codereview.appspot.com/5975050/diff/3003/src/pkg/net/url/url.go#newcod...
src/pkg/net/url/url.go:326: for i := 0; i < len(s); i++ {
Why do you iterate forward, instead of going backwards, from len(s) -1 to 0?

[smwca, 2012-03-31 17:04:50]

Hello snaury@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2012-04-02 16:07:32]

Thanks.  Please add a test or two with more than just http://user@host
(add /path...)

[snaury, 2012-05-17 21:13:54]

On 2012/04/02 16:07:32, rsc wrote:
> Thanks.  Please add a test or two with more than just http://user%40host
> (add /path...)

Hi, what is going on with this bug? It's Go 1.0.1 already, bug the patch is
still in limbo. If the original submitter abandoned this patch, should I submit
a new one, or is it possible to take over this one?

[rsc, 2012-05-21 15:36:52]

I'm sorry this didn't make it into Go 1.0.1.  This CL still needs tests.

Russ

[snaury, 2012-05-21 16:23:59]

> I'm sorry this didn't make it into Go 1.0.1.  This CL still needs tests.

It's ok, I didn't really hope for 1.0.1, would be cool, but ok at least
eventually (1.1 or 2.0). But since I'm not CL author I cannot add more
tests, so I created CL 6206090 (different patch, a lot simpler).

As for more tests, I have two with /p@th, just to show it doesn't split
there. Anything else is excessive in my opinion, since it only affects
parsing of authority, and other cases are already tested by other tests.
Would be happy to add more, though, if you feel strongly about it.

[remyoudompheng, 2013-07-20 15:52:24]

R=close

See also https://codereview.appspot.com/6206090