Bug 54271

Created attachment 82120 [details] Patch

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

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

Created attachment 82121 [details] Patch

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.

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.

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.

+ * 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.

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

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.

I'm confused by the r-. Was it for the comment? It seems there were no negative comments about the code change.

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

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

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

(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

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.

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

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