Commit 580b4792 authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

Handle non-Latin-1 file paths when converting to FormDataElement.

When uploading form data in a subresource request that goes through a
service worker, ServiceWorkerSubresourceLoader::DispatchFetchEvent()
converts network::ResourceRequest(=network.mojom.URLRequest) to
blink::mojom::FetchAPIRequest which means the body is converted from
network::ResourceRequestBody(=network.mojom.URLRequestBody) to
blink::mojom::FetchAPIRequestBody, which means each
body element is converted from
network::DataElement(=network.mojom.DataElement) to
blink::FormDataElement(=blink.mojom.FetchAPIDataElement).

The conversion of files was incorrectly assuming Latin-1 file paths. Use
blink::FilePathToString() instead for less breakage.

Bug: 1017184
Change-Id: Iabf96f3dda129f60bb03fbe98dc33d89f0d0bb63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1888855Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711628}
parent 761f6f59
......@@ -190,6 +190,37 @@ class ServiceWorkerFileUploadTest : public ContentBrowserTest {
EXPECT_THAT(result, ::testing::HasSubstr("form-data; name=\"file\""));
}
void RunSubresourceTest(const base::FilePath& file_path,
std::string* out_result) {
// Install the service worker.
EXPECT_TRUE(NavigateToURL(
shell(), embedded_test_server()->GetURL(
"/service_worker/create_service_worker.html")));
EXPECT_EQ("DONE", EvalJs(shell(), "register('file_upload_worker.js');"));
// Generate the URL for the page with the file upload form. It will upload
// to |target_url|.
GURL page_url = embedded_test_server()->GetURL("/service_worker/form.html");
GURL target_url = BuildTargetUrl("/service_worker/upload", "getAs=text");
page_url = net::AppendQueryParameter(page_url, "target", target_url.spec());
// Navigate to the page with a file upload form.
EXPECT_TRUE(NavigateToURL(shell(), page_url));
// Fill out the form to refer to the test file.
base::RunLoop run_loop;
auto delegate = std::make_unique<FileChooserDelegate>(
file_path, run_loop.QuitClosure());
shell()->web_contents()->SetDelegate(delegate.get());
EXPECT_TRUE(ExecJs(shell(), "fileInput.click();"));
run_loop.Run();
// Submit the form using XHR.
EvalJsResult result = EvalJs(shell(), "submitXhr()");
ASSERT_TRUE(result.error.empty());
*out_result = result.ExtractString();
}
std::string BuildExpectedBodyAsText(const std::string& boundary,
const std::string& filename) {
return "--" + boundary + "\r\n" +
......@@ -305,4 +336,49 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerFileUploadTest,
RunNetworkFallbackTest(TargetOrigin::kCrossOrigin);
}
// Tests a subresource request.
IN_PROC_BROWSER_TEST_F(ServiceWorkerFileUploadTest, Subresource) {
// Prepare a file for the upload form.
base::ScopedAllowBlockingForTesting allow_blocking;
base::ScopedTempDir temp_dir;
base::FilePath file_path;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir.GetPath(), &file_path));
ASSERT_EQ(static_cast<int>(kFileSize),
base::WriteFile(file_path, kFileContent, kFileSize));
std::string result;
RunSubresourceTest(file_path, &result);
// Test that the file name and contents are present.
EXPECT_THAT(result,
::testing::HasSubstr(file_path.BaseName().MaybeAsASCII()));
EXPECT_THAT(result, ::testing::HasSubstr(kFileContent));
}
// Tests a subresource request where the filename is non-ascii. Regression test
// for https://crbug.com/1017184.
IN_PROC_BROWSER_TEST_F(ServiceWorkerFileUploadTest,
Subresource_NonAsciiFilename) {
// "こんにちは"
const base::FilePath::CharType nonAsciiFilename[] =
FILE_PATH_LITERAL("\u3053\u3093\u306B\u3061\u306F");
// Prepare a file for the upload form.
base::ScopedAllowBlockingForTesting allow_blocking;
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::FilePath file_path = temp_dir.GetPath().Append(nonAsciiFilename);
ASSERT_EQ(static_cast<int>(kFileSize),
base::WriteFile(file_path, kFileContent, kFileSize));
std::string result;
RunSubresourceTest(file_path, &result);
// Test that the file name and contents are present. Repeat "こんにちは" here
// since HasSubstr() doesn't work with FilePath::CharType on Windows.
EXPECT_THAT(result, ::testing::HasSubstr("\u3053\u3093\u306B\u3061\u306F"));
EXPECT_THAT(result, ::testing::HasSubstr(kFileContent));
}
} // namespace content
......@@ -14,6 +14,23 @@
<input id="file" type="file" name="file">
</form>
<script>
// Submits using XHR. Used for the subresource test.
async function submitXhr() {
const xhr = new XMLHttpRequest();
const loaded = new Promise(resolve => {
xhr.onload = resolve;
});
const loadEnded = new Promise(resolve => {
xhr.onloadend = resolve;
});
xhr.open("post", form.action);
xhr.send(new FormData(form));
await loaded;
await loadEnded;
return xhr.responseText;
}
const fileInput = document.querySelector('#file');
const form = document.querySelector('#form');
......
......@@ -17,6 +17,7 @@
#include "services/network/public/mojom/data_pipe_getter.mojom-blink.h"
#include "third_party/blink/public/mojom/blob/blob.mojom-blink.h"
#include "third_party/blink/public/mojom/blob/blob_registry.mojom-blink.h"
#include "third_party/blink/public/platform/file_path_conversion.h"
#include "third_party/blink/public/platform/interface_provider.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/platform/network/wrapped_data_pipe_getter.h"
......@@ -137,8 +138,7 @@ bool StructTraits<blink::mojom::FetchAPIDataElementDataView,
return false;
}
out->expected_file_modification_time_ = expected_time.ToDoubleT();
out->filename_ =
WTF::String(file_path.value().data(), file_path.value().size());
out->filename_ = blink::FilePathToString(file_path);
break;
}
case network::mojom::DataElementType::kDataPipe: {
......
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