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.

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()),

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",

cli/tests/testdata/webstorage/config_a.jsonc

@@ -0,0 +1,3 @@
+{
+  "compilerOptions": {}
+}

cli/tests/testdata/webstorage/config_b.jsonc

@@ -0,0 +1,3 @@
+{
+  "compilerOptions": {}
+}

cli/tests/testdata/webstorage/fixture.ts

@@ -0,0 +1,2 @@
+import "./logger.ts";
+import "./setter.ts";

cli/tests/testdata/webstorage/logger.ts

@@ -0,0 +1 @@
+console.log(window.localStorage);

cli/tests/testdata/webstorage/serialization.ts

@@ -0,0 +1,4 @@
+window.sessionStorage.setItem("hello", "deno");
+
+console.log(window.localStorage);
+console.log(window.sessionStorage);

cli/tests/testdata/webstorage/serialization.ts.out

@@ -0,0 +1,3 @@
+[WILDCARD]
+Storage {[WILDCARD]
+Storage { length: 1, hello: "deno" }

cli/tests/testdata/webstorage/setter.ts

@@ -0,0 +1 @@
+window.localStorage.setItem("hello", "deno");

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);

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(())
     })