Bug 1484990 - Use BulkWrite instead of write past length via BeginWriting() in XHR. r=baku

The old code assumes that it's OK to use nsAString::BeginWriting() to write
past the string's logical length if the string has enough capacity. This is
bogus, because the string doesn't know of data written past its logical
length.

The BulkWrite API has been created precisely for this purpose and allows
orderly capacity-aware low-level writes to the string.

MozReview-Commit-ID: BYQHl8Z9Fbd

Differential Revision: https://phabricator.services.mozilla.com/D3886

--HG--
extra : moz-landing-system : lando

dom/xhr/XMLHttpRequestMainThread.cpp

@@ -493,8 +493,7 @@ XMLHttpRequestMainThread::DetectCharset()
 }
 
 nsresult
-XMLHttpRequestMainThread::AppendToResponseText(const char * aSrcBuffer,
-                                               uint32_t aSrcBufferLen,
+XMLHttpRequestMainThread::AppendToResponseText(Span<const uint8_t> aBuffer,
                                                bool aLast)
 {
   // Call this with an empty buffer to send the decoder the signal
@@ -502,22 +501,22 @@ XMLHttpRequestMainThread::AppendToResponseText(const char * aSrcBuffer,
 
   NS_ENSURE_STATE(mDecoder);
 
-  CheckedInt<size_t> destBufferLen =
-    mDecoder->MaxUTF16BufferLength(aSrcBufferLen);
-  if (!destBufferLen.isValid()) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+  uint32_t len = mResponseText.Length();
 
-  CheckedInt32 size = mResponseText.Length();
-  size += destBufferLen.value();
-  if (!size.isValid()) {
+  CheckedInt<size_t> destBufferLen =
+    mDecoder->MaxUTF16BufferLength(aBuffer.Length());
+  destBufferLen += len;    
+  if (!destBufferLen.isValid() || destBufferLen.value() > UINT32_MAX) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   XMLHttpRequestStringWriterHelper helper(mResponseText);
 
-  if (!helper.AddCapacity(destBufferLen.value())) {
-    return NS_ERROR_OUT_OF_MEMORY;
+  nsresult rv;
+  BulkWriteHandle<char16_t> handle =
+    helper.BulkWrite(destBufferLen.value(), rv);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
   uint32_t result;
@@ -525,14 +524,15 @@ XMLHttpRequestMainThread::AppendToResponseText(const char * aSrcBuffer,
   size_t written;
   bool hadErrors;
   Tie(result, read, written, hadErrors) = mDecoder->DecodeToUTF16(
-    AsBytes(MakeSpan(aSrcBuffer, aSrcBufferLen)),
-    MakeSpan(helper.EndOfExistingData(), destBufferLen.value()),
+    aBuffer,
+    handle,
     aLast);
   MOZ_ASSERT(result == kInputEmpty);
-  MOZ_ASSERT(read == aSrcBufferLen);
-  MOZ_ASSERT(written <= destBufferLen.value());
+  MOZ_ASSERT(read == aBuffer.Length());
+  len += written;
+  MOZ_ASSERT(len <= destBufferLen.value());
   Unused << hadErrors;
-  helper.AddLength(written);
+  handle.Finish(len, false);
   if (aLast) {
     // Drop the finished decoder to avoid calling into a decoder
     // that has finished.
@@ -597,8 +597,8 @@ XMLHttpRequestMainThread::GetResponseText(XMLHttpRequestStringSnapshot& aSnapsho
   MOZ_ASSERT(mResponseBodyDecodedPos < mResponseBody.Length() ||
              mState == XMLHttpRequest_Binding::DONE,
              "Unexpected mResponseBodyDecodedPos");
-  aRv = AppendToResponseText(mResponseBody.get() + mResponseBodyDecodedPos,
-                             mResponseBody.Length() - mResponseBodyDecodedPos,
+  Span<const uint8_t> span = mResponseBody;
+  aRv = AppendToResponseText(span.From(mResponseBodyDecodedPos),
                              mState == XMLHttpRequest_Binding::DONE);
   if (aRv.Failed()) {
     return;
@@ -1608,7 +1608,7 @@ XMLHttpRequestMainThread::StreamReaderFunc(nsIInputStream* in,
              xmlHttpRequest->mResponseType == XMLHttpRequestResponseType::Json) {
     MOZ_ASSERT(!xmlHttpRequest->mResponseXML,
                "We shouldn't be parsing a doc here");
-    rv = xmlHttpRequest->AppendToResponseText(fromRawSegment, count);
+    rv = xmlHttpRequest->AppendToResponseText(AsBytes(MakeSpan(fromRawSegment, count)));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
@@ -2099,7 +2099,7 @@ XMLHttpRequestMainThread::OnStopRequest(nsIRequest *request, nsISupports *ctxt,
       ((mResponseType == XMLHttpRequestResponseType::Text) ||
         (mResponseType == XMLHttpRequestResponseType::Json) ||
         (mResponseType == XMLHttpRequestResponseType::_empty && !mResponseXML))) {
-    AppendToResponseText(nullptr, 0, true);
+    AppendToResponseText(Span<const uint8_t>(), true);
   }
 
   mWaitingForOnStopRequest = false;

dom/xhr/XMLHttpRequestMainThread.h

@@ -481,7 +481,7 @@ public:
 
 protected:
   nsresult DetectCharset();
-  nsresult AppendToResponseText(const char * aBuffer, uint32_t aBufferLen,
+  nsresult AppendToResponseText(Span<const uint8_t> aBuffer,
                                 bool aLast = false);
   static nsresult StreamReaderFunc(nsIInputStream* in,
                                    void* closure,

dom/xhr/XMLHttpRequestString.cpp

@@ -38,6 +38,12 @@ public:
     return mData.Length();
   }
 
+  mozilla::BulkWriteHandle<char16_t>
+  UnsafeBulkWrite(uint32_t aCapacity, nsresult& aRv)
+  {
+    return mData.BulkWrite(aCapacity, UnsafeLength(), false, aRv);
+  }
+
   void
   Append(const nsAString& aString)
   {
@@ -226,22 +232,16 @@ XMLHttpRequestStringWriterHelper::XMLHttpRequestStringWriterHelper(XMLHttpReques
 {
 }
 
-bool
-XMLHttpRequestStringWriterHelper::AddCapacity(int32_t aCapacity)
+uint32_t
+XMLHttpRequestStringWriterHelper::Length() const
 {
-  return mBuffer->UnsafeData().SetCapacity(mBuffer->UnsafeLength() + aCapacity, fallible);
+  return mBuffer->UnsafeLength();
 }
 
-char16_t*
-XMLHttpRequestStringWriterHelper::EndOfExistingData()
+mozilla::BulkWriteHandle<char16_t>
+XMLHttpRequestStringWriterHelper::BulkWrite(uint32_t aCapacity, nsresult& aRv)
 {
-  return mBuffer->UnsafeData().BeginWriting() + mBuffer->UnsafeLength();
-}
-
-void
-XMLHttpRequestStringWriterHelper::AddLength(int32_t aLength)
-{
-  mBuffer->UnsafeData().SetLength(mBuffer->UnsafeLength() + aLength);
+  return mBuffer->UnsafeBulkWrite(aCapacity, aRv);
 }
 
 // ---------------------------------------------------------------------------

dom/xhr/XMLHttpRequestString.h

@@ -64,14 +64,14 @@ class MOZ_STACK_CLASS XMLHttpRequestStringWriterHelper final
 public:
   explicit XMLHttpRequestStringWriterHelper(XMLHttpRequestString& aString);
 
-  bool
-  AddCapacity(int32_t aCapacity);
+  /**
+   * The existing length of the string. Do not call during BulkWrite().
+   */
+  uint32_t
+  Length() const;
 
-  char16_t*
-  EndOfExistingData();
-
-  void
-  AddLength(int32_t aLength);
+  mozilla::BulkWriteHandle<char16_t>
+  BulkWrite(uint32_t aCapacity, nsresult& aRv);
 
 private:
   XMLHttpRequestStringWriterHelper(const XMLHttpRequestStringWriterHelper&) = delete;