Bug 1070763 - Ensure that XHRs sniff the BOM for non-JSON responseTypes, and flush the decoder upon end-of-stream; r?hsivonen draft
authorThomas Wisniewski <wisniewskit@gmail.com>
Tue, 03 Oct 2017 08:54:14 -0400
changeset 674224 a399a1295c4a3982871207ed5eb52086db1301e1
parent 674178 11fe0a2895aab26c57bcfe61b3041d7837e954cd
child 734265 6bdc0b67a5fd2b9a43ed31bbb37e08184ac34745
push id82765
push userwisniewskit@gmail.com
push dateTue, 03 Oct 2017 13:59:27 +0000
reviewershsivonen
bugs1070763
milestone58.0a1
Bug 1070763 - Ensure that XHRs sniff the BOM for non-JSON responseTypes, and flush the decoder upon end-of-stream; r?hsivonen MozReview-Commit-ID: ICHbs2BQcbR
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
testing/web-platform/meta/XMLHttpRequest/responsetext-decoding.htm.ini
testing/web-platform/meta/XMLHttpRequest/send-receive-utf16.htm.ini
testing/web-platform/meta/encoding/unsupported-encodings.html.ini
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -516,25 +516,35 @@ XMLHttpRequestMainThread::DetectCharset(
   if (mResponseType == XMLHttpRequestResponseType::Json &&
       encoding != UTF_8_ENCODING) {
     // The XHR spec says only UTF-8 is supported for responseType == "json"
     LogMessage("JSONCharsetWarning", GetOwner());
     encoding = UTF_8_ENCODING;
   }
 
   mResponseCharset = encoding;
-  mDecoder = encoding->NewDecoderWithBOMRemoval();
+
+  // Only sniff the BOM for non-JSON responseTypes
+  if (mResponseType == XMLHttpRequestResponseType::Json) {
+    mDecoder = encoding->NewDecoderWithBOMRemoval();
+  } else {
+    mDecoder = encoding->NewDecoder();
+  }
 
   return NS_OK;
 }
 
 nsresult
 XMLHttpRequestMainThread::AppendToResponseText(const char * aSrcBuffer,
-                                               uint32_t aSrcBufferLen)
+                                               uint32_t aSrcBufferLen,
+                                               bool aLast)
 {
+  // Call this with an empty buffer to send the decoder the signal
+  // that we have hit the end of the stream.
+
   NS_ENSURE_STATE(mDecoder);
 
   CheckedInt<size_t> destBufferLen =
     mDecoder->MaxUTF16BufferLength(aSrcBufferLen);
   if (!destBufferLen.isValid()) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
@@ -545,25 +555,24 @@ XMLHttpRequestMainThread::AppendToRespon
   }
 
   XMLHttpRequestStringWriterHelper helper(mResponseText);
 
   if (!helper.AddCapacity(destBufferLen.value())) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  // XXX there's no handling for incomplete byte sequences on EOF!
   uint32_t result;
   size_t read;
   size_t written;
   bool hadErrors;
   Tie(result, read, written, hadErrors) = mDecoder->DecodeToUTF16(
     AsBytes(MakeSpan(aSrcBuffer, aSrcBufferLen)),
     MakeSpan(helper.EndOfExistingData(), destBufferLen.value()),
-    false);
+    aLast);
   MOZ_ASSERT(result == kInputEmpty);
   MOZ_ASSERT(read == aSrcBufferLen);
   MOZ_ASSERT(written <= destBufferLen.value());
   Unused << hadErrors;
   helper.AddLength(written);
   return NS_OK;
 }
 
@@ -2184,16 +2193,22 @@ XMLHttpRequestMainThread::OnStopRequest(
 {
   AUTO_PROFILER_LABEL("XMLHttpRequestMainThread::OnStopRequest", NETWORK);
 
   if (request != mChannel) {
     // Can this still happen?
     return NS_OK;
   }
 
+  // send the decoder the signal that we've hit the end of the stream,
+  // but only when parsing text (not XML, which does this already).
+  if (mDecoder && !mFlagParseBody) {
+    AppendToResponseText(nullptr, 0, true);
+  }
+
   mWaitingForOnStopRequest = false;
 
   if (mRequestObserver) {
     NS_ASSERTION(mFirstStartRequestSeen, "Inconsistent state!");
     mFirstStartRequestSeen = false;
     mRequestObserver->OnStopRequest(request, ctxt, status);
   }
 
@@ -2359,17 +2374,17 @@ XMLHttpRequestMainThread::OnBodyParseEnd
 
 void
 XMLHttpRequestMainThread::MatchCharsetAndDecoderToResponseDocument()
 {
   if (mResponseXML && mResponseCharset != mResponseXML->GetDocumentCharacterSet()) {
     mResponseCharset = mResponseXML->GetDocumentCharacterSet();
     TruncateResponseText();
     mResponseBodyDecodedPos = 0;
-    mDecoder = mResponseCharset->NewDecoderWithBOMRemoval();
+    mDecoder = mResponseCharset->NewDecoder();
   }
 }
 
 void
 XMLHttpRequestMainThread::ChangeStateToDone()
 {
   StopProgressEventTimer();
 
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -515,17 +515,18 @@ protected:
     unsent,           // object has been constructed.
     opened,           // open() has been successfully invoked.
     headers_received, // redirects followed and response headers received.
     loading,          // response body is being received.
     done,             // data transfer concluded, whether success or error.
   };
 
   nsresult DetectCharset();
-  nsresult AppendToResponseText(const char * aBuffer, uint32_t aBufferLen);
+  nsresult AppendToResponseText(const char* aBuffer, uint32_t aBufferLen,
+                                bool aLast = false);
   static nsresult StreamReaderFunc(nsIInputStream* in,
                                    void* closure,
                                    const char* fromRawSegment,
                                    uint32_t toOffset,
                                    uint32_t count,
                                    uint32_t *writeCount);
   nsresult CreateResponseParsedJSON(JSContext* aCx);
   // Change the state of the object with this. The broadcast argument
deleted file mode 100644
--- a/testing/web-platform/meta/XMLHttpRequest/responsetext-decoding.htm.ini
+++ /dev/null
@@ -1,11 +0,0 @@
-[responsetext-decoding.htm]
-  type: testharness
-  [XMLHttpRequest: responseText decoding (text/plain %FE%FF)]
-    expected: FAIL
-
-  [XMLHttpRequest: responseText decoding (text/plain %FE%FF%FE%FF)]
-    expected: FAIL
-
-  [XMLHttpRequest: responseText decoding (text/plain %C2)]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/XMLHttpRequest/send-receive-utf16.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[send-receive-utf16.htm]
-  type: testharness
-  [UTF-16 with BOM, no encoding in content-type]
-    expected: FAIL
-
--- a/testing/web-platform/meta/encoding/unsupported-encodings.html.ini
+++ b/testing/web-platform/meta/encoding/unsupported-encodings.html.ini
@@ -1,20 +1,8 @@
 [unsupported-encodings.html]
   type: testharness
-  [UTF-32 with BOM should decode as UTF-16LE]
-    expected: FAIL
-
-  [utf-32 with BOM should decode as UTF-16LE]
-    expected: FAIL
-
-  [UTF-32LE with BOM should decode as UTF-16LE]
-    expected: FAIL
-
-  [utf-32le with BOM should decode as UTF-16LE]
-    expected: FAIL
-
   [UTF-32be with BOM should decode as windows-1252]
     expected: FAIL
 
   [utf-32be with BOM should decode as windows-1252]
     expected: FAIL