Changeset 277087 in webkit

Timestamp:

May 6, 2021 7:52:13 AM (3 years ago)

Author:

Chris Dumez

Message:

imported/w3c/web-platform-tests/eventsource/format-utf-8.htm is failing on some platforms
​https://bugs.webkit.org/show_bug.cgi?id=225416
<rdar://77529801>

Reviewed by Youenn Fablet.

Source/WebCore:

The HTML specification [1] states that EventSource streams should always be decoded as UTF-8.
However, we had logic in WebKit that was checking the charset of the response and aborting
when the charset is not UTF-8. It looks like we were lucky until now that we would fail to
determine the charset of the response and proceed anyway. However, due to underlying system
changes, in the context of this test, we are now properly detecting the response charset as
"windows-1252" and (incorrectly) aborting. To address the issue, I no longer abort the
connection if the charset is not "utf-8". We do still log a console warning but we proceed
decoding the stream as UTF-8 no matter the response's charset, as per the specification.

[1] ​https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation

No new tests, covered by existing test that is failing on some platforms.

• page/EventSource.cpp:

(WebCore::EventSource::responseIsValid const):

LayoutTests:

Dump JS Console messages to stderr for this test as it will only get logged on some
platforms.

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/http/tests/eventsource/eventsource-content-type-charset-expected.txt (modified) (1 diff)
• LayoutTests/http/tests/eventsource/eventsource-content-type-charset.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/EventSource.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-05-06  Chris Dumez  <cdumez@apple.com>
+
+        imported/w3c/web-platform-tests/eventsource/format-utf-8.htm is failing on some platforms
+        https://bugs.webkit.org/show_bug.cgi?id=225416
+        <rdar://77529801>
+
+        Reviewed by Youenn Fablet.
+
+        Dump JS Console messages to stderr for this test as it will only get logged on some
+        platforms.
+
+        * TestExpectations:
+
 2021-05-06  Tim Nguyen  <ntim@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -388 +388 @@
 
 # Console log lines may appear in a different order so we silence them.
+imported/w3c/web-platform-tests/eventsource/format-utf-8.htm [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ DumpJSConsoleLogInStdErr Failure Pass ]
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ DumpJSConsoleLogInStdErr ]

trunk/LayoutTests/http/tests/eventsource/eventsource-content-type-charset-expected.txt

@@ -1 @@
-CONSOLE MESSAGE: EventSource's response has a charset ("windows-1251") that is not UTF-8. Aborting the connection.
+CONSOLE MESSAGE: EventSource's response has a charset ("windows-1251") that is not UTF-8. The response will be decoded as UTF-8.
 CONSOLE MESSAGE: EventSource's response has a MIME type ("text/event-stream-foobar") that is not "text/event-stream". Aborting the connection.
 Test for bug 45372: https://bugs.webkit.org/show_bug.cgi?id=45372

trunk/LayoutTests/http/tests/eventsource/eventsource-content-type-charset.html

@@ -40 +40 @@
 
 var expectedResultCallback = [ shouldGetMessage,
-                               shouldFail,
+                               shouldGetMessage,
                                shouldGetMessage,
                                shouldGetMessage,

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-06  Chris Dumez  <cdumez@apple.com>
+
+        imported/w3c/web-platform-tests/eventsource/format-utf-8.htm is failing on some platforms
+        https://bugs.webkit.org/show_bug.cgi?id=225416
+        <rdar://77529801>
+
+        Reviewed by Youenn Fablet.
+
+        The HTML specification [1] states that EventSource streams should always be decoded as UTF-8.
+        However, we had logic in WebKit that was checking the charset of the response and aborting
+        when the charset is not UTF-8. It looks like we were lucky until now that we would fail to
+        determine the charset of the response and proceed anyway. However, due to underlying system
+        changes, in the context of this test, we are now properly detecting the response charset as
+        "windows-1252" and (incorrectly) aborting. To address the issue, I no longer abort the
+        connection if the charset is not "utf-8". We do still log a console warning but we proceed
+        decoding the stream as UTF-8 no matter the response's charset, as per the specification.
+
+        [1] https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation
+
+        No new tests, covered by existing test that is failing on some platforms.
+
+        * page/EventSource.cpp:
+        (WebCore::EventSource::responseIsValid const):
+
 2021-05-06  Martin Robinson  <mrobinson@webkit.org>
 

trunk/Source/WebCore/page/EventSource.cpp

@@ -184 +184 @@
     }
 
-    // If we have a charset, the only allowed value is UTF-8 (case-insensitive).
+    // The specification states we should always decode as UTF-8. If there is a provided charset and it is not UTF-8, then log a warning
+    // message but keep going anyway.
     auto& charset = response.textEncodingName();
     if (!charset.isEmpty() && !equalLettersIgnoringASCIICase(charset, "utf-8")) {
-        auto message = makeString("EventSource's response has a charset (\"", charset, "\") that is not UTF-8. Aborting the connection.");
+        auto message = makeString("EventSource's response has a charset (\"", charset, "\") that is not UTF-8. The response will be decoded as UTF-8.");
         // FIXME: Console message would be better with a source code location; where would we get that?
         scriptExecutionContext()->addConsoleMessage(MessageSource::JS, MessageLevel::Error, WTFMove(message));
-        return false;
     }