code review 7135051: net/url: generate correct Path when hostname empty

net/url: generate correct Path when hostname empty

Parse("file:///foo") previously returned a URL with Scheme "file"
and Path "///foo". Now it returns a URL with Path "/foo",
such that
        &URL{Scheme: "file", Path: "/foo"}.String() == "file:///foo"

This means that parsing and stringifying the URL "file:/foo"
returns "file:///foo", technically a regression but one that only
affects a corner case.

Fixes issue 4189.

[adg, 2013-01-17 04:57:18]

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

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

[rsc, 2013-01-17 14:06:24]

might want to ping CL 6931047 to let him know you've started this instead.
(that one's been idle for a month so i think it's fine.)

[bradfitz, 2013-01-17 20:04:00]

https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go
File src/pkg/net/url/url_test.go (left):

https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go#old...
src/pkg/net/url/url_test.go:125: // non-authority
I'd prefer to keep this test. At least keep its input and update the comment and
expected results.

But why is this not right?  Isn't this non-authority + path-absolute?  How do
you read RFC 3986?

[adg, 2013-01-17 23:28:53]

https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go
File src/pkg/net/url/url_test.go (left):

https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go#old...
src/pkg/net/url/url_test.go:125: // non-authority
On 2013/01/17 20:04:00, bradfitz wrote:
> I'd prefer to keep this test. At least keep its input and update the comment
and
> expected results.
> 
> But why is this not right?  Isn't this non-authority + path-absolute?  How do
> you read RFC 3986?

Actually, on re-reading I think you're right.

However we're kinda stuck now. url.URL doesn't give us a way to distinguish the
absence of an authority and the absence of a host.

That is, if we represent

mailto:/foo
as
URL{Scheme: "mailto", Path: "/foo"}

then we can't also represent
file:///foo
as
URL{Scheme: "file", Path: "/foo"}

The latter case seems more common to me, so that's what I would optimize for,
but it breaks the existing behavior. Thoughts?

[bradfitz, 2013-01-18 18:58:45]

On Thu, Jan 17, 2013 at 3:28 PM, <adg@golang.org> wrote:

>
> https://codereview.appspot.**com/7135051/diff/1/src/pkg/**
>
net/url/url_test.go<https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go>
> File src/pkg/net/url/url_test.go (left):
>
> https://codereview.appspot.**com/7135051/diff/1/src/pkg/**
>
net/url/url_test.go#oldcode125<https://codereview.appspot.com/7135051/diff/1/src/pkg/net/url/url_test.go#oldcode125>
> src/pkg/net/url/url_test.go:**125: // non-authority
> On 2013/01/17 20:04:00, bradfitz wrote:
>
>> I'd prefer to keep this test. At least keep its input and update the
>>
> comment and
>
>> expected results.
>>
>
>  But why is this not right?  Isn't this non-authority + path-absolute?
>>
> How do
>
>> you read RFC 3986?
>>
>
> Actually, on re-reading I think you're right.
>
> However we're kinda stuck now. url.URL doesn't give us a way to
> distinguish the absence of an authority and the absence of a host.
>
> That is, if we represent
>
> mailto:/foo
> as
> URL{Scheme: "mailto", Path: "/foo"}
>
> then we can't also represent
> file:///foo
> as
> URL{Scheme: "file", Path: "/foo"}
>
> The latter case seems more common to me, so that's what I would optimize
> for, but it breaks the existing behavior. Thoughts?
>

Bleh.

All the options suck.  (special-casing schemes, breaking compat, adding new
URL fields, ...)

[rsc, 2013-01-18 19:11:04]

is there a semantic difference between file:///foo and file:/foo?

[adg, 2013-01-18 22:54:08]

On 19 January 2013 06:11, Russ Cox <rsc@golang.org> wrote:

> is there a semantic difference between file:///foo and file:/foo?


Not in my reading of the spec, but I would assume most programs expect
file:///foo.

As one data point, Chrome's URL bar rewrites file:/foo to file:///foo.

Perhaps we should just rewrite mailto:/foo (which I've never seen before)
to mailto:///foo ?

[rsc, 2013-01-18 22:56:47]

As long as mailto:foo stays mailto:foo I don't care if mailto:/foo turns
into mailto:///foo.

Russ

[bradfitz, 2013-01-19 01:20:26]

SGTM
 On Jan 18, 2013 2:56 PM, "Russ Cox" <rsc@golang.org> wrote:

> As long as mailto:foo stays mailto:foo I don't care if mailto:/foo turns
> into mailto:///foo.
>
> Russ
>
>

[adg, 2013-01-21 05:06:57]

Hello bradfitz@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-01-22 21:37:57]

LGTM

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url.go
File src/pkg/net/url/url.go (right):

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url.go#newc...
src/pkg/net/url/url.go:437: // TODO: Rewrite to use bytes.Buffer
I think it's time to do this.  In a following CL soon.

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url_test.go
File src/pkg/net/url/url_test.go (right):

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url_test.go...
src/pkg/net/url/url_test.go:132: "mailto:///webmaster@golang.org", // unforunate
compromise
typo in unfortunate

[adg, 2013-01-23 00:37:13]

*** Submitted as https://code.google.com/p/go/source/detail?r=c7719e0e0a2c ***

net/url: generate correct Path when hostname empty

Parse("file:///foo") previously returned a URL with Scheme "file"
and Path "///foo". Now it returns a URL with Path "/foo",
such that
        &URL{Scheme: "file", Path: "/foo"}.String() == "file:///foo"

This means that parsing and stringifying the URL "file:/foo"
returns "file:///foo", technically a regression but one that only
affects a corner case.

Fixes issue 4189.

R=bradfitz, rsc
CC=golang-dev
https://codereview.appspot.com/7135051

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url_test.go
File src/pkg/net/url/url_test.go (right):

https://codereview.appspot.com/7135051/diff/12002/src/pkg/net/url/url_test.go...
src/pkg/net/url/url_test.go:132: "mailto:///webmaster@golang.org", // unforunate
compromise
On 2013/01/22 21:37:57, bradfitz wrote:
> typo in unfortunate

Done.

[adg, 2013-01-23 00:37:37]

On 23 January 2013 08:37, <bradfitz@golang.org> wrote:

> I think it's time to do this.  In a following CL soon.


Will do.