Skip to content

Commit cda6cf7

Browse files
mingycChromium LUCI CQ
authored andcommitted
[fetch-later] Allow fetchLater() to be used with HTTP localhost URLs.
The [current spec][1] says ``` If request’s URL’s scheme is not an HTTP(S) scheme, then throw a TypeError. ``` Also, [previous spec discussion][2] concludes that http localhost should be supported in addition to https urls. [1]: https://fetch.spec.whatwg.org/#dom-window-fetchlater [2]: whatwg/fetch#1647 (comment) Bug: 440277813 Change-Id: I8a3257570ad8fbde5b2e7243fb024b7690d5d733 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6894278 Commit-Queue: Ming-Ying Chung <[email protected]> Reviewed-by: Andrew Paseltiner <[email protected]> Reviewed-by: Nidhi Jaju <[email protected]> Cr-Commit-Position: refs/heads/main@{#1508981}
1 parent d98dca4 commit cda6cf7

File tree

3 files changed

+237
-36
lines changed

3 files changed

+237
-36
lines changed

third_party/blink/renderer/core/fetch/fetch_manager.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,10 +1675,11 @@ FetchLaterResult* FetchLaterManager::FetchLater(
16751675
return nullptr;
16761676
}
16771677

1678-
// 8. If request’s URL’s scheme is not an HTTPS scheme, then throw a
1678+
// 8. If request’s URL’s scheme is not an HTTP(S) scheme, then throw a
16791679
// TypeError.
1680-
if (!request->Url().ProtocolIs(g_https_atom)) {
1681-
exception_state.ThrowTypeError("fetchLater is only supported over HTTPS.");
1680+
if (!request->Url().ProtocolIsInHTTPFamily()) {
1681+
exception_state.ThrowTypeError(
1682+
"fetchLater is only supported over HTTP(S).");
16821683
return nullptr;
16831684
}
16841685
// 9. If request’s URL is not a potentially trustworthy url, then throw a

third_party/blink/renderer/core/fetch/fetch_manager_test.cc

Lines changed: 153 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
#include "third_party/blink/renderer/core/fetch/fetch_manager.h"
66

77
#include <optional>
8+
#include <string>
9+
#include <string_view>
10+
#include <utility>
811

912
#include "base/memory/scoped_refptr.h"
1013
#include "base/strings/strcat.h"
@@ -56,43 +59,43 @@ using ::testing::Eq;
5659
using ::testing::IsNull;
5760
using ::testing::Not;
5861

59-
MATCHER_P(HasRangeError,
60-
expected_message,
61-
base::StrCat({"has ", negation ? "no " : "", "RangeError('",
62-
expected_message, "')"})) {
62+
MATCHER_P2(HasException,
63+
error_name,
64+
expected_message,
65+
base::StrCat({"has ", negation ? "no " : "", error_name, "('",
66+
expected_message, "')"})) {
6367
const DummyExceptionStateForTesting& exception_state = arg;
6468
if (!exception_state.HadException()) {
6569
*result_listener << "no exception";
6670
return false;
6771
}
68-
if (exception_state.CodeAs<ESErrorType>() != ESErrorType::kRangeError) {
69-
*result_listener << "exception is not RangeError";
70-
return false;
71-
}
72-
if (exception_state.Message() != expected_message) {
73-
*result_listener << "unexpected message from RangeError: "
74-
<< exception_state.Message();
75-
return false;
76-
}
77-
return true;
78-
}
7972

80-
MATCHER_P(HasAbortError,
81-
expected_message,
82-
base::StrCat({"has ", negation ? "no " : "", "AbortError('",
83-
expected_message, "')"})) {
84-
const DummyExceptionStateForTesting& exception_state = arg;
85-
if (!exception_state.HadException()) {
86-
*result_listener << "no exception";
73+
bool type_matches = false;
74+
const std::string_view err(error_name);
75+
if (err == "RangeError") {
76+
type_matches =
77+
exception_state.CodeAs<ESErrorType>() == ESErrorType::kRangeError;
78+
} else if (err == "AbortError") {
79+
type_matches = exception_state.CodeAs<DOMExceptionCode>() ==
80+
DOMExceptionCode::kAbortError;
81+
} else if (err == "TypeError") {
82+
type_matches =
83+
exception_state.CodeAs<ESErrorType>() == ESErrorType::kTypeError;
84+
} else if (err == "SecurityError") {
85+
type_matches = exception_state.CodeAs<DOMExceptionCode>() ==
86+
DOMExceptionCode::kSecurityError;
87+
} else {
88+
*result_listener << "unsupported error name in matcher: " << error_name;
8789
return false;
8890
}
89-
if (exception_state.CodeAs<DOMExceptionCode>() !=
90-
DOMExceptionCode::kAbortError) {
91-
*result_listener << "exception is not AbortError";
91+
92+
if (!type_matches) {
93+
*result_listener << "exception is not " << error_name;
9294
return false;
9395
}
96+
9497
if (exception_state.Message() != expected_message) {
95-
*result_listener << "unexpected message from AbortError: "
98+
*result_listener << "unexpected message from " << error_name << ": "
9699
<< exception_state.Message();
97100
return false;
98101
}
@@ -186,9 +189,9 @@ MATCHER_P(MatchNetworkResourceRequest,
186189

187190
} // namespace
188191

189-
class FetchLaterTest : public testing::Test {
192+
class FetchLaterTestBase : public testing::Test {
190193
public:
191-
FetchLaterTest()
194+
FetchLaterTestBase()
192195
: task_runner_(base::MakeRefCounted<base::TestMockTimeTaskRunner>()) {
193196
feature_list_.InitAndEnableFeature(blink::features::kFetchLaterAPI);
194197
}
@@ -261,6 +264,8 @@ class FetchLaterTest : public testing::Test {
261264
base::HistogramTester histogram_;
262265
};
263266

267+
class FetchLaterTest : public FetchLaterTestBase {};
268+
264269
// A FetchLater request where its URL has same-origin as its execution context.
265270
TEST_F(FetchLaterTest, CreateSameOriginFetchLaterRequest) {
266271
FetchLaterTestingScope scope(FrameClient(), GetSourcePageURL());
@@ -317,7 +322,8 @@ TEST_F(FetchLaterTest, NegativeActivateAfterThrowRangeError) {
317322

318323
EXPECT_THAT(result, IsNull());
319324
EXPECT_THAT(exception_state,
320-
HasRangeError("fetchLater's activateAfter cannot be negative."));
325+
HasException("RangeError",
326+
"fetchLater's activateAfter cannot be negative."));
321327
EXPECT_EQ(fetch_later_manager->NumLoadersForTesting(), 0u);
322328
Histogram().ExpectTotalCount("FetchLater.Renderer.Total", 0);
323329
}
@@ -345,8 +351,9 @@ TEST_F(FetchLaterTest, AbortBeforeFetchLater) {
345351
request->signal(), /*activate_after_ms=*/std::nullopt, exception_state);
346352

347353
EXPECT_THAT(result, IsNull());
348-
EXPECT_THAT(exception_state,
349-
HasAbortError("The user aborted a fetchLater request."));
354+
EXPECT_THAT(
355+
exception_state,
356+
HasException("AbortError", "The user aborted a fetchLater request."));
350357
EXPECT_EQ(fetch_later_manager->NumLoadersForTesting(), 0u);
351358
Histogram().ExpectTotalCount("FetchLater.Renderer.Total", 0);
352359
}
@@ -495,5 +502,120 @@ TEST_F(FetchLaterTest, ForcedSendingWithBackgroundSyncOff) {
495502
3 /*kActivatedOnEnteredBackForwardCache*/, 1);
496503
}
497504

505+
// Base class for fetchLater() URL validation tests.
506+
class FetchLaterUrlTestBase : public FetchLaterTestBase {
507+
protected:
508+
std::pair<Persistent<FetchLaterManager>, Persistent<FetchLaterResult>>
509+
CallFetchLater(V8TestingScope& scope, const std::string& url) {
510+
auto* manager =
511+
MakeGarbageCollected<FetchLaterManager>(scope.GetExecutionContext());
512+
auto* controller = AbortController::Create(scope.GetScriptState());
513+
auto& exception_state = scope.GetExceptionState();
514+
auto* request = CreateFetchLaterRequest(scope, String::FromUTF8(url),
515+
controller->signal());
516+
auto* result = manager->FetchLater(
517+
scope.GetScriptState(),
518+
request->PassRequestData(scope.GetScriptState(), exception_state),
519+
controller->signal(), /*activate_after=*/std::nullopt, exception_state);
520+
return {manager, result};
521+
}
522+
};
523+
524+
struct UrlTestParam {
525+
const std::string test_name;
526+
const std::string url;
527+
};
528+
529+
// This test verifies that fetchLater() only accepts URLs with HTTP or HTTPS
530+
// schemes. It covers various valid and invalid URL schemes, including http(s),
531+
// localhost, IP addresses, and non-http schemes like data:, file:, etc.
532+
class FetchLaterWithValidUrlTest
533+
: public FetchLaterUrlTestBase,
534+
public testing::WithParamInterface<UrlTestParam> {};
535+
536+
INSTANTIATE_TEST_SUITE_P(All,
537+
FetchLaterWithValidUrlTest,
538+
testing::ValuesIn(std::vector<UrlTestParam>{
539+
{"https_example", "https://example.com/"},
540+
{"http_localhost", "http://localhost/"},
541+
{"https_localhost", "https://localhost/"},
542+
{"http_127_0_0_1", "http://127.0.0.1/"},
543+
{"https_127_0_0_1", "https://127.0.0.1/"},
544+
{"http_ipv6_localhost", "http://[::1]/"},
545+
{"https_ipv6_localhost", "https://[::1]/"},
546+
}),
547+
[](const testing::TestParamInfo<UrlTestParam>& info) {
548+
return info.param.test_name;
549+
});
550+
551+
// Verifies that fetchLater() succeeds with valid URLs, including HTTPS URLs and
552+
// potentially trustworthy HTTP URLs like localhost.
553+
TEST_P(FetchLaterWithValidUrlTest, Succeeds) {
554+
FetchLaterTestingScope scope(FrameClient(), GetSourcePageURL());
555+
556+
auto [manager, result] = CallFetchLater(scope, GetParam().url);
557+
558+
EXPECT_THAT(result, Not(IsNull()));
559+
EXPECT_FALSE(scope.GetExceptionState().HadException());
560+
EXPECT_EQ(manager->NumLoadersForTesting(), 1u);
561+
}
562+
563+
class FetchLaterWithInsecureUrlTest
564+
: public FetchLaterUrlTestBase,
565+
public testing::WithParamInterface<UrlTestParam> {};
566+
567+
INSTANTIATE_TEST_SUITE_P(All,
568+
FetchLaterWithInsecureUrlTest,
569+
testing::ValuesIn(std::vector<UrlTestParam>{
570+
{"http_example", "http://example.com/"},
571+
}),
572+
[](const testing::TestParamInfo<UrlTestParam>& info) {
573+
return info.param.test_name;
574+
});
575+
576+
// Verifies that fetchLater() throws a SecurityError for insecure URLs, such as
577+
// an HTTP URL that is not localhost.
578+
TEST_P(FetchLaterWithInsecureUrlTest, FailsWithSecurityError) {
579+
FetchLaterTestingScope scope(FrameClient(), GetSourcePageURL());
580+
581+
auto [manager, result] = CallFetchLater(scope, GetParam().url);
582+
583+
EXPECT_THAT(result, IsNull());
584+
EXPECT_THAT(
585+
scope.GetExceptionState(),
586+
HasException("SecurityError", "fetchLater was passed an insecure URL."));
587+
EXPECT_EQ(manager->NumLoadersForTesting(), 0u);
588+
}
589+
590+
class FetchLaterWithInvalidSchemeUrlTest
591+
: public FetchLaterUrlTestBase,
592+
public testing::WithParamInterface<UrlTestParam> {};
593+
594+
INSTANTIATE_TEST_SUITE_P(All,
595+
FetchLaterWithInvalidSchemeUrlTest,
596+
testing::ValuesIn(std::vector<UrlTestParam>{
597+
{"data", "data:text/plain,Hello"},
598+
{"file", "file:///etc/passwd"},
599+
{"ftp", "ftp://example.com/"},
600+
{"javascript", "javascript:alert(1)"},
601+
{"blob", "blob:https://example.com/some-uuid"},
602+
}),
603+
[](const testing::TestParamInfo<UrlTestParam>& info) {
604+
return info.param.test_name;
605+
});
606+
607+
// Verifies that fetchLater() throws a TypeError for URLs with schemes other
608+
// than HTTP or HTTPS.
609+
TEST_P(FetchLaterWithInvalidSchemeUrlTest, FailsWithTypeError) {
610+
FetchLaterTestingScope scope(FrameClient(), GetSourcePageURL());
611+
612+
auto [manager, result] = CallFetchLater(scope, GetParam().url);
613+
614+
EXPECT_THAT(result, IsNull());
615+
EXPECT_THAT(
616+
scope.GetExceptionState(),
617+
HasException("TypeError", "fetchLater is only supported over HTTP(S)."));
618+
EXPECT_EQ(manager->NumLoadersForTesting(), 0u);
619+
}
498620

499621
} // namespace blink

third_party/blink/web_tests/external/wpt/fetch/fetch-later/basic.tentative.https.window.js

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,91 @@ test(() => {
55
}, `fetchLater() cannot be called without request.`);
66

77
test(() => {
8-
assert_throws_js(TypeError, () => fetchLater('http://www.google.com'));
8+
const result = fetchLater('/');
9+
assert_false(result.activated, `result.activated should be false for '/'`);
10+
}, `fetchLater() with same-origin (https) URL does not throw.`);
11+
12+
test(() => {
13+
const url = 'http://localhost';
14+
const result = fetchLater(url);
15+
assert_false(result.activated, `result.activated should be false for ${url}`);
16+
}, `fetchLater() with http://localhost URL does not throw.`);
17+
18+
test(() => {
19+
const url = 'https://localhost';
20+
const result = fetchLater(url);
21+
assert_false(result.activated, `result.activated should be false for ${url}`);
22+
}, `fetchLater() with https://localhost URL does not throw.`);
23+
24+
test(() => {
25+
const url = 'http://127.0.0.1';
26+
const result = fetchLater(url);
27+
assert_false(result.activated, `result.activated should be false for ${url}`);
28+
}, `fetchLater() with http://127.0.0.1 URL does not throw.`);
29+
30+
test(() => {
31+
const url = 'https://127.0.0.1';
32+
const result = fetchLater(url);
33+
assert_false(result.activated, `result.activated should be false for ${url}`);
34+
}, `fetchLater() with https://127.0.0.1 URL does not throw.`);
35+
36+
test(() => {
37+
const url = 'http://[::1]';
38+
const result = fetchLater(url);
39+
assert_false(result.activated, `result.activated should be false for ${url}`);
40+
}, `fetchLater() with http://[::1] URL does not throw.`);
41+
42+
test(() => {
43+
const url = 'https://[::1]';
44+
const result = fetchLater(url);
45+
assert_false(result.activated, `result.activated should be false for ${url}`);
46+
}, `fetchLater() with https://[::1] URL does not throw.`);
47+
48+
test(() => {
49+
const url = 'https://example.com';
50+
const result = fetchLater(url);
51+
assert_false(result.activated, `result.activated should be false for ${url}`);
52+
}, `fetchLater() with https://example.com URL does not throw.`);
53+
54+
test(() => {
55+
const httpUrl = 'http://example.com';
56+
assert_throws_dom(
57+
'SecurityError', () => fetchLater(httpUrl),
58+
`should throw SecurityError for insecure http url ${httpUrl}`);
59+
}, `fetchLater() throws SecurityError on non-trustworthy http URL.`);
60+
61+
test(() => {
962
assert_throws_js(TypeError, () => fetchLater('file://tmp'));
63+
}, `fetchLater() throws TypeError on file:// scheme.`);
64+
65+
test(() => {
66+
assert_throws_js(TypeError, () => fetchLater('ftp://example.com'));
67+
}, `fetchLater() throws TypeError on ftp:// scheme.`);
68+
69+
test(() => {
1070
assert_throws_js(TypeError, () => fetchLater('ssh://example.com'));
71+
}, `fetchLater() throws TypeError on ssh:// scheme.`);
72+
73+
test(() => {
1174
assert_throws_js(TypeError, () => fetchLater('wss://example.com'));
75+
}, `fetchLater() throws TypeError on wss:// scheme.`);
76+
77+
test(() => {
1278
assert_throws_js(TypeError, () => fetchLater('about:blank'));
79+
}, `fetchLater() throws TypeError on about: scheme.`);
80+
81+
test(() => {
1382
assert_throws_js(TypeError, () => fetchLater(`javascript:alert('');`));
14-
}, `fetchLater() throws TypeError on non-HTTPS URL.`);
83+
}, `fetchLater() throws TypeError on javascript: scheme.`);
84+
85+
test(() => {
86+
assert_throws_js(TypeError, () => fetchLater('data:text/plain,Hello'));
87+
}, `fetchLater() throws TypeError on data: scheme.`);
88+
89+
test(() => {
90+
assert_throws_js(
91+
TypeError, () => fetchLater('blob:https://example.com/some-uuid'));
92+
}, `fetchLater() throws TypeError on blob: scheme.`);
1593

1694
test(() => {
1795
assert_throws_js(

0 commit comments

Comments
 (0)