From 203072cfe1e84bcecaa69e5b54f950fcb8af01c4 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 19 Sep 2017 16:26:21 +0200 Subject: [PATCH] Bug 1401171 - Make nsIMultiplexInputStream not inherit from nsIInputStream. r=bkelly This is a preexisting issue that makes nsMultiplexInputStream multiple-inherit from nsIInputStream: once via nsIMultipartInputStream and once via nsIAsyncInputStream. This causes problems once we end up with more multiplex streams that are async streams, because then some assingments to nsCOMPtr start asserting. This patch just removes the footgun by getting rid of the multiple inheritance. --- dom/file/MultipartBlobImpl.cpp | 2 +- dom/html/HTMLFormSubmission.cpp | 11 +++++++--- dom/html/HTMLFormSubmission.h | 8 ++++++- dom/network/TCPSocket.cpp | 6 +++-- .../PresentationTCPSessionTransport.cpp | 3 ++- dom/workers/FileReaderSync.cpp | 4 +++- netwerk/protocol/http/nsHttpTransaction.cpp | 3 ++- xpcom/io/nsIMultiplexInputStream.idl | 2 +- xpcom/io/nsMultiplexInputStream.cpp | 6 ++--- xpcom/tests/gtest/TestCloneInputStream.cpp | 12 ++++++---- .../tests/gtest/TestMultiplexInputStream.cpp | 22 ++++++++++--------- 11 files changed, 51 insertions(+), 28 deletions(-) diff --git a/dom/file/MultipartBlobImpl.cpp b/dom/file/MultipartBlobImpl.cpp index 73af16cf145c..3d062f116947 100644 --- a/dom/file/MultipartBlobImpl.cpp +++ b/dom/file/MultipartBlobImpl.cpp @@ -85,7 +85,7 @@ MultipartBlobImpl::GetInternalStream(nsIInputStream** aStream, } } - stream.forget(aStream); + CallQueryInterface(stream, aStream); } already_AddRefed diff --git a/dom/html/HTMLFormSubmission.cpp b/dom/html/HTMLFormSubmission.cpp index 97fec081e15b..a249282cf8e2 100644 --- a/dom/html/HTMLFormSubmission.cpp +++ b/dom/html/HTMLFormSubmission.cpp @@ -394,8 +394,13 @@ FSMultipartFormData::FSMultipartFormData(NotNull aEncoding, nsIContent* aOriginatingElement) : EncodingFormSubmission(aEncoding, aOriginatingElement) { - mPostDataStream = + mPostData = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1"); + + nsCOMPtr inputStream = do_QueryInterface(mPostData); + MOZ_ASSERT(SameCOMIdentity(mPostData, inputStream)); + mPostDataStream = inputStream; + mTotalLength = 0; mBoundary.AssignLiteral("---------------------------"); @@ -600,7 +605,7 @@ FSMultipartFormData::AddDataChunk(const nsACString& aName, // here, since we're about to add the file input stream AddPostDataStream(); - mPostDataStream->AppendStream(aInputStream); + mPostData->AppendStream(aInputStream); mTotalLength += aInputStreamSize; } @@ -640,7 +645,7 @@ FSMultipartFormData::AddPostDataStream() mPostDataChunk); NS_ASSERTION(postDataChunkStream, "Could not open a stream for POST!"); if (postDataChunkStream) { - mPostDataStream->AppendStream(postDataChunkStream); + mPostData->AppendStream(postDataChunkStream); mTotalLength += mPostDataChunk.Length(); } diff --git a/dom/html/HTMLFormSubmission.h b/dom/html/HTMLFormSubmission.h index 138a5214fb7e..7eea8f3924a9 100644 --- a/dom/html/HTMLFormSubmission.h +++ b/dom/html/HTMLFormSubmission.h @@ -193,7 +193,13 @@ private: * chunks--string streams and file streams interleaved to make one big POST * stream. */ - nsCOMPtr mPostDataStream; + nsCOMPtr mPostData; + + /** + * The same stream, but as an nsIInputStream. + * Raw pointers because it is just QI of mInputStream. + */ + nsIInputStream* mPostDataStream; /** * The current string chunk. When a file is hit, the string chunk gets diff --git a/dom/network/TCPSocket.cpp b/dom/network/TCPSocket.cpp index 4403e1fa55d7..d3c4b57939a0 100644 --- a/dom/network/TCPSocket.cpp +++ b/dom/network/TCPSocket.cpp @@ -210,6 +210,7 @@ TCPSocket::CreateStream() mMultiplexStream = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1", &rv); NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr stream = do_QueryInterface(mMultiplexStream); mMultiplexStreamCopier = do_CreateInstance("@mozilla.org/network/async-stream-copier;1", &rv); NS_ENSURE_SUCCESS(rv, rv); @@ -218,7 +219,7 @@ TCPSocket::CreateStream() do_GetService("@mozilla.org/network/socket-transport-service;1"); nsCOMPtr target = do_QueryInterface(sts); - rv = mMultiplexStreamCopier->Init(mMultiplexStream, + rv = mMultiplexStreamCopier->Init(stream, mSocketOutputStream, target, true, /* source buffered */ @@ -596,7 +597,8 @@ TCPSocket::BufferedAmount() } if (mMultiplexStream) { uint64_t available = 0; - mMultiplexStream->Available(&available); + nsCOMPtr stream(do_QueryInterface(mMultiplexStream)); + stream->Available(&available); return available; } return 0; diff --git a/dom/presentation/PresentationTCPSessionTransport.cpp b/dom/presentation/PresentationTCPSessionTransport.cpp index 103d388cddeb..65db9b890b6c 100644 --- a/dom/presentation/PresentationTCPSessionTransport.cpp +++ b/dom/presentation/PresentationTCPSessionTransport.cpp @@ -240,6 +240,7 @@ PresentationTCPSessionTransport::CreateStream() if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + nsCOMPtr stream = do_QueryInterface(mMultiplexStream); mMultiplexStreamCopier = do_CreateInstance("@mozilla.org/network/async-stream-copier;1", &rv); if (NS_WARN_IF(NS_FAILED(rv))) { @@ -253,7 +254,7 @@ PresentationTCPSessionTransport::CreateStream() } nsCOMPtr target = do_QueryInterface(sts); - rv = mMultiplexStreamCopier->Init(mMultiplexStream, + rv = mMultiplexStreamCopier->Init(stream, mSocketOutputStream, target, true, /* source buffered */ diff --git a/dom/workers/FileReaderSync.cpp b/dom/workers/FileReaderSync.cpp index 301f0045232b..2e02d2c92965 100644 --- a/dom/workers/FileReaderSync.cpp +++ b/dom/workers/FileReaderSync.cpp @@ -232,7 +232,9 @@ FileReaderSync::ReadAsText(Blob& aBlob, nsAutoCString charset; encoding->Name(charset); - aRv = ConvertStream(multiplexStream, charset.get(), aResult); + + nsCOMPtr multiplex(do_QueryInterface(multiplexStream)); + aRv = ConvertStream(multiplex, charset.get(), aResult); if (NS_WARN_IF(aRv.Failed())) { return; } diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index d6efcf0641ce..52eb74bd7560 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -382,7 +382,8 @@ nsHttpTransaction::Init(uint32_t caps, // wrap the multiplexed input stream with a buffered input stream, so // that we write data in the largest chunks possible. this is actually // necessary to workaround some common server bugs (see bug 137155). - rv = NS_NewBufferedInputStream(getter_AddRefs(mRequestStream), multi, + nsCOMPtr stream(do_QueryInterface(multi)); + rv = NS_NewBufferedInputStream(getter_AddRefs(mRequestStream), stream, nsIOService::gDefaultSegmentSize); if (NS_FAILED(rv)) return rv; } else { diff --git a/xpcom/io/nsIMultiplexInputStream.idl b/xpcom/io/nsIMultiplexInputStream.idl index d42adcbd40fa..f01d6ecacd39 100644 --- a/xpcom/io/nsIMultiplexInputStream.idl +++ b/xpcom/io/nsIMultiplexInputStream.idl @@ -11,7 +11,7 @@ */ [scriptable, uuid(a076fd12-1dd1-11b2-b19a-d53b5dffaade)] -interface nsIMultiplexInputStream : nsIInputStream +interface nsIMultiplexInputStream : nsISupports { /** * Number of streams in this multiplex-stream diff --git a/xpcom/io/nsMultiplexInputStream.cpp b/xpcom/io/nsMultiplexInputStream.cpp index 646878cdd5d6..f81057821ed3 100644 --- a/xpcom/io/nsMultiplexInputStream.cpp +++ b/xpcom/io/nsMultiplexInputStream.cpp @@ -103,7 +103,7 @@ NS_IMPL_CLASSINFO(nsMultiplexInputStream, nullptr, nsIClassInfo::THREADSAFE, NS_INTERFACE_MAP_BEGIN(nsMultiplexInputStream) NS_INTERFACE_MAP_ENTRY(nsIMultiplexInputStream) - NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIInputStream, nsIMultiplexInputStream) + NS_INTERFACE_MAP_ENTRY(nsIInputStream) NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsISeekableStream, IsSeekable()) NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIIPCSerializableInputStream, IsIPCSerializable()) @@ -349,7 +349,7 @@ nsMultiplexInputStream::ReadSegments(nsWriteSegmentFun aWriter, void* aClosure, nsresult rv = NS_OK; ReadSegmentsState state; - state.mThisStream = static_cast(this); + state.mThisStream = this; state.mOffset = 0; state.mWriter = aWriter; state.mClosure = aClosure; @@ -1048,7 +1048,7 @@ nsMultiplexInputStream::Clone(nsIInputStream** aClone) return NS_ERROR_FAILURE; } - nsCOMPtr clone = new nsMultiplexInputStream(); + RefPtr clone = new nsMultiplexInputStream(); nsresult rv; uint32_t len = mStreams.Length(); diff --git a/xpcom/tests/gtest/TestCloneInputStream.cpp b/xpcom/tests/gtest/TestCloneInputStream.cpp index de4dd5ea3be7..678a7a65b99a 100644 --- a/xpcom/tests/gtest/TestCloneInputStream.cpp +++ b/xpcom/tests/gtest/TestCloneInputStream.cpp @@ -116,8 +116,10 @@ TEST(CloneInputStream, NonCloneableInput_Fallback) TEST(CloneInputStream, CloneMultiplexStream) { - nsCOMPtr stream = + nsCOMPtr multiplexStream = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1"); + ASSERT_TRUE(multiplexStream); + nsCOMPtr stream(do_QueryInterface(multiplexStream)); ASSERT_TRUE(stream); nsTArray inputData; @@ -129,7 +131,7 @@ TEST(CloneInputStream, CloneMultiplexStream) nsresult rv = NS_NewCStringInputStream(getter_AddRefs(base), inputString); ASSERT_TRUE(NS_SUCCEEDED(rv)); - rv = stream->AppendStream(base); + rv = multiplexStream->AppendStream(base); ASSERT_TRUE(NS_SUCCEEDED(rv)); } @@ -156,8 +158,10 @@ TEST(CloneInputStream, CloneMultiplexStream) TEST(CloneInputStream, CloneMultiplexStreamPartial) { - nsCOMPtr stream = + nsCOMPtr multiplexStream = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1"); + ASSERT_TRUE(multiplexStream); + nsCOMPtr stream(do_QueryInterface(multiplexStream)); ASSERT_TRUE(stream); nsTArray inputData; @@ -169,7 +173,7 @@ TEST(CloneInputStream, CloneMultiplexStreamPartial) nsresult rv = NS_NewCStringInputStream(getter_AddRefs(base), inputString); ASSERT_TRUE(NS_SUCCEEDED(rv)); - rv = stream->AppendStream(base); + rv = multiplexStream->AppendStream(base); ASSERT_TRUE(NS_SUCCEEDED(rv)); } diff --git a/xpcom/tests/gtest/TestMultiplexInputStream.cpp b/xpcom/tests/gtest/TestMultiplexInputStream.cpp index ce2a6dca968b..179d612da9f2 100644 --- a/xpcom/tests/gtest/TestMultiplexInputStream.cpp +++ b/xpcom/tests/gtest/TestMultiplexInputStream.cpp @@ -34,6 +34,8 @@ TEST(MultiplexInputStream, Seek_SET) nsCOMPtr multiplexStream = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1"); ASSERT_TRUE(multiplexStream); + nsCOMPtr stream(do_QueryInterface(multiplexStream)); + ASSERT_TRUE(stream); rv = multiplexStream->AppendStream(inputStream1); ASSERT_TRUE(NS_SUCCEEDED(rv)); @@ -51,16 +53,16 @@ TEST(MultiplexInputStream, Seek_SET) // Seek forward in first input stream rv = seekStream->Seek(nsISeekableStream::NS_SEEK_SET, 1); ASSERT_TRUE(NS_SUCCEEDED(rv)); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf1.Length() + buf2.Length() + buf3.Length() - 1, length); // Check read is correct - rv = multiplexStream->Read(readBuf, 3, &count); + rv = stream->Read(readBuf, 3, &count); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)3, count); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf1.Length() + buf2.Length() + buf3.Length() - 4, length); @@ -70,15 +72,15 @@ TEST(MultiplexInputStream, Seek_SET) rv = seekStream->Seek(nsISeekableStream::NS_SEEK_SET, buf1.Length() + buf2.Length()); ASSERT_TRUE(NS_SUCCEEDED(rv)); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf3.Length(), length); // Check read is correct - rv = multiplexStream->Read(readBuf, 5, &count); + rv = stream->Read(readBuf, 5, &count); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)5, count); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf3.Length() - 5, length); ASSERT_EQ(0, strncmp(readBuf, "Foo b", count)); @@ -86,16 +88,16 @@ TEST(MultiplexInputStream, Seek_SET) // Seek back to start of second stream (covers bug 1272371) rv = seekStream->Seek(nsISeekableStream::NS_SEEK_SET, buf1.Length()); ASSERT_TRUE(NS_SUCCEEDED(rv)); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf2.Length() + buf3.Length(), length); // Check read is correct - rv = multiplexStream->Read(readBuf, 6, &count); + rv = stream->Read(readBuf, 6, &count); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)6, count); - rv = multiplexStream->Available(&length); + rv = stream->Available(&length); ASSERT_TRUE(NS_SUCCEEDED(rv)); ASSERT_EQ((uint64_t)buf2.Length() - 6 + buf3.Length(), length); ASSERT_EQ(0, strncmp(readBuf, "The qu", count)); -} \ No newline at end of file +}