Bug 166426 – Add WebCore::URL::protocolHostAndPort

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Keith Rollin]

In Bug 166041 comment 4, Darin suggests:

> Seems to me that this should be a function in URL or URLParser itself, not
> something in the network capture code. I don’t think the name “base URL” is
> right for this, since that is a term that means something specific in the
> context of URLs. It’s more like topLevelOfServerURL; it would be good to use
> a more precise term in the name here.

In Bug 166041 comment 5, Alex adds:

> This should definitely be in URL.h and URL.cpp. 

This bug tracks that work.

Darin seemed open to different names for the function, so I'm considering schemeHostAndPort(). This matches the hostAndPort() that's already there.

[Keith Rollin]

For my work, I don't think it matters if the username and password are in the string I return. I don't think the record/replay mechanism will be exposed to such URLs. But what about in general? Should I do what the name of the function says and just return the scheme, host, and port, or should I do what my code currently does which is return the left part of the string up to but not including the path? I think the former.

Also, Alex says:

> It looks like you just want to include the '/' after
> the host if there is one.  There will always be one if the scheme is
> something like http, but an unknown scheme without a '/' like
> "asdf://noSlash" won't have one.

Is including that trailing slash important/required/expected? It seems odd to me to special case a "scheme that is something like http". It's also not clear to me what qualifies as "something like http". Certainly http(s), but is there anything else?

[Keith Rollin]

For consistency with existing terms in the URL class, I guess it should be protocolHostAndPort rather than schemeHostAndPort.

Are there significant semantic differences between protocol and scheme?

[Alex Christensen]

scheme and protocol are basically the same thing.
special schemes are listed here https://url.spec.whatwg.org/#special-scheme

[Keith Rollin]

Created attachment 297685 [details]
Patch

[Alex Christensen]

Comment on attachment 297685 [details]
Patch

Most of the time this should just return a substring from 0 to m_portEnd + 1 with a check that the string is long enough and that there is no user or password and an assertion that a '/' is immediately after the port.  If the string is not long enough, the '/' should be appended.  If there is a user and password, we will have to do additional string building, but that is rare.

This should have a test of a URL with a user and password.
This should have a test of a URL with no port.
This should have a test of a URL with a non-special scheme (such as asdf) with and without a slash after the host.

[Alexey Proskuryakov]

Comment on attachment 297685 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297685&action=review

> Tools/TestWebKitAPI/Tests/WebCore/URL.cpp:63
>      EXPECT_EQ(8080, kurl.port().value());

kurl!

[Keith Rollin]

Alex, you had said:

> It looks like you just want to include the '/' after
> the host if there is one.  There will always be one if the scheme is
> something like http, but an unknown scheme without a '/' like
> "asdf://noSlash" won't have one.

I then asked if the slash was required, and what qualified as a being "something like http". You subsequently posted a comment "special schemes are listed here https://url.spec.whatwg.org/#special-scheme", but it's not clear to me if that's your answer to my questions. If so, my reading of that section of the spec tells me that the trailing slash you are talking about is part of the path component, not part of the host+port components. RFC 3986 also indicates this:

         foo://example.com:8042/over/there?name=ferret#nose
         \_/   \______________/\_________/ \_________/ \__/
          |           |            |            |        |
       scheme     authority       path        query   fragment

If the slash is part of the path, then it doesn't seem to me that it should be included in a call that returns the scheme+authority (less any username and/or password).

What's your take here?

[Alex Christensen]

I agree, the slash is in the path.  The scheme+authority should not return the slash

[Keith Rollin]

Created attachment 298053 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 298053 [details]
Patch

Clearing flags on attachment: 298053

Committed r210365: <http://trac.webkit.org/changeset/210365>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

In a sense it's reinvented WebCore::SecurityOrigin. Maybe you should return one of those instead? I added an extremely barebones API test for it last week; all of the tests added here would be good to have in that file too. It would also be good to validate that this function always returns the same result as SecurityOrigin::create(url).toString().

On the other hand, it maybe makes sense to have a way to avoid the construction of a SecurityOrigin object in situations where that's overkill. And there is certainly a difference between a string representation of a security origin (that's what is returned by this function) and an actual security origin (which could additionally contain a separate domain for CORS, or could be opaque and not have protocol/host/port at all).