Commit 9f90c933 authored by Hoch Hochkeppel's avatar Hoch Hochkeppel Committed by Commit Bot

Postpone MaxSharedFileBytes check

Removing the check of the size of the shared files from
share_service_impl, but leaving the unit tests prepped to cover these
cases to validate that the platform-specific implementations check this
when they are processing the shared files.

The scenario being fixed with this change is sharing native files,
easily exercised with the Web Share Test page:
https://w3c.github.io/web-share/demos/share-files.html

When these files are initially created as blobs they are marked as being
of unknown size, and though the navigator_share code does check their
size as part of the Share operation, this check does not modify the
actual blob, so when it is serialized the recorded size will be the max
uint64 value. Matching the Android implementation, this postpones the
file size check till the files are actually being processed (in the
platform-specific implementations).

Bug: 1035527
Change-Id: I59ea9bfadb06205ae155f31947fa04093f06503d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2415431Reviewed-by: default avatarEric Willigers <ericwilligers@chromium.org>
Commit-Queue: Hoch Hochkeppel <mhochk@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#807765}
parent 48d76d3d
...@@ -133,7 +133,6 @@ void ShareServiceImpl::Share(const std::string& title, ...@@ -133,7 +133,6 @@ void ShareServiceImpl::Share(const std::string& title,
return; return;
} }
uint64_t total_bytes = 0;
for (auto& file : files) { for (auto& file : files) {
if (!file || !file->blob) { if (!file || !file->blob) {
mojo::ReportBadMessage("Invalid file to share()"); mojo::ReportBadMessage("Invalid file to share()");
...@@ -146,12 +145,11 @@ void ShareServiceImpl::Share(const std::string& title, ...@@ -146,12 +145,11 @@ void ShareServiceImpl::Share(const std::string& title,
return; return;
} }
total_bytes += file->blob->size; // In the case where the original blob handle was to a native file (of
} // unknown size), the serialized data does not contain an accurate file
// size. To handle this, the comparison against kMaxSharedFileBytes should
if (total_bytes > kMaxSharedFileBytes) { // be done by the platform-specific implementations as part of processing
std::move(callback).Run(blink::mojom::ShareError::PERMISSION_DENIED); // the blobs.
return;
} }
// TODO(crbug.com/1035527): Add implementation for OS_WIN // TODO(crbug.com/1035527): Add implementation for OS_WIN
......
...@@ -79,7 +79,7 @@ TEST_F(ShareServiceUnitTest, TotalBytes) { ...@@ -79,7 +79,7 @@ TEST_F(ShareServiceUnitTest, TotalBytes) {
kMaxSharedFileBytes / kMaxSharedFileCount, kMaxSharedFileBytes / kMaxSharedFileCount,
kMaxSharedFileCount)); kMaxSharedFileCount));
EXPECT_EQ( EXPECT_EQ(
ShareError::PERMISSION_DENIED, ShareError::CANCELED,
ShareGeneratedFileData(".txt", "text/plain", ShareGeneratedFileData(".txt", "text/plain",
(kMaxSharedFileBytes / kMaxSharedFileCount) + 1, (kMaxSharedFileBytes / kMaxSharedFileCount) + 1,
kMaxSharedFileCount)); kMaxSharedFileCount));
...@@ -89,7 +89,7 @@ TEST_F(ShareServiceUnitTest, FileBytes) { ...@@ -89,7 +89,7 @@ TEST_F(ShareServiceUnitTest, FileBytes) {
EXPECT_EQ(ShareError::CANCELED, EXPECT_EQ(ShareError::CANCELED,
ShareGeneratedFileData(".txt", "text/plain", kMaxSharedFileBytes)); ShareGeneratedFileData(".txt", "text/plain", kMaxSharedFileBytes));
EXPECT_EQ( EXPECT_EQ(
ShareError::PERMISSION_DENIED, ShareError::CANCELED,
ShareGeneratedFileData(".txt", "text/plain", kMaxSharedFileBytes + 1)); ShareGeneratedFileData(".txt", "text/plain", kMaxSharedFileBytes + 1));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment