Bug 166985 – Add layout test for Basic Authentication UTF-8 encoding by default

• Component: WebKit Misc.
• Status: NEW

[John Wilander]

WebCore and WebKit2 currently use UTF-8 for cached Basic Authentication credentials. It should be ISO-8859-1 by default. At least CFNetwork uses ISO-8859-1 for the initial challenge response which means we currently first do ISO-8859-1 and then switch to UTF-8.

The spec says the following about default encoding (https://tools.ietf.org/html/rfc7617#appendix-B.3):

B.3.  Why not simply switch the default encoding to UTF-8?

   There are sites in use today that default to a local character
   encoding scheme, such as ISO-8859-1 ([ISO-8859-1]), and expect user
   agents to use that encoding.  Authentication on these sites will stop
   working if the user agent switches to a different encoding, such as
   UTF-8.

   Note that sites might even inspect the User-Agent header field
   ([RFC7231], Section 5.5.3) to decide which character encoding scheme
   to expect from the client.  Therefore, they might support UTF-8 for
   some user agents, but default to something else for others.  User
   agents in the latter group will have to continue to do what they do
   today until the majority of these servers have been upgraded to
   always use UTF-8.

[John Wilander]

rdar://problem/16025463

[John Wilander]

Created attachment 298708 [details]
Patch

[John Wilander]

I should mention that I have tested manually in WK1.

[Alex Christensen]

Comment on attachment 298708 [details]
Patch

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

> LayoutTests/http/tests/security/authentication/basic-authentication-character-encoding.html:9
> +const nonAsciiCharacters = "\u00e5\u00e4\u00f6";  // The ISO-8859-1 characters åäö

We should add a check for Cyrillic characters, too!

> LayoutTests/http/tests/security/authentication/resources/basic-auth-encoding-checker.php:2
> +//header("Cache-Control: no-store");

there's some commented-out code in this file :(

[Alexey Proskuryakov]

Comment on attachment 298708 [details]
Patch

Do other browsers pass these tests?

[John Wilander]

(In reply to comment #4)
> Comment on attachment 298708 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=298708&action=review
> 
> > LayoutTests/http/tests/security/authentication/basic-authentication-character-encoding.html:9
> > +const nonAsciiCharacters = "\u00e5\u00e4\u00f6";  // The ISO-8859-1 characters åäö
> 
> We should add a check for Cyrillic characters, too!

True. But let's hash out what Alexey is asking about first.

> > LayoutTests/http/tests/security/authentication/resources/basic-auth-encoding-checker.php:2
> > +//header("Cache-Control: no-store");
> 
> there's some commented-out code in this file :(

Oh, will fix.

[John Wilander]

(In reply to comment #5)
> Comment on attachment 298708 [details]
> Patch
> 
> Do other browsers pass these tests?

· Firefox passes for the iframes but uses UTF-8 for the XHR.
· Chrome uses UTF-8 for all three.

Note that the bug in our case is that we switch.

[John Wilander]

I filed a bug with CFNetwork to see if they can switch to UTF-8 instead since the web seems to be fine with that encoding.

[Build Bot]

Comment on attachment 298708 [details]
Patch

Attachment 298708 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2879092

New failing tests:
fast/mediastream/MediaStream-video-element-track-stop.html

[Build Bot]

Created attachment 298717 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Darin Adler]

Comment on attachment 298708 [details]
Patch

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

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:100
> -    String authenticationHeader = "Basic " + base64Encode(String(credential.user() + ":" + credential.password()).utf8());
> +    String authenticationHeader = "Basic " + base64Encode(String(credential.user() + ":" + credential.password()).latin1());

This is a big difference in semantics between the utf8() function and the latin1() function.

The utf8() function works on any string, and can always re-encode valid UTF-16 strings as UTF-8. But the latin1() function turns characters with Unicode values greater than U+00FF into "?" characters. Is that behavior acceptable here? If so, why?

[Darin Adler]

Note also that the latin1() function uses a different version of what Latin-1 means than parsing of webpages. When parsing webpages, Latin-1 always means the Windows variant of Latin-1, which uses 0x80-0x9F to represent non-Latin-1 Unicode characters. The latin1() function uses a more literal straightforward definition and will use 0x80-0x9F to encode U+0080-U+009F instead.

Again, not sure how relevant that is to this use case.

[John Wilander]

CFNetwork agrees this should be fixed in their layer (for the platforms that use them). Repurposing this bug to add a layout test for consistent UTF-8 encoding once it's possible. The layout test in the existing patch should be easy to change.

[Maciej Stachowiak]

Comment on attachment 298708 [details]
Patch

This patch no longer applies. And it seems like only the layout test is desired any more. r- for current version of the patch, but it seems like a good idea to update and land the layout test.