Commit 4c75c0c9 authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

[fetch] Enable origin trial with AllowHTTP1ForStreamingUpload property.

This CL enables origin trial flag FetchUploadStreaming.
You can try the feature with
"Chrome --enable-features=FetchUploadStreaming" or an OT token.
Given https://github.com/whatwg/fetch/issues/966#issuecomment-608128154,
this CL also introduces the temporal AllowHTTP1ForStreamingUpload property
to measure upload streaming capability over each protocol.
API explaner: https://bit.ly/2SVvKbR

Bug: 688906
Change-Id: I46ccf37b2268371bb98af50c16b032f2d5fd470a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2174099
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781762}
parent 55f186ee
......@@ -4282,6 +4282,8 @@ test("net_unittests") {
"http/mock_allow_http_auth_preferences.h",
"http/structured_headers_generated_unittest.cc",
"http/structured_headers_unittest.cc",
"http/test_upload_data_stream_not_allow_http1.cc",
"http/test_upload_data_stream_not_allow_http1.h",
"http/transport_security_persister_unittest.cc",
"http/transport_security_state_unittest.cc",
"http/url_security_manager_unittest.cc",
......
......@@ -124,6 +124,9 @@ NET_ERROR(CLEARTEXT_NOT_PERMITTED, -29)
// The request was blocked by a Content Security Policy
NET_ERROR(BLOCKED_BY_CSP, -30)
// The request was blocked because of no H/2 or QUIC session.
NET_ERROR(H2_OR_QUIC_REQUIRED, -31)
// A connection was closed (corresponding to a TCP FIN).
NET_ERROR(CONNECTION_CLOSED, -100)
......
......@@ -190,4 +190,8 @@ UploadProgress UploadDataStream::GetUploadProgress() const {
return UploadProgress(current_position_, total_size_);
}
bool UploadDataStream::AllowHTTP1() const {
return true;
}
} // namespace net
......@@ -95,6 +95,10 @@ class NET_EXPORT UploadDataStream {
// empty UploadProgress.
virtual UploadProgress GetUploadProgress() const;
// Indicates whether fetch upload streaming is allowed/rejected over H/1.
// Even if this is false but there is a QUIC/H2 stream, the upload is allowed.
virtual bool AllowHTTP1() const;
protected:
// Must be called by subclasses when InitInternal and ReadInternal complete
// asynchronously.
......
......@@ -1103,6 +1103,10 @@ int HttpStreamFactory::Job::DoCreateStream() {
->CreateBasicStream(std::move(connection_), using_proxy,
session_->websocket_endpoint_lock_manager());
} else {
if (request_info_.upload_data_stream &&
!request_info_.upload_data_stream->AllowHTTP1()) {
return ERR_H2_OR_QUIC_REQUIRED;
}
stream_ = std::make_unique<HttpBasicStream>(std::move(connection_),
using_proxy);
}
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "net/http/test_upload_data_stream_not_allow_http1.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"
namespace net {
bool UploadDataStreamNotAllowHTTP1::AllowHTTP1() const {
return false;
}
int UploadDataStreamNotAllowHTTP1::InitInternal(const NetLogWithSource&) {
return net::OK;
}
int UploadDataStreamNotAllowHTTP1::ReadInternal(IOBuffer* buf, int buf_len) {
const size_t bytes_to_read =
std::min(content_.length(), static_cast<size_t>(buf_len));
memcpy(buf->data(), content_.c_str(), bytes_to_read);
content_ = content_.substr(bytes_to_read);
if (!content_.length())
SetIsFinalChunk();
return bytes_to_read;
}
void UploadDataStreamNotAllowHTTP1::ResetInternal() {}
} // namespace net
\ No newline at end of file
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef NET_HTTP_TEST_UPLOAD_DATA_STREAM_NOT_ALLOW_HTTP1_H_
#define NET_HTTP_TEST_UPLOAD_DATA_STREAM_NOT_ALLOW_HTTP1_H_
#include "net/base/upload_data_stream.h"
namespace net {
// UploadDataStreamNotAllowHTTP1 simply disallows HTTP/1 and uploads content.
class UploadDataStreamNotAllowHTTP1 : public UploadDataStream {
public:
explicit UploadDataStreamNotAllowHTTP1(const std::string& content)
: UploadDataStream(true, 0), content_(content) {}
UploadDataStreamNotAllowHTTP1(const UploadDataStreamNotAllowHTTP1&) = delete;
UploadDataStreamNotAllowHTTP1& operator=(
const UploadDataStreamNotAllowHTTP1&) = delete;
bool AllowHTTP1() const override;
private:
int InitInternal(const NetLogWithSource& net_log) override;
int ReadInternal(IOBuffer* buf, int buf_len) override;
void ResetInternal() override;
std::string content_;
};
} // namespace net
#endif // NET_HTTP_TEST_UPLOAD_DATA_STREAM_NOT_ALLOW_HTTP1_H_
\ No newline at end of file
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "net/quic/mock_quic_data.h"
#include "net/base/hex_utils.h"
namespace net {
namespace test {
......
......@@ -41,6 +41,7 @@
#include "net/http/http_response_info.h"
#include "net/http/http_server_properties.h"
#include "net/http/http_transaction_test_util.h"
#include "net/http/test_upload_data_stream_not_allow_http1.h"
#include "net/http/transport_security_state.h"
#include "net/log/net_log_event_type.h"
#include "net/log/net_log_with_source.h"
......@@ -10488,4 +10489,30 @@ TEST_F(SpdyNetworkTransactionTest, OnDataSentDoesNotCrashWithGreasedFrameType) {
helper.VerifyDataConsumed();
}
TEST_F(SpdyNetworkTransactionTest, NotAllowHTTP1NotBlockH2Post) {
spdy::SpdySerializedFrame req(
spdy_util_.ConstructChunkedSpdyPost(nullptr, 0));
spdy::SpdySerializedFrame body(spdy_util_.ConstructSpdyDataFrame(1, true));
MockWrite writes[] = {
CreateMockWrite(req, 0), CreateMockWrite(body, 1), // POST upload frame
};
spdy::SpdySerializedFrame resp(spdy_util_.ConstructSpdyPostReply(nullptr, 0));
MockRead reads[] = {
CreateMockRead(resp, 2), CreateMockRead(body, 3),
MockRead(ASYNC, 0, 4) // EOF
};
SequencedSocketData data(reads, writes);
request_.method = "POST";
UploadDataStreamNotAllowHTTP1 upload_data(kUploadData);
request_.upload_data_stream = &upload_data;
NormalSpdyTransactionHelper helper(request_, DEFAULT_PRIORITY, log_, nullptr);
helper.RunToCompletion(&data);
TransactionHelperResult out = helper.output();
EXPECT_THAT(out.rv, IsOk());
EXPECT_EQ("HTTP/1.1 200", out.status_line);
EXPECT_EQ("hello!", out.response_data);
}
} // namespace net
......@@ -33,6 +33,10 @@ ChunkedDataPipeUploadDataStream::ChunkedDataPipeUploadDataStream(
ChunkedDataPipeUploadDataStream::~ChunkedDataPipeUploadDataStream() {}
bool ChunkedDataPipeUploadDataStream::AllowHTTP1() const {
return resource_request_body_->AllowHTTP1ForStreamingUpload();
}
int ChunkedDataPipeUploadDataStream::InitInternal(
const net::NetLogWithSource& net_log) {
// If there was an error either passed to the ReadCallback or as a result of
......
......@@ -43,6 +43,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ChunkedDataPipeUploadDataStream
~ChunkedDataPipeUploadDataStream() override;
bool AllowHTTP1() const override;
private:
// net::UploadDataStream implementation.
int InitInternal(const net::NetLogWithSource& net_log) override;
......
......@@ -80,6 +80,12 @@ class COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequestBody
// support chunked uploads.
void SetToChunkedDataPipe(mojo::PendingRemote<mojom::ChunkedDataPipeGetter>
chunked_data_pipe_getter);
void SetAllowHTTP1ForStreamingUpload(bool allow) {
allow_http1_for_streaming_upload_ = allow;
}
bool AllowHTTP1ForStreamingUpload() const {
return allow_http1_for_streaming_upload_;
}
const std::vector<DataElement>* elements() const { return &elements_; }
std::vector<DataElement>* elements_mutable() { return &elements_; }
......@@ -117,6 +123,8 @@ class COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequestBody
bool contains_sensitive_info_;
bool allow_http1_for_streaming_upload_ = true;
DISALLOW_COPY_AND_ASSIGN(ResourceRequestBody);
};
......
......@@ -242,6 +242,8 @@ bool StructTraits<network::mojom::URLRequestBodyDataView,
return false;
body->set_identifier(data.identifier());
body->set_contains_sensitive_info(data.contains_sensitive_info());
body->SetAllowHTTP1ForStreamingUpload(
data.allow_http1_for_streaming_upload());
*out = std::move(body);
return true;
}
......
......@@ -288,6 +288,11 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE)
return r->contains_sensitive_info_;
}
static bool allow_http1_for_streaming_upload(
const scoped_refptr<network::ResourceRequestBody>& r) {
return r->allow_http1_for_streaming_upload_;
}
static bool Read(network::mojom::URLRequestBodyDataView data,
scoped_refptr<network::ResourceRequestBody>* out);
};
......
......@@ -402,6 +402,10 @@ struct URLRequestBody {
// Indicates whether the post data contains sensitive information like
// passwords.
bool contains_sensitive_info;
// Indicates whether fetch upload streaming is allowed/rejected over H/1.
// Even if this is false but there is a QUIC/H2 stream, the upload is allowed.
bool allow_http1_for_streaming_upload;
};
// Represents part of an upload body. This could be either one of bytes, file or
......
......@@ -749,6 +749,8 @@ void FetchManager::Loader::PerformHTTPFetch(ExceptionState& exception_state) {
if (exception_state.HadException())
return;
request.MutableBody().SetStreamBody(std::move(pending_remote));
request.SetAllowHTTP1ForStreamingUpload(
fetch_request_data_->AllowHTTP1ForStreamingUpload());
}
}
}
......
......@@ -208,6 +208,8 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
request->is_history_navigation_ = is_history_navigation_;
request->window_id_ = window_id_;
request->trust_token_params_ = trust_token_params_;
request->allow_http1_for_streaming_upload_ =
allow_http1_for_streaming_upload_;
return request;
}
......
......@@ -138,6 +138,13 @@ class CORE_EXPORT FetchRequestData final
trust_token_params_ = std::move(trust_token_params);
}
void SetAllowHTTP1ForStreamingUpload(bool allow) {
allow_http1_for_streaming_upload_ = allow;
}
bool AllowHTTP1ForStreamingUpload() const {
return allow_http1_for_streaming_upload_;
}
void Trace(Visitor*) const;
private:
......@@ -183,6 +190,7 @@ class CORE_EXPORT FetchRequestData final
url_loader_factory_;
base::UnguessableToken window_id_;
Member<ExecutionContext> execution_context_;
bool allow_http1_for_streaming_upload_ = false;
DISALLOW_COPY_AND_ASSIGN(FetchRequestData);
};
......
......@@ -558,6 +558,10 @@ Request* Request::CreateRequestWithRequestOrString(
request->SetTrustTokenParams(std::move(params));
}
if (init->hasAllowHTTP1ForStreamingUpload()) {
request->SetAllowHTTP1ForStreamingUpload(
init->allowHTTP1ForStreamingUpload());
}
// "Let |r| be a new Request object associated with |request| and a new
// Headers object whose guard is "request"."
......
......@@ -27,7 +27,7 @@ dictionary RequestInit {
// contexts, this has to be enforced after the fact because the
// SecureContext IDL attribute doesn't affect dictionary members.
[RuntimeEnabled=TrustTokens] TrustToken trustToken;
[RuntimeEnabled=FetchUploadStreaming] boolean allowHTTP1ForStreamingUpload;
// TODO(domfarolino): add support for RequestInit window member.
//any window; // can only be set to null
};
......@@ -462,6 +462,13 @@ class PLATFORM_EXPORT ResourceRequestHead {
// |url|,
bool CanDisplay(const KURL&) const;
void SetAllowHTTP1ForStreamingUpload(bool allow) {
allowHTTP1ForStreamingUpload_ = allow;
}
bool AllowHTTP1ForStreamingUpload() const {
return allowHTTP1ForStreamingUpload_;
}
private:
const CacheControlHeader& GetCacheControlHeader() const;
......@@ -546,6 +553,8 @@ class PLATFORM_EXPORT ResourceRequestHead {
// the prefetch cache will be restricted to top-level-navigations.
bool prefetch_maybe_for_top_level_navigation_ = false;
bool allowHTTP1ForStreamingUpload_ = false;
// This is used when fetching preload header requests from cross-origin
// prefetch responses. The browser process uses this token to ensure the
// request is cached correctly.
......
......@@ -357,6 +357,8 @@ void PopulateResourceRequest(const ResourceRequestHead& src,
mojo::PendingRemote<network::mojom::ChunkedDataPipeGetter>
network_stream_body(stream_body.PassPipe(), 0u);
dest->request_body->SetToChunkedDataPipe(std::move(network_stream_body));
dest->request_body->SetAllowHTTP1ForStreamingUpload(
src.AllowHTTP1ForStreamingUpload());
}
if (resource_type == mojom::ResourceType::kStylesheet) {
......
......@@ -755,7 +755,8 @@
},
{
name: "FetchUploadStreaming",
status: "test",
origin_trial_feature_name: "FetchUploadStreaming",
status: "experimental",
},
{
// Also enabled when blink::features::kFileHandlingAPI is overridden
......
......@@ -10,7 +10,7 @@ PASS Fetch with POST with Float32Array body
PASS Fetch with POST with Float64Array body
PASS Fetch with POST with DataView body
PASS Fetch with POST with Blob body with mime type
FAIL Fetch with POST with ReadableStream assert_equals: expected "Test" but got ""
FAIL Fetch with POST with ReadableStream promise_test: Unhandled rejection with value: object "TypeError: Failed to fetch"
PASS Fetch with POST with ReadableStream containing String
PASS Fetch with POST with ReadableStream containing null
PASS Fetch with POST with ReadableStream containing number
......
......@@ -10,7 +10,7 @@ PASS Fetch with POST with Float32Array body
PASS Fetch with POST with Float64Array body
PASS Fetch with POST with DataView body
PASS Fetch with POST with Blob body with mime type
FAIL Fetch with POST with ReadableStream assert_equals: expected "Test" but got ""
FAIL Fetch with POST with ReadableStream promise_test: Unhandled rejection with value: object "TypeError: Failed to fetch"
PASS Fetch with POST with ReadableStream containing String
PASS Fetch with POST with ReadableStream containing null
PASS Fetch with POST with ReadableStream containing number
......
......@@ -5,11 +5,23 @@ if (self.importScripts) {
var {BASE_ORIGIN, OTHER_ORIGIN} = get_fetch_test_options();
function fetch_echo(stream) {
return fetch(
'/serviceworker/resources/fetch-echo-body.php',
{method: 'POST', body: stream, mode: 'same-origin'});
return fetch('/serviceworker/resources/fetch-echo-body.php', {
method: 'POST',
body: stream,
mode: 'same-origin',
allowHTTP1ForStreamingUpload: true
});
}
promise_test(async (t) => {
await promise_rejects_js(
t, TypeError, fetch('/serviceworker/resources/fetch-echo-body.php', {
method: 'POST',
body: create_stream([new Uint8Array(0)]),
allowHTTP1ForStreamingUpload: false
}));
}, 'AllowHTTP1:false makes fetch failed over HTTP/1.');
function fetch_echo_body(stream) {
return fetch_echo(stream).then(response => response.text());
}
......@@ -103,9 +115,11 @@ function fetch_redirect(status) {
BASE_ORIGIN + '/fetch/resources/fetch-status.php?status=200';
const redirect_original_url = BASE_ORIGIN +
'/serviceworker/resources/redirect.php?Redirect=' + redirect_target_url;
return fetch(
redirect_original_url + `&Status=${status}`,
{method: 'POST', body: create_foo_stream()});
return fetch(redirect_original_url + `&Status=${status}`, {
method: 'POST',
body: create_foo_stream(),
allowHTTP1ForStreamingUpload: true
});
}
promise_test(async (t) => {
......@@ -126,9 +140,12 @@ promise_test(async () => {
promise_test(async (t) => {
await promise_rejects_js(
t, TypeError,
fetch(
'/serviceworker/resources/fetch-access-control.php?AuthFail',
{method: 'POST', body: create_foo_stream(), mode: 'same-origin'}));
fetch('/serviceworker/resources/fetch-access-control.php?AuthFail', {
method: 'POST',
body: create_foo_stream(),
mode: 'same-origin',
allowHTTP1ForStreamingUpload: true
}));
}, 'Upload streaming with 401 Unauthorized response should fail.');
function report(content) {
......@@ -144,7 +161,8 @@ promise_test(async () => {
const response_text = await fetch(request_url, {
method: 'POST',
body: 'content a',
mode: 'cors'
mode: 'cors',
allowHTTP1ForStreamingUpload: true
}).then(r => r.text());
const report_json = eval(response_text);
assert_false(report_json['did_preflight'], 'Should not trigger preflight');
......@@ -160,6 +178,7 @@ promise_test(async () => {
method: 'POST',
body: create_foo_stream(),
mode: 'cors',
allowHTTP1ForStreamingUpload: true
}).then(r => r.text());
const report_json = eval(response_text);
assert_true(report_json['did_preflight'], 'Should preflight');
......
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