Fetch's Response.statusText is unexpectedly the full http status line…

… for HTTP/2 responses https://bugs.webkit.org/show_bug.cgi?id=176479 Patch by Joseph Pecoraro <pecoraro@apple.com> on 2017-09-08 Reviewed by Alex Christensen. Source/WebCore: Test: http/wpt/fetch/response-status-text.html HTTP/2 doesn't include a status reason phrase. So the "status line" ends up just being the version and status code. Fallback to the empty string instead of the full line. * platform/network/HTTPParsers.cpp: (WebCore::extractReasonPhraseFromHTTPStatusLine): Source/WebKit: * NetworkProcess/NetworkProcess.cpp: (WebKit::NetworkProcess::initializeNetworkProcess): Initialize AtomicString statics like emptyAtom(). LayoutTests: * http/wpt/fetch/resources/status-garbage.asis: Added. * http/wpt/fetch/resources/status-normal.txt: Added. * http/wpt/fetch/resources/status-with-message.asis: Added. * http/wpt/fetch/resources/status-without-message.asis: Added. Various text HTTP responses with different status lines. * http/wpt/fetch/response-status-text-expected.txt: Added. * http/wpt/fetch/response-status-text.html: Added. Test the Fetch Response's status / statusText for different HTTP status lines. The status without a message is similiar to HTTP/2 because HTTP/2 only has a :status pseudo-header and lacks a reason phrase. Canonical link: https://commits.webkit.org/193157@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221804 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Sep 9, 2017 · 93719e1 · 93719e1
1 parent 3cc77ed
commit 93719e1
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 0 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2017-09-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Fetch's Response.statusText is unexpectedly the full http status line for HTTP/2 responses
+        https://bugs.webkit.org/show_bug.cgi?id=176479
+
+        Reviewed by Alex Christensen.
+
+        * http/wpt/fetch/resources/status-garbage.asis: Added.
+        * http/wpt/fetch/resources/status-normal.txt: Added.
+        * http/wpt/fetch/resources/status-with-message.asis: Added.
+        * http/wpt/fetch/resources/status-without-message.asis: Added.
+        Various text HTTP responses with different status lines.
+
+        * http/wpt/fetch/response-status-text-expected.txt: Added.
+        * http/wpt/fetch/response-status-text.html: Added.
+        Test the Fetch Response's status / statusText for different HTTP status lines.
+        The status without a message is similiar to HTTP/2 because HTTP/2 only
+        has a :status pseudo-header and lacks a reason phrase.
+
 2017-09-08  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Implement the attribute HTMLImageElement.async 

diff --git a/LayoutTests/http/wpt/fetch/resources/status-garbage.asis b/LayoutTests/http/wpt/fetch/resources/status-garbage.asis
@@ -0,0 +1 @@
+ALPHA BETA
diff --git a/LayoutTests/http/wpt/fetch/resources/status-normal.txt b/LayoutTests/http/wpt/fetch/resources/status-normal.txt
@@ -0,0 +1 @@
+Intentionally empty.
diff --git a/LayoutTests/http/wpt/fetch/resources/status-with-message.asis b/LayoutTests/http/wpt/fetch/resources/status-with-message.asis
@@ -0,0 +1 @@
+HTTP/1.1 200 Alpha
diff --git a/LayoutTests/http/wpt/fetch/resources/status-without-message.asis b/LayoutTests/http/wpt/fetch/resources/status-without-message.asis
@@ -0,0 +1 @@
+HTTP/1.1 200
diff --git a/LayoutTests/http/wpt/fetch/response-status-text-expected.txt b/LayoutTests/http/wpt/fetch/response-status-text-expected.txt
@@ -0,0 +1,6 @@
+
+PASS Normal status text. 
+PASS Abnormal status text. 
+PASS Empty status text. 
+PASS Garbage status line. 
+
diff --git a/LayoutTests/http/wpt/fetch/response-status-text.html b/LayoutTests/http/wpt/fetch/response-status-text.html
@@ -0,0 +1,38 @@
+<!doctype html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Response status and statusText given various HTTP response status lines.</title>
+  <script src="/resources/testharness.js"></script>
+  <script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+promise_test(test => {
+    return fetch("resources/status-normal.txt").then((response) => {
+        assert_equals(response.status, 200);
+        assert_equals(response.statusText, "OK");
+    });
+}, "Normal status text.");
+
+promise_test(test => {
+    return fetch("resources/status-with-message.asis").then((response) => {
+        assert_equals(response.status, 200);
+        assert_equals(response.statusText, "Alpha");
+    });
+}, "Abnormal status text.");
+
+promise_test(test => {
+    return fetch("resources/status-without-message.asis").then((response) => {
+        assert_equals(response.status, 200);
+        assert_equals(response.statusText, "");
+    });
+}, "Empty status text.");
+
+promise_test(test => {
+    let promise = fetch("resources/status-garbage.asis");
+    return promise_rejects(test, new TypeError(), promise);
+}, "Garbage status line.");
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2017-09-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Fetch's Response.statusText is unexpectedly the full http status line for HTTP/2 responses
+        https://bugs.webkit.org/show_bug.cgi?id=176479
+
+        Reviewed by Alex Christensen.
+
+        Test: http/wpt/fetch/response-status-text.html
+
+        HTTP/2 doesn't include a status reason phrase. So the "status line"
+        ends up just being the version and status code. Fallback to the empty
+        string instead of the full line.
+
+        * platform/network/HTTPParsers.cpp:
+        (WebCore::extractReasonPhraseFromHTTPStatusLine):
+
 2017-09-08  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Implement the attribute HTMLImageElement.async 

diff --git a/Source/WebCore/platform/network/HTTPParsers.cpp b/Source/WebCore/platform/network/HTTPParsers.cpp
@@ -478,12 +478,18 @@ ContentTypeOptionsDisposition parseContentTypeOptionsHeader(const String& header
     return ContentTypeOptionsNone;
 }
 
+// For example: "HTTP/1.1 200 OK" => "OK".
+// Note that HTTP/2 does not include a reason phrase, so we return the empty atom.
 AtomicString extractReasonPhraseFromHTTPStatusLine(const String& statusLine)
 {
     StringView view = statusLine;
     size_t spacePos = view.find(' ');
+
     // Remove status code from the status line.
     spacePos = view.find(' ', spacePos + 1);
+    if (spacePos == notFound)
+        return emptyAtom();
+
     return view.substring(spacePos + 1).toAtomicString();
 }
 

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,14 @@
+2017-09-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Fetch's Response.statusText is unexpectedly the full http status line for HTTP/2 responses
+        https://bugs.webkit.org/show_bug.cgi?id=176479
+
+        Reviewed by Alex Christensen.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::initializeNetworkProcess):
+        Initialize AtomicString statics like emptyAtom().
+
 2017-09-08  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r221773.

diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
@@ -66,6 +66,7 @@
 #include <pal/SessionID.h>
 #include <wtf/OptionSet.h>
 #include <wtf/RunLoop.h>
+#include <wtf/text/AtomicString.h>
 #include <wtf/text/CString.h>
 
 #if ENABLE(SEC_ITEM_SHIM)
@@ -207,6 +208,7 @@ void NetworkProcess::initializeNetworkProcess(NetworkProcessCreationParameters&&
     platformInitializeNetworkProcess(parameters);
 
     WTF::Thread::setCurrentThreadIsUserInitiated();
+    AtomicString::init();
 
     m_suppressMemoryPressureHandler = parameters.shouldSuppressMemoryPressureHandler;
     m_loadThrottleLatency = parameters.loadThrottleLatency;