Bug 54271 – Safari Extensions broken due to KURL canonicalization added in r78044

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: Platform

[Eric Seidel (no email)]

safari-extension: scheme needs special case-sensitive treatment in KURL

safari-extensions broke after http://trac.webkit.org/changeset/78044 (or so I'm told).

See further discussion in:
https://bugs.webkit.org/show_bug.cgi?id=54084#c4

[Eric Seidel (no email)]

Created attachment 82120 [details]
Patch

[Early Warning System Bot]

Attachment 82120 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7884571

[Eric Seidel (no email)]

append() was never used on non-apple ports.  Fixing it now.

[Eric Seidel (no email)]

Created attachment 82121 [details]
Patch

[Timothy Hatcher]

Comment on attachment 82121 [details]
Patch

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

> Source/WebCore/platform/KURL.cpp:1354
> +            // safari-extension: host contents are case sensitive (in violation of RFC 2396).
> +            // https://bugs.webkit.org/show_bug.cgi?id=54271

No where in RFC 2396 do I see that safari-extension is in violation.

> This authority component is typically defined by an Internet-based
> server or a scheme-specific registry of naming authorities.

And:

> The structure of a registry-based naming authority is specific to the
> URI scheme, but constrained to the allowed characters for an
> authority component.
>
>     reg_name      = 1*( unreserved | escaped | "$" | "," |
>                          ";" | ":" | "@" | "&" | "=" | "+" )

And "unreserved" is defined as (expanded) "lowalpha | upalpha | digit | mark".

Only if the scheme uses "Server-based Naming Authority" does the case-insensitive domain name logic apply.

> Hostnames take the form described in Section 3 of [RFC1034] and section 2.1 of [RFC1123]

So I disagree that "safari-extension" is in violation, and that any domain canonicalization should only apply to schemes we know treat the athority as server-based. I would also argue that this should apply to security origin too. Only the scheme is called out as being case-insensitive for all URIs.

[Adam Roben (:aroben)]

Section 6 "URI Normalization and Equivalence" seems relevant:

In many cases, different URI strings may actually identify the
identical resource. For example, the host names used in URL are
actually case insensitive, and the URL <http://www.XEROX.com> is
equivalent to <http://www.xerox.com>. In general, the rules for
equivalence and definition of a normal form, if any, are scheme
dependent. When a scheme uses elements of the common syntax, it will
also use the common syntax equivalence rules, namely that the scheme
and hostname are case insensitive and a URL with an explicit ":port",
where the port is the default for the scheme, is equivalent to one
where the port is elided.

[Adam Roben (:aroben)]

I guess Tim is arguing that the authority part of safari-extension: URLs falls under "Registry-based Naming Authority", not "Server-based Naming Authority". I think I agree with him. Here's my attempt to clarify the argument.

From section 3.2 "Authority Component":

> Many URI schemes include a top hierarchical element for a naming
> authority, such that the namespace defined by the remainder of the
> URI is governed by that authority. 

This certainly applies to safari-extension: URLs.

> This authority component is typically defined by an Internet-based
> server or a scheme-specific registry of naming authorities.

safari-extension: URLs definitely don't use an "Internet-based server", and they do use a "scheme-specific registry of naming authorities".

In addition, section 3.2.2 "Server-based Naming Authority" specifically mentions "URL schemes that involve the direct use of an IP-based protocol to a specified server on the Internet", which does not describe safari-extension: URLs.

So, given that safari-extension: URLs do not use a server-based naming authority, what rules apply to their syntax? Section 3.2.1 "Registry-based Naming Authority" says only this:

> The structure of a registry-based naming authority is specific to the
> URI scheme, but constrained to the allowed characters for an
> authority component.
>
>    reg_name      = 1*( unreserved | escaped | "$" | "," |
>                        ";" | ":" | "@" | "&" | "=" | "+" )

Note that "hostname" isn't defined here at all; it's only defined for URIs that use a server-based naming authority. So presumably the prose in section 6 that mentions hostnames being case-insensitive doesn't apply here; safari-extension: URLs have no hostnames at all.

[Alexey Proskuryakov]

+        * platform/mac/fast/url/script-tests/TEMPLATE.html: Added.
+        * platform/mac/fast/url/script-tests/safari-extension.js: Added.

Please don't add a script-tests directory.

[Timothy Hatcher]

Should we revert this KURL change so we can take our time considering the right fix?

[Alexey Proskuryakov]

I would encourage reverting if we cannot come up with a solution very quickly. We should also take a closer look at other schemes that may be affected by this change.

[Eric Seidel (no email)]

I'm confused by the r-.  Was it for the comment?

It seems there were no negative comments about the code change.

[Adam Barth]

As discussed with Eric, this issue is coupled with the fact that KURL don't have a notion of a hierarchal URI scheme.

[Adam Barth]

More specifically, it attempts to infer whether a scheme is hierarchical based on its structure, which is different from how other browsers behave.

[Eric Seidel (no email)]

I'm working on a new patch which only lowers the host name for schemes which are known to be "standard".
http://www.google.com/codesearch/p?hl=en#R_oU-jn3RFk/trunk/src/url_util.cc&q=standardurl%20package:http://google-url%5C.googlecode%5C.com&l=62

[Eric Seidel (no email)]

(In reply to comment #13)
> More specifically, it attempts to infer whether a scheme is hierarchical based on its structure, which is different from how other browsers behave.

An example of what Adam is referring to:
https://bugs.webkit.org/show_bug.cgi?id=10323

[Timothy Hatcher]

The r- was not for the comment specifically — though the comment is incorrect according to my interpretation of the RFCs.

The r- was because special casing the "safari-extension" scheme is not the right solution. It sounds like you are working on the right solution now.

[Timothy Hatcher]

The original change that broke this was rolled out by Eric in r78395. Should we close this then?

[Adam Barth]

Sure.  This patch in this bug will likely be useful when we re-land hostname canonicalization, but this issue is resolved.