2011-04-07 Julien Chaffraix <jchaffraix@codeaurora.org>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2011 06:45:30 +0000 (06:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Apr 2011 06:45:30 +0000 (06:45 +0000)
        Reviewed by Alexey Proskuryakov.

        EventSource should only accept UTF-8 charset
        https://bugs.webkit.org/show_bug.cgi?id=56942

        * ChangeLog-2011-02-16: Corrected previous commit message. Pointed out by Alexey.

        * http/tests/eventsource/eventsource-bad-mime-type-expected.txt: Updated with the new console message.

        * http/tests/eventsource/eventsource-content-type-charset-expected.txt:
        * http/tests/eventsource/eventsource-content-type-charset.html: Beefed up this test and merged
        the following test into it. The new tests check that we dispatch a console error if the charset is
        wrong.

        * http/tests/eventsource/eventsource-content-type-text-event-stream-foobar-expected.txt: Removed.
        * http/tests/eventsource/eventsource-content-type-text-event-stream-foobar.html: Removed.

        * http/tests/eventsource/resources/response-content-type-charset.php: Return the charset passed
        in the URL. Also added PHP's magic quotes handling as this would make the test fail on Mac.

        * http/tests/eventsource/resources/response-content-type-event-stream-foobar.php: Removed.
2011-04-07  Julien Chaffraix  <jchaffraix@codeaurora.org>

        Reviewed by Alexey Proskuryakov.

        EventSource should only accept UTF-8 charset
        https://bugs.webkit.org/show_bug.cgi?id=56942

        Following the discussion on bug 45372, this change implements the recommended
        way of handling "charset". We only accept UTF-8 but no other encoding. This matches
        the encoding of the EventSource and also may fix TomCat that automatically send this
        charset.

        * page/EventSource.cpp:
        (WebCore::EventSource::didReceiveResponse): We now check the charset attribute and if it is
        not UTF-8, abort the connection and log the error to the console. Also we log if the MIME type
        is wrong to the console to help debugging (only in the case of an HTTP 200 response though).

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83260 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/ChangeLog-2011-02-16
LayoutTests/http/tests/eventsource/eventsource-bad-mime-type-expected.txt
LayoutTests/http/tests/eventsource/eventsource-content-type-charset-expected.txt
LayoutTests/http/tests/eventsource/eventsource-content-type-charset.html
LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar-expected.txt [deleted file]
LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar.html [deleted file]
LayoutTests/http/tests/eventsource/resources/response-content-type-charset.php
LayoutTests/http/tests/eventsource/resources/response-content-type-event-stream-foobar.php [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/page/EventSource.cpp

index 3d73a6b..de43e58 100644 (file)
@@ -1,3 +1,27 @@
+2011-04-07  Julien Chaffraix  <jchaffraix@codeaurora.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        EventSource should only accept UTF-8 charset
+        https://bugs.webkit.org/show_bug.cgi?id=56942
+
+        * ChangeLog-2011-02-16: Corrected previous commit message. Pointed out by Alexey.
+
+        * http/tests/eventsource/eventsource-bad-mime-type-expected.txt: Updated with the new console message.
+
+        * http/tests/eventsource/eventsource-content-type-charset-expected.txt:
+        * http/tests/eventsource/eventsource-content-type-charset.html: Beefed up this test and merged
+        the following test into it. The new tests check that we dispatch a console error if the charset is
+        wrong.
+
+        * http/tests/eventsource/eventsource-content-type-text-event-stream-foobar-expected.txt: Removed.
+        * http/tests/eventsource/eventsource-content-type-text-event-stream-foobar.html: Removed.
+
+        * http/tests/eventsource/resources/response-content-type-charset.php: Return the charset passed
+        in the URL. Also added PHP's magic quotes handling as this would make the test fail on Mac.
+
+        * http/tests/eventsource/resources/response-content-type-event-stream-foobar.php: Removed.
+
 2011-04-07  Kent Tamura  <tkent@chromium.org>
 
         [Chromium] Update expectations for xsl-blocked.php and pate-text-011.html.
index d113205..bdc2102 100644 (file)
         https://bugs.webkit.org/show_bug.cgi?id=45372
 
         Test that a Content-Type of "text/event-stream; charset=UTF8" works correctly but
-        "text/event-stream" does not work.
+        "text/event-stream-foobar" does not work.
 
         * http/tests/eventsource/eventsource-content-type-charset-expected.txt: Added.
         * http/tests/eventsource/eventsource-content-type-charset.html: Added.
index 20341bc..6d5c5bb 100644 (file)
@@ -1,3 +1,4 @@
+CONSOLE MESSAGE: line 1: EventSource's response has a MIME type ("text/bogus") that is not "text/event-stream". Aborting the connection.
 Test EventSource with an event-stream with incorrect mime-type.
 
 PASS
index ef1f63b..c53ccb3 100644 (file)
@@ -1,7 +1,12 @@
+CONSOLE MESSAGE: line 1: EventSource's response has a charset ("windows-1251") that is not UTF-8. Aborting the connection.
+CONSOLE MESSAGE: line 1: 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
 
-Test EventSource with an event-stream with a Content-Type with a charset is still recognized. You should see 2 PASSED below (one for open and one for message).
+Test EventSource with an event-stream with a Content-Type with a charset is still recognized. You should see 5 PASSED below.
 
-PASSED: got open event
-PASSED: got message event
+PASSED: text/event-stream; charset=UTF-8
+PASSED: text/event-stream; charset=windows-1251
+PASSED: text/event-stream; charset=utf-8
+PASSED: text/event-stream; charset="UTF-8"
+PASSED: text/event-stream-foobar;
 
index 4e07fa8..3ddae4a 100644 (file)
@@ -2,7 +2,7 @@
 <html>
 <body>
 <p>Test for bug <a href="https://bugs.webkit.org/show_bug.cgi?id=45372">45372</a>: https://bugs.webkit.org/show_bug.cgi?id=45372</p>
-<p>Test EventSource with an event-stream with a Content-Type with a charset is still recognized. You should see 2 PASSED below (one for open and one for message).</p>
+<p>Test EventSource with an event-stream with a Content-Type with a charset is still recognized. You should see 5 PASSED below.</p>
 <div id="result"></div>
 <script>
 function log(msg) {
@@ -14,27 +14,79 @@ if (window.layoutTestController) {
     layoutTestController.waitUntilDone();
 }
 
-var es = new EventSource("resources/response-content-type-charset.php");
+function shouldGetMessage(es)
+{
+    if (es.sawOpen && es.sawMessage && !es.sawError)
+        log("PASSED: " + es.contentType);
+    else
+        log("FAILED: " + es.contentType);
+}
+
+function shouldFail(es)
+{
+    if (es.sawError && !es.sawOpen && !es.sawMessages)
+        log("PASSED: " + es.contentType);
+    else
+        log("FAILED: " + es.contentType);
+}
+
+var i = 0;
+var contentTypes = [ 'text/event-stream; charset=UTF-8',
+                     'text/event-stream; charset=windows-1251',
+                     'text/event-stream; charset=utf-8',
+                     'text/event-stream; charset="UTF-8"',
+                     'text/event-stream-foobar;'
+                   ];
+
+var expectedResultCallback = [ shouldGetMessage,
+                               shouldFail,
+                               shouldGetMessage,
+                               shouldGetMessage,
+                               shouldFail
+                             ];
 
-es.onopen = function (evt) {
-    log("PASSED: got open event");
+function openListener(evt) {
+    evt.target.sawOpen = true;
 };
 
-es.onmessage = function (evt) {
-    log("PASSED: got message event");
-    end();
+function messageListener(evt) {
+    evt.target.sawMessage = true;
+    evt.target.successCallback(evt.target);
+    evt.target.close();
+    next();
 };
 
-es.onerror = function () {
-    log("FAILED: unexpected error event");
-    end();
+function errorListener(evt) {
+    evt.target.sawError = true;
+    evt.target.successCallback(evt.target);
+    evt.target.close();
+    next();
 };
 
+function startRequest()
+{
+    es = new EventSource("resources/response-content-type-charset.php?contentType=" + escape(contentTypes[i]));
+    es.onopen = openListener;
+    es.onmessage = messageListener;
+    es.onerror = errorListener;
+    es.successCallback = expectedResultCallback[i];
+    es.contentType = contentTypes[i];
+    ++i;
+}
+
+function next() {
+    if (i >= contentTypes.length) {
+        end();
+        return;
+    }
+    startRequest();
+}
+
 function end() {
-    es.close();
     if (window.layoutTestController)
         layoutTestController.notifyDone();
 }
+startRequest();
 </script>
 </body>
 </html>
diff --git a/LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar-expected.txt b/LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar-expected.txt
deleted file mode 100644 (file)
index 69d1460..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-Test for bug 45372: https://bugs.webkit.org/show_bug.cgi?id=45372
-
-Test EventSource with an event-stream with a Content-Type of "text/event-stream-foobar" is not recognized as valid. You should see 1 PASSED below.
-
-PASSED
-
diff --git a/LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar.html b/LayoutTests/http/tests/eventsource/eventsource-content-type-text-event-stream-foobar.html
deleted file mode 100644 (file)
index 6fcfe5c..0000000
+++ /dev/null
@@ -1,44 +0,0 @@
-<!DOCTYPE html>
-<html>
-<body>
-<p>Test for bug <a href="https://bugs.webkit.org/show_bug.cgi?id=45372">45372</a>: https://bugs.webkit.org/show_bug.cgi?id=45372</p>
-<p>Test EventSource with an event-stream with a Content-Type of "text/event-stream-foobar" is not recognized as valid. You should see 1 PASSED below.</p>
-<div id="result"></div>
-<script>
-function log(msg) {
-    document.getElementById("result").innerHTML += msg + "<br>";
-}
-
-if (window.layoutTestController) {
-    layoutTestController.dumpAsText();
-    layoutTestController.waitUntilDone();
-}
-
-var es = new EventSource("resources/response-content-type-text-stream-foobar.php");
-
-es.onopen = function (evt) {
-    log("FAILED: got unexpected open event");
-    end();
-};
-
-es.onmessage = function (evt) {
-    log("FAILED: got unexpected message event");
-    end();
-};
-
-es.onerror = function () {
-    if (es.readyState == es.CLOSED)
-        log("PASSED");
-    else
-        log("FAILED: invalid state");
-    end();
-};
-
-function end() {
-    es.close();
-    if (window.layoutTestController)
-        layoutTestController.notifyDone();
-}
-</script>
-</body>
-</html>
index a7cd42e..37f6dad 100644 (file)
@@ -1,8 +1,13 @@
 <?php
-header("Content-Type: text/event-stream; charset=UTF8");
+$contentType = $_GET["contentType"];
+// If the magic quotes option is enabled, the charset could be escaped and we
+// would fail our test. For example, charset="utf-8" would become charset=\"utf-8\".
+if (get_magic_quotes_gpc()) {
+    $contentType = stripslashes($contentType);
+}
+header("Content-Type: $contentType");
 ?>
 
 id: 77
 retry: 300
 data: hello
-
diff --git a/LayoutTests/http/tests/eventsource/resources/response-content-type-event-stream-foobar.php b/LayoutTests/http/tests/eventsource/resources/response-content-type-event-stream-foobar.php
deleted file mode 100644 (file)
index ef1e460..0000000
+++ /dev/null
@@ -1,8 +0,0 @@
-<?php
-header("Content-Type: text/event-stream-foobar");
-?>
-
-id: 77
-retry: 300
-data: hello
-
index 8218762..3dc3d6c 100644 (file)
@@ -1,3 +1,20 @@
+2011-04-07  Julien Chaffraix  <jchaffraix@codeaurora.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        EventSource should only accept UTF-8 charset
+        https://bugs.webkit.org/show_bug.cgi?id=56942
+
+        Following the discussion on bug 45372, this change implements the recommended
+        way of handling "charset". We only accept UTF-8 but no other encoding. This matches
+        the encoding of the EventSource and also may fix TomCat that automatically send this
+        charset.
+
+        * page/EventSource.cpp:
+        (WebCore::EventSource::didReceiveResponse): We now check the charset attribute and if it is
+        not UTF-8, abort the connection and log the error to the console. Also we log if the MIME type
+        is wrong to the console to help debugging (only in the case of an HTTP 200 response though).
+
 2011-04-07  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Maciej Stachowiak.
index 08d0868..2a52bcb 100644 (file)
@@ -2,6 +2,7 @@
  * Copyright (C) 2009 Ericsson AB
  * All rights reserved.
  * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, Code Aurora Forum. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -45,6 +46,7 @@
 #include "ResourceError.h"
 #include "ResourceRequest.h"
 #include "ResourceResponse.h"
+#include "ScriptCallStack.h"
 #include "ScriptExecutionContext.h"
 #include "SerializedScriptValue.h"
 #include "TextResourceDecoder.h"
@@ -180,7 +182,32 @@ ScriptExecutionContext* EventSource::scriptExecutionContext() const
 void EventSource::didReceiveResponse(const ResourceResponse& response)
 {
     int statusCode = response.httpStatusCode();
-    if (statusCode == 200 && response.mimeType() == "text/event-stream") {
+    bool mimeTypeIsValid = response.mimeType() == "text/event-stream";
+    bool responseIsValid = statusCode == 200 && mimeTypeIsValid;
+    if (responseIsValid) {
+        const String& charset = response.textEncodingName();
+        // If we have a charset, the only allowed value is UTF-8 (case-insensitive). This should match
+        // the updated EventSource standard.
+        responseIsValid = charset.isEmpty() || equalIgnoringCase(charset, "UTF-8");
+        if (!responseIsValid) {
+            String message = "EventSource's response has a charset (\"";
+            message += charset;
+            message += "\") that is not UTF-8. Aborting the connection.";
+            // FIXME: We are missing the source line.
+            scriptExecutionContext()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message, 1, String(), 0);
+        }
+    } else {
+        // To keep the signal-to-noise ratio low, we only log 200-response with an invalid MIME type.
+        if (statusCode == 200 && !mimeTypeIsValid) {
+            String message = "EventSource's response has a MIME type (\"";
+            message += response.mimeType();
+            message += "\") that is not \"text/event-stream\". Aborting the connection.";
+            // FIXME: We are missing the source line.
+            scriptExecutionContext()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message, 1, String(), 0);
+        }
+    }
+
+    if (responseIsValid) {
         m_state = OPEN;
         dispatchEvent(Event::create(eventNames().openEvent, false, false));
     } else {