|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Marijn Kruisselbrink <mek@chromium.org> |
| 3 | +Date: Tue, 21 Apr 2020 23:51:25 +0000 |
| 4 | +Subject: Fix bug when BytesProvider replies with invalid data. |
| 5 | + |
| 6 | +Bug: 1072983 |
| 7 | +Change-Id: Ideaa0a67680375e770995880a4b8d2014b51d642 |
| 8 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2159583 |
| 9 | +Reviewed-by: enne <enne@chromium.org> |
| 10 | +Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> |
| 11 | +Cr-Commit-Position: refs/heads/master@{#761203} |
| 12 | + |
| 13 | +diff --git a/storage/browser/blob/blob_registry_impl.cc b/storage/browser/blob/blob_registry_impl.cc |
| 14 | +index f3ae0c6a94277796d710a6c4c6c088f56a80bae0..2c70ceeb8dc25e03644fd1c14a7da8999a93bd4e 100644 |
| 15 | +--- a/storage/browser/blob/blob_registry_impl.cc |
| 16 | ++++ b/storage/browser/blob/blob_registry_impl.cc |
| 17 | +@@ -430,6 +430,10 @@ void BlobRegistryImpl::BlobUnderConstruction::TransportComplete( |
| 18 | + // try to delete |this| again afterwards. |
| 19 | + auto weak_this = weak_ptr_factory_.GetWeakPtr(); |
| 20 | + |
| 21 | ++ // Store the bad_message_callback_, so we can invoke it if needed, after |
| 22 | ++ // notifying about the blob being finished. |
| 23 | ++ auto bad_message_callback = std::move(bad_message_callback_); |
| 24 | ++ |
| 25 | + // The blob might no longer have any references, in which case it may no |
| 26 | + // longer exist. If that happens just skip calling Complete. |
| 27 | + // TODO(mek): Stop building sooner if a blob is no longer referenced. |
| 28 | +@@ -443,7 +447,7 @@ void BlobRegistryImpl::BlobUnderConstruction::TransportComplete( |
| 29 | + // BlobTransportStrategy might have already reported a BadMessage on the |
| 30 | + // BytesProvider binding, but just to be safe, also report one on the |
| 31 | + // BlobRegistry binding itself. |
| 32 | +- std::move(bad_message_callback_) |
| 33 | ++ std::move(bad_message_callback) |
| 34 | + .Run("Received invalid data while transporting blob"); |
| 35 | + } |
| 36 | + if (weak_this) |
| 37 | +diff --git a/storage/browser/blob/blob_registry_impl_unittest.cc b/storage/browser/blob/blob_registry_impl_unittest.cc |
| 38 | +index 62a8b5664b0300240f289eafb2f18c0a1f7c9fcc..eb65111ae8853eee54ec6b5e5cc419cd53bb6ed9 100644 |
| 39 | +--- a/storage/browser/blob/blob_registry_impl_unittest.cc |
| 40 | ++++ b/storage/browser/blob/blob_registry_impl_unittest.cc |
| 41 | +@@ -769,6 +769,36 @@ TEST_F(BlobRegistryImplTest, Register_ValidBytesAsReply) { |
| 42 | + EXPECT_EQ(0u, BlobsUnderConstruction()); |
| 43 | + } |
| 44 | + |
| 45 | ++TEST_F(BlobRegistryImplTest, Register_InvalidBytesAsReply) { |
| 46 | ++ const std::string kId = "id"; |
| 47 | ++ const std::string kData = "hello"; |
| 48 | ++ |
| 49 | ++ std::vector<blink::mojom::DataElementPtr> elements; |
| 50 | ++ elements.push_back( |
| 51 | ++ blink::mojom::DataElement::NewBytes(blink::mojom::DataElementBytes::New( |
| 52 | ++ kData.size(), base::nullopt, CreateBytesProvider("")))); |
| 53 | ++ |
| 54 | ++ mojo::PendingRemote<blink::mojom::Blob> blob; |
| 55 | ++ EXPECT_TRUE(registry_->Register(blob.InitWithNewPipeAndPassReceiver(), kId, |
| 56 | ++ "", "", std::move(elements))); |
| 57 | ++ |
| 58 | ++ std::unique_ptr<BlobDataHandle> handle = context_->GetBlobDataFromUUID(kId); |
| 59 | ++ WaitForBlobCompletion(handle.get()); |
| 60 | ++ |
| 61 | ++ EXPECT_TRUE(handle->IsBroken()); |
| 62 | ++ ASSERT_EQ(BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS, |
| 63 | ++ handle->GetBlobStatus()); |
| 64 | ++ |
| 65 | ++ EXPECT_EQ(1u, reply_request_count_); |
| 66 | ++ EXPECT_EQ(0u, stream_request_count_); |
| 67 | ++ EXPECT_EQ(0u, file_request_count_); |
| 68 | ++ EXPECT_EQ(0u, BlobsUnderConstruction()); |
| 69 | ++ |
| 70 | ++ // Expect 2 bad messages, one for the bad reply by the bytes provider, and one |
| 71 | ++ // for the original register call. |
| 72 | ++ EXPECT_EQ(2u, bad_messages_.size()); |
| 73 | ++} |
| 74 | ++ |
| 75 | + TEST_F(BlobRegistryImplTest, Register_ValidBytesAsStream) { |
| 76 | + const std::string kId = "id"; |
| 77 | + const std::string kData = |
0 commit comments