diff options
| -rw-r--r-- | cli/flags.rs | 1 | ||||
| -rw-r--r-- | cli/global_state.rs | 48 | ||||
| -rw-r--r-- | cli/lockfile.rs | 81 | ||||
| -rw-r--r-- | cli/main.rs | 16 | ||||
| -rw-r--r-- | cli/tests/integration_tests.rs | 13 | ||||
| -rw-r--r-- | cli/tests/lock_check_err.json | 2 | ||||
| -rw-r--r-- | cli/tests/lock_dynamic_imports.json | 6 | ||||
| -rw-r--r-- | cli/tests/lock_dynamic_imports.out | 3 | ||||
| -rw-r--r-- | cli/tests/lock_write_requires_lock.out | 3 |
9 files changed, 98 insertions, 75 deletions
diff --git a/cli/flags.rs b/cli/flags.rs index ff262b8da..bbd23148b 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1119,6 +1119,7 @@ fn lock_arg<'a, 'b>() -> Arg<'a, 'b> { fn lock_write_arg<'a, 'b>() -> Arg<'a, 'b> { Arg::with_name("lock-write") .long("lock-write") + .requires("lock") .help("Write lock file. Use with --lock.") } diff --git a/cli/global_state.rs b/cli/global_state.rs index ef5814f90..cd04d03e6 100644 --- a/cli/global_state.rs +++ b/cli/global_state.rs @@ -74,9 +74,9 @@ impl GlobalState { dir.gen_cache.clone(), )?; - // Note: reads lazily from disk on first call to lockfile.check() let lockfile = if let Some(filename) = &flags.lock { - Some(Mutex::new(Lockfile::new(filename.to_string()))) + let lockfile = Lockfile::new(filename.to_string(), flags.lock_write)?; + Some(Mutex::new(lockfile)) } else { None }; @@ -142,8 +142,26 @@ impl GlobalState { .fetch_cached_source_file(&module_specifier, permissions.clone()) .expect("Source file not found"); - // Check if we need to compile files. let module_graph_files = module_graph.values().collect::<Vec<_>>(); + // Check integrity of every file in module graph + if let Some(ref lockfile) = self.lockfile { + let mut g = lockfile.lock().unwrap(); + + for graph_file in &module_graph_files { + let check_passed = + g.check_or_insert(&graph_file.url, &graph_file.source_code); + + if !check_passed { + eprintln!( + "Subresource integrity check failed --lock={}\n{}", + g.filename, graph_file.url + ); + std::process::exit(10); + } + } + } + + // Check if we need to compile files. let should_compile = needs_compilation( self.ts_compiler.compile_js, out.media_type, @@ -165,6 +183,11 @@ impl GlobalState { .await?; } + if let Some(ref lockfile) = self.lockfile { + let g = lockfile.lock().unwrap(); + g.write()?; + } + drop(compile_lock); Ok(()) @@ -181,7 +204,6 @@ impl GlobalState { _maybe_referrer: Option<ModuleSpecifier>, ) -> Result<CompiledModule, ErrBox> { let state1 = self.clone(); - let state2 = self.clone(); let module_specifier = module_specifier.clone(); let out = self @@ -222,24 +244,6 @@ impl GlobalState { drop(compile_lock); - if let Some(ref lockfile) = state2.lockfile { - let mut g = lockfile.lock().unwrap(); - if state2.flags.lock_write { - g.insert(&out.url, out.source_code); - } else { - let check = match g.check(&out.url, out.source_code) { - Err(e) => return Err(ErrBox::from(e)), - Ok(v) => v, - }; - if !check { - eprintln!( - "Subresource integrity check failed --lock={}\n{}", - g.filename, compiled_module.name - ); - std::process::exit(10); - } - } - } Ok(compiled_module) } diff --git a/cli/lockfile.rs b/cli/lockfile.rs index 9a70c71a1..0192c08df 100644 --- a/cli/lockfile.rs +++ b/cli/lockfile.rs @@ -1,26 +1,40 @@ use serde_json::json; pub use serde_json::Value; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::io::Result; -use url::Url; pub struct Lockfile { - need_read: bool, - map: HashMap<String, String>, + write: bool, + map: BTreeMap<String, String>, pub filename: String, } impl Lockfile { - pub fn new(filename: String) -> Lockfile { - Lockfile { - map: HashMap::new(), + pub fn new(filename: String, write: bool) -> Result<Lockfile> { + debug!("lockfile \"{}\", write: {}", filename, write); + + let map = if write { + BTreeMap::new() + } else { + let s = std::fs::read_to_string(&filename)?; + serde_json::from_str(&s)? + }; + + Ok(Lockfile { + write, + map, filename, - need_read: true, - } + }) } + // Synchronize lock file to disk - noop if --lock-write file is not specified. pub fn write(&self) -> Result<()> { - let j = json!(self.map); + if !self.write { + return Ok(()); + } + // Will perform sort so output is deterministic + let map: BTreeMap<_, _> = self.map.iter().collect(); + let j = json!(map); let s = serde_json::to_string_pretty(&j).unwrap(); let mut f = std::fs::OpenOptions::new() .write(true) @@ -33,40 +47,35 @@ impl Lockfile { Ok(()) } - pub fn read(&mut self) -> Result<()> { - debug!("lockfile read {}", self.filename); - let s = std::fs::read_to_string(&self.filename)?; - self.map = serde_json::from_str(&s)?; - self.need_read = false; - Ok(()) + pub fn check_or_insert(&mut self, specifier: &str, code: &str) -> bool { + if self.write { + // In case --lock-write is specified check always passes + self.insert(specifier, code); + true + } else { + self.check(specifier, code) + } } - /// Lazily reads the filename, checks the given module is included. - /// Returns Ok(true) if check passed - pub fn check(&mut self, url: &Url, code: Vec<u8>) -> Result<bool> { - let url_str = url.to_string(); - if url_str.starts_with("file:") { - return Ok(true); + /// Checks the given module is included. + /// Returns Ok(true) if check passed. + fn check(&mut self, specifier: &str, code: &str) -> bool { + if specifier.starts_with("file:") { + return true; } - if self.need_read { - self.read()?; - } - assert!(!self.need_read); - Ok(if let Some(lockfile_checksum) = self.map.get(&url_str) { - let compiled_checksum = crate::checksum::gen(&[&code]); + if let Some(lockfile_checksum) = self.map.get(specifier) { + let compiled_checksum = crate::checksum::gen(&[code.as_bytes()]); lockfile_checksum == &compiled_checksum } else { false - }) + } } - // Returns true if module was not already inserted. - pub fn insert(&mut self, url: &Url, code: Vec<u8>) -> bool { - let url_str = url.to_string(); - if url_str.starts_with("file:") { - return false; + fn insert(&mut self, specifier: &str, code: &str) { + if specifier.starts_with("file:") { + return; } - let checksum = crate::checksum::gen(&[&code]); - self.map.insert(url_str, checksum).is_none() + let checksum = crate::checksum::gen(&[code.as_bytes()]); + self.map.insert(specifier.to_string(), checksum); } } diff --git a/cli/main.rs b/cli/main.rs index 7dcc7326a..3f5b9f734 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -140,19 +140,6 @@ fn write_to_stdout_ignore_sigpipe(bytes: &[u8]) -> Result<(), std::io::Error> { } } -fn write_lockfile(global_state: GlobalState) -> Result<(), std::io::Error> { - if global_state.flags.lock_write { - if let Some(ref lockfile) = global_state.lockfile { - let g = lockfile.lock().unwrap(); - g.write()?; - } else { - eprintln!("--lock flag must be specified when using --lock-write"); - std::process::exit(11); - } - } - Ok(()) -} - fn print_cache_info(state: &GlobalState) { println!( "{} {:?}", @@ -356,8 +343,6 @@ async fn cache_command(flags: Flags, files: Vec<String>) -> Result<(), ErrBox> { worker.preload_module(&specifier).await.map(|_| ())?; } - write_lockfile(global_state)?; - Ok(()) } @@ -606,7 +591,6 @@ async fn run_command(flags: Flags, script: String) -> Result<(), ErrBox> { debug!("main_module {}", main_module); worker.execute_module(&main_module).await?; - write_lockfile(global_state)?; worker.execute("window.dispatchEvent(new Event('load'))")?; (&mut *worker).await?; worker.execute("window.dispatchEvent(new Event('unload'))")?; diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 6f8299c92..6855df707 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1562,6 +1562,12 @@ itest!(js_import_detect { exit_code: 0, }); +itest!(lock_write_requires_lock { + args: "run --lock-write some_file.ts", + output: "lock_write_requires_lock.out", + exit_code: 1, +}); + itest!(lock_write_fetch { args: "run --quiet --allow-read --allow-write --allow-env --allow-run lock_write_fetch.ts", @@ -1582,6 +1588,13 @@ itest_ignore!(lock_check_ok2 { http_server: true, }); +itest!(lock_dynamic_imports { + args: "run --lock=lock_dynamic_imports.json --allow-read --allow-net http://127.0.0.1:4545/cli/tests/013_dynamic_import.ts", + output: "lock_dynamic_imports.out", + exit_code: 10, + http_server: true, +}); + itest!(lock_check_err { args: "run --lock=lock_check_err.json http://127.0.0.1:4545/cli/tests/003_relative_import.ts", output: "lock_check_err.out", diff --git a/cli/tests/lock_check_err.json b/cli/tests/lock_check_err.json index 43fc53227..55a1446d4 100644 --- a/cli/tests/lock_check_err.json +++ b/cli/tests/lock_check_err.json @@ -1,4 +1,4 @@ { - "http://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "5c93c66125878389f47f4abcac003f4be1276c5223612c26302460d71841e287", + "http://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "fe7bbccaedb6579200a8b582f905139296402d06b1b91109d6e12c41a23125da", "http://127.0.0.1:4545/cli/tests/003_relative_import.ts": "bad" } diff --git a/cli/tests/lock_dynamic_imports.json b/cli/tests/lock_dynamic_imports.json new file mode 100644 index 000000000..df77e179c --- /dev/null +++ b/cli/tests/lock_dynamic_imports.json @@ -0,0 +1,6 @@ +{ + "http://127.0.0.1:4545/cli/tests/013_dynamic_import.ts": "c875f10de49bded1ad76f1709d68e6cf2c0cfb8e8e862542a3fcb4ab09257b99", + "http://127.0.0.1:4545/cli/tests/subdir/mod1.ts": "f627f1649f9853adfa096241ae2defa75e4e327cbeb6af0e82a11304b3e5c8be", + "http://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "fe7bbccaedb6579200a8b582f905139296402d06b1b91109d6e12c41a23125da", + "http://127.0.0.1:4545/cli/tests/subdir/subdir2/mod2.ts": "bad" +} diff --git a/cli/tests/lock_dynamic_imports.out b/cli/tests/lock_dynamic_imports.out new file mode 100644 index 000000000..57bc053b9 --- /dev/null +++ b/cli/tests/lock_dynamic_imports.out @@ -0,0 +1,3 @@ +[WILDCARD] +Subresource integrity check failed --lock=lock_dynamic_imports.json +http://127.0.0.1:4545/cli/tests/subdir/subdir2/mod2.ts diff --git a/cli/tests/lock_write_requires_lock.out b/cli/tests/lock_write_requires_lock.out new file mode 100644 index 000000000..7cc5906f6 --- /dev/null +++ b/cli/tests/lock_write_requires_lock.out @@ -0,0 +1,3 @@ +error: The following required arguments were not provided: + --lock <FILE> +[WILDCARD]
\ No newline at end of file |