Commit 8e47d509 authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

Expose kv-storage built-in module only on SecureContexts

This CL adds actual restriction in Blink implementation.

This CL also modifies tests and expectations to match with
the current Blink built-in/import maps infra.

A subtest in WPT test `import-statement.html` is failing
because the current Blink implementation is based on
pre-import-map spec and thus
importing unavailable built-in causes fetch error.
See 'import 'std:nonexistent';' row of
https://github.com/WICG/import-maps/issues/159.

Bug: 977470
Change-Id: I107f60ed95e1f91d62f468c29c5d741eef13f4f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1702754
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691821}
parent 30bff49e
...@@ -54,6 +54,14 @@ bool ModulatorImplBase::BuiltInModuleInfraEnabled() const { ...@@ -54,6 +54,14 @@ bool ModulatorImplBase::BuiltInModuleInfraEnabled() const {
bool ModulatorImplBase::BuiltInModuleEnabled(layered_api::Module module) const { bool ModulatorImplBase::BuiltInModuleEnabled(layered_api::Module module) const {
DCHECK(BuiltInModuleInfraEnabled()); DCHECK(BuiltInModuleInfraEnabled());
// Some built-in APIs are available only on SecureContexts.
// https://crbug.com/977470
if (BuiltInModuleRequireSecureContext(module) &&
!GetExecutionContext()->IsSecureContext()) {
return false;
}
if (RuntimeEnabledFeatures::BuiltInModuleAllEnabled()) if (RuntimeEnabledFeatures::BuiltInModuleAllEnabled())
return true; return true;
switch (module) { switch (module) {
...@@ -74,6 +82,20 @@ bool ModulatorImplBase::BuiltInModuleEnabled(layered_api::Module module) const { ...@@ -74,6 +82,20 @@ bool ModulatorImplBase::BuiltInModuleEnabled(layered_api::Module module) const {
} }
} }
bool ModulatorImplBase::BuiltInModuleRequireSecureContext(
layered_api::Module module) {
switch (module) {
case layered_api::Module::kBlank:
case layered_api::Module::kElementsInternal:
case layered_api::Module::kElementsSwitch:
case layered_api::Module::kElementsToast:
case layered_api::Module::kElementsVirtualScroller:
return false;
case layered_api::Module::kKvStorage:
return true;
}
}
void ModulatorImplBase::BuiltInModuleUseCount( void ModulatorImplBase::BuiltInModuleUseCount(
layered_api::Module module) const { layered_api::Module module) const {
DCHECK(BuiltInModuleInfraEnabled()); DCHECK(BuiltInModuleInfraEnabled());
......
...@@ -45,6 +45,8 @@ class ModulatorImplBase : public Modulator { ...@@ -45,6 +45,8 @@ class ModulatorImplBase : public Modulator {
bool BuiltInModuleEnabled(layered_api::Module) const override; bool BuiltInModuleEnabled(layered_api::Module) const override;
void BuiltInModuleUseCount(layered_api::Module) const override; void BuiltInModuleUseCount(layered_api::Module) const override;
static bool BuiltInModuleRequireSecureContext(layered_api::Module);
ModuleRecordResolver* GetModuleRecordResolver() override { ModuleRecordResolver* GetModuleRecordResolver() override {
return module_record_resolver_.Get(); return module_record_resolver_.Get();
} }
......
This is a testharness.js-based test.
PASS Prerequisite check
FAIL KV Storage: in non-secure contexts, import map mappings should fall back promise_test: Unhandled rejection with value: object "TypeError: Failed to resolve module specifier 'std:kv-storage'"
Harness: the test ran to completion.
...@@ -15,7 +15,7 @@ test(() => { ...@@ -15,7 +15,7 @@ test(() => {
<script type="importmap"> <script type="importmap">
{ {
"imports": { "imports": {
"std:kv-storage": [ "./resources/dummy-module.js": [
"std:kv-storage", "std:kv-storage",
"./resources/dummy-module.js" "./resources/dummy-module.js"
] ]
...@@ -25,7 +25,7 @@ test(() => { ...@@ -25,7 +25,7 @@ test(() => {
<script type="module"> <script type="module">
promise_test(async () => { promise_test(async () => {
const result = await import("std:kv-storage"); const result = await import("./resources/dummy-module.js");
assert_equals(namespaceObj.myExport, "not the real KV storage"); assert_equals(result.myExport, "not the real KV storage");
}); });
</script> </script>
This is a testharness.js-based test. This is a testharness.js-based test.
PASS Prerequisite check PASS Prerequisite check
FAIL KV Storage: should fail in non-secure contexts in the fetching phase, not evaluation phase assert_false: The side effects module didn't evaluate either expected false got true FAIL Static import kv-storage in non-secure context assert_unreached: script error event should not be fired Reached unreachable code
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -13,13 +13,14 @@ test(() => { ...@@ -13,13 +13,14 @@ test(() => {
assert_false(self.isSecureContext, "This test must run in a non-secure context"); assert_false(self.isSecureContext, "This test must run in a non-secure context");
}, "Prerequisite check"); }, "Prerequisite check");
async_test(t => { const t = async_test('Static import kv-storage in non-secure context');
window.addEventListener("error", t.step_func_done(errorEvent => {
assert_equals(errorEvent.error.constructor, TypeError, "Must trigger a TypeError"); window.addEventListener("error", t.step_func_done(errorEvent => {
}, { once: true })); assert_equals(errorEvent.error.constructor, TypeError, "Must trigger a TypeError");
}); }, { once: true }));
</script> </script>
<script type="module"> <script type="module"
onerror="t.unreached_func('script error event should not be fired')()">
import "std:kv-storage"; import "std:kv-storage";
</script> </script>
This is a testharness.js-based test.
Harness Error. harness_status.status = 1 , harness_status.message = Uncaught TypeError: KV Storage is only available in secure contexts
PASS Prerequisite check
FAIL Check the events assert_unreached: load event fired Reached unreachable code
Harness: the test ran to completion.
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