Fetch's Response.statusText is unexpectedly the full http status line for HTTP/2...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Sep 2017 04:05:25 +0000 (04:05 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Sep 2017 04:05:25 +0000 (04:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176479

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2017-09-06
Reviewed by Alexey Proskuryakov.

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):

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.

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

LayoutTests/ChangeLog
LayoutTests/http/wpt/fetch/resources/status-garbage.asis [new file with mode: 0644]
LayoutTests/http/wpt/fetch/resources/status-normal.txt [new file with mode: 0644]
LayoutTests/http/wpt/fetch/resources/status-with-message.asis [new file with mode: 0644]
LayoutTests/http/wpt/fetch/resources/status-without-message.asis [new file with mode: 0644]
LayoutTests/http/wpt/fetch/response-status-text-expected.txt [new file with mode: 0644]
LayoutTests/http/wpt/fetch/response-status-text.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/network/HTTPParsers.cpp

index f8e2452..ad7789e 100644 (file)
@@ -1,3 +1,22 @@
+2017-09-06  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 Alexey Proskuryakov.
+
+        * 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-06  Youenn Fablet  <youenn@apple.com>
 
         NetworkProcess Cache and Caches should be cleared when the last related WebProcess Cache or CacheStorage is destroyed
diff --git a/LayoutTests/http/wpt/fetch/resources/status-garbage.asis b/LayoutTests/http/wpt/fetch/resources/status-garbage.asis
new file mode 100644 (file)
index 0000000..0070f74
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..f3b868d
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..b34781f
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..12bf0b9
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..564c83b
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..c7b8350
--- /dev/null
@@ -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>
index f623e88..7c6bc1d 100644 (file)
@@ -1,3 +1,19 @@
+2017-09-06  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 Alexey Proskuryakov.
+
+        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-06  Eric Carlson  <eric.carlson@apple.com>
 
         Require LoggingHelper overrides to provide identifier
index 64ae972..954fd33 100644 (file)
@@ -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();
 }