diff options
author | Kitson Kelly <me@kitsonkelly.com> | 2021-10-27 11:10:27 +1100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-27 11:10:27 +1100 |
commit | 1c739470b590ea13dc0aa67c0ecc7ea6f49b5746 (patch) | |
tree | a7e6cb862cde74cc7861bc7124c540e62fa951b0 | |
parent | 6268703487da02d66552d1e1a42858aa273def90 (diff) |
feat(ext/webstorage): use implied origin when --location not set (#12548)
Closes #11882
BREAKING CHANGE: Previously when `--location` was set, the unique storage key was derived from the the URL of the location instead of just the origin. This change correctly uses just the origin. This may cause previously persisted storage to change its key and data to not be available with the same location as before.
-rw-r--r-- | cli/main.rs | 35 | ||||
-rw-r--r-- | cli/tests/integration/run_tests.rs | 190 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/config_a.jsonc | 3 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/config_b.jsonc | 3 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/fixture.ts | 2 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/logger.ts | 1 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/serialization.ts | 4 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/serialization.ts.out | 3 | ||||
-rw-r--r-- | cli/tests/testdata/webstorage/setter.ts | 1 | ||||
-rw-r--r-- | ext/webstorage/01_webstorage.js | 5 | ||||
-rw-r--r-- | ext/webstorage/lib.rs | 4 |
11 files changed, 234 insertions, 17 deletions
diff --git a/cli/main.rs b/cli/main.rs index f4d4046df..67b59a443 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -204,6 +204,32 @@ pub fn create_main_worker( let create_web_worker_cb = create_web_worker_callback(ps.clone()); + let maybe_storage_key = if let Some(location) = &ps.flags.location { + // if a location is set, then the ascii serialization of the location is + // used, unless the origin is opaque, and then no storage origin is set, as + // we can't expect the origin to be reproducible + let storage_origin = location.origin().ascii_serialization(); + if storage_origin == "null" { + None + } else { + Some(storage_origin) + } + } else if let Some(config_file) = &ps.maybe_config_file { + // otherwise we will use the path to the config file + config_file.path.to_str().map(|s| s.to_string()) + } else { + // otherwise we will use the path to the main module + Some(main_module.to_string()) + }; + + let origin_storage_dir = maybe_storage_key.map(|key| { + ps.dir + .root + // TODO(@crowlKats): change to origin_data for 2.0 + .join("location_data") + .join(checksum::gen(&[key.as_bytes()])) + }); + let options = WorkerOptions { bootstrap: BootstrapOptions { apply_source_maps: true, @@ -231,14 +257,7 @@ pub fn create_main_worker( should_break_on_first_statement, module_loader, get_error_class_fn: Some(&crate::errors::get_error_class_name), - origin_storage_dir: ps.flags.location.clone().map(|loc| { - ps.dir - .root - .clone() - // TODO(@crowlKats): change to origin_data for 2.0 - .join("location_data") - .join(checksum::gen(&[loc.to_string().as_bytes()])) - }), + origin_storage_dir, blob_store: ps.blob_store.clone(), broadcast_channel: ps.broadcast_channel.clone(), shared_array_buffer_store: Some(ps.shared_array_buffer_store.clone()), diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 1d033d9e2..d4e7ccf72 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -237,10 +237,192 @@ itest!(_071_location_unset { }); itest!(_072_location_relative_fetch { - args: "run --location http://127.0.0.1:4545/ --allow-net 072_location_relative_fetch.ts", - output: "072_location_relative_fetch.ts.out", - http_server: true, - }); + args: "run --location http://127.0.0.1:4545/ --allow-net 072_location_relative_fetch.ts", + output: "072_location_relative_fetch.ts.out", + http_server: true, +}); + +// tests the serialization of webstorage (both localStorage and sessionStorage) +itest!(webstorage_serialization { + args: "run webstorage/serialization.ts", + output: "webstorage/serialization.ts.out", +}); + +// tests to ensure that when `--location` is set, all code shares the same +// localStorage cache based on the origin of the location URL. +#[test] +fn webstorage_location_shares_origin() { + let deno_dir = util::new_deno_dir(); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--location") + .arg("https://example.com/a.ts") + .arg("webstorage/fixture.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--location") + .arg("https://example.com/b.ts") + .arg("webstorage/logger.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 1, hello: \"deno\" }\n"); +} + +// test to ensure that when a --config file is set, but no --location, that +// storage persists against unique configuration files. +#[test] +fn webstorage_config_file() { + let deno_dir = util::new_deno_dir(); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--config") + .arg("webstorage/config_a.jsonc") + .arg("webstorage/fixture.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--config") + .arg("webstorage/config_b.jsonc") + .arg("webstorage/logger.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--config") + .arg("webstorage/config_a.jsonc") + .arg("webstorage/logger.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 1, hello: \"deno\" }\n"); +} + +// tests to ensure `--config` does not effect persisted storage when a +// `--location` is provided. +#[test] +fn webstorage_location_precedes_config() { + let deno_dir = util::new_deno_dir(); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--location") + .arg("https://example.com/a.ts") + .arg("--config") + .arg("webstorage/config_a.jsonc") + .arg("webstorage/fixture.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("--location") + .arg("https://example.com/b.ts") + .arg("--config") + .arg("webstorage/config_b.jsonc") + .arg("webstorage/logger.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 1, hello: \"deno\" }\n"); +} + +// test to ensure that when there isn't a configuration or location, that the +// main module is used to determine how to persist storage data. +#[test] +fn webstorage_main_module() { + let deno_dir = util::new_deno_dir(); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("webstorage/fixture.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("webstorage/logger.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 0 }\n"); + + let mut deno_cmd = util::deno_cmd_with_deno_dir(deno_dir.path()); + let output = deno_cmd + .current_dir(util::testdata_path()) + .arg("run") + .arg("webstorage/fixture.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"Storage { length: 1, hello: \"deno\" }\n"); +} itest!(_075_import_local_query_hash { args: "run 075_import_local_query_hash.ts", diff --git a/cli/tests/testdata/webstorage/config_a.jsonc b/cli/tests/testdata/webstorage/config_a.jsonc new file mode 100644 index 000000000..875cb6001 --- /dev/null +++ b/cli/tests/testdata/webstorage/config_a.jsonc @@ -0,0 +1,3 @@ +{ + "compilerOptions": {} +} diff --git a/cli/tests/testdata/webstorage/config_b.jsonc b/cli/tests/testdata/webstorage/config_b.jsonc new file mode 100644 index 000000000..875cb6001 --- /dev/null +++ b/cli/tests/testdata/webstorage/config_b.jsonc @@ -0,0 +1,3 @@ +{ + "compilerOptions": {} +} diff --git a/cli/tests/testdata/webstorage/fixture.ts b/cli/tests/testdata/webstorage/fixture.ts new file mode 100644 index 000000000..cf4bd9f1f --- /dev/null +++ b/cli/tests/testdata/webstorage/fixture.ts @@ -0,0 +1,2 @@ +import "./logger.ts"; +import "./setter.ts"; diff --git a/cli/tests/testdata/webstorage/logger.ts b/cli/tests/testdata/webstorage/logger.ts new file mode 100644 index 000000000..3898c4445 --- /dev/null +++ b/cli/tests/testdata/webstorage/logger.ts @@ -0,0 +1 @@ +console.log(window.localStorage); diff --git a/cli/tests/testdata/webstorage/serialization.ts b/cli/tests/testdata/webstorage/serialization.ts new file mode 100644 index 000000000..f3791d355 --- /dev/null +++ b/cli/tests/testdata/webstorage/serialization.ts @@ -0,0 +1,4 @@ +window.sessionStorage.setItem("hello", "deno"); + +console.log(window.localStorage); +console.log(window.sessionStorage); diff --git a/cli/tests/testdata/webstorage/serialization.ts.out b/cli/tests/testdata/webstorage/serialization.ts.out new file mode 100644 index 000000000..fea76aa43 --- /dev/null +++ b/cli/tests/testdata/webstorage/serialization.ts.out @@ -0,0 +1,3 @@ +[WILDCARD] +Storage {[WILDCARD] +Storage { length: 1, hello: "deno" } diff --git a/cli/tests/testdata/webstorage/setter.ts b/cli/tests/testdata/webstorage/setter.ts new file mode 100644 index 000000000..ec6d474f5 --- /dev/null +++ b/cli/tests/testdata/webstorage/setter.ts @@ -0,0 +1 @@ +window.localStorage.setItem("hello", "deno"); diff --git a/ext/webstorage/01_webstorage.js b/ext/webstorage/01_webstorage.js index 558522a3c..4abb64bfc 100644 --- a/ext/webstorage/01_webstorage.js +++ b/ext/webstorage/01_webstorage.js @@ -91,8 +91,6 @@ } function createStorage(persistent) { - if (persistent) window.location; - const storage = webidl.createBranded(Storage); storage[_persistent] = persistent; @@ -133,7 +131,8 @@ return true; }, has(target, p) { - return (typeof target.getItem(p)) === "string"; + return p === SymbolFor("Deno.customInspect") || + (typeof target.getItem(p)) === "string"; }, ownKeys() { return core.opSync("op_webstorage_iterate_keys", persistent); diff --git a/ext/webstorage/lib.rs b/ext/webstorage/lib.rs index e7e53d983..9894c265d 100644 --- a/ext/webstorage/lib.rs +++ b/ext/webstorage/lib.rs @@ -38,8 +38,8 @@ pub fn init(origin_storage_dir: Option<PathBuf>) -> Extension { ), ]) .state(move |state| { - if let Some(origin_storage_dir) = origin_storage_dir.clone() { - state.put(OriginStorageDir(origin_storage_dir)); + if let Some(origin_storage_dir) = &origin_storage_dir { + state.put(OriginStorageDir(origin_storage_dir.clone())); } Ok(()) }) |