From e5381c9b76f1d88dedb4a453cd026c8e98be5533 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 18:32:33 +0000 Subject: Ensure that the hash always gets checked --- dhall/src/semantics/resolve/env.rs | 31 +++++++++++++--------- dhall/src/semantics/resolve/resolve.rs | 28 ++++++++++++++----- dhall/tests/import/data/simple.dhall | 1 + .../tests/import/failure/unit/HashMismatch2.dhall | 2 ++ dhall/tests/import/failure/unit/HashMismatch2.txt | 9 +++++++ 5 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 dhall/tests/import/data/simple.dhall create mode 100644 dhall/tests/import/failure/unit/HashMismatch2.dhall create mode 100644 dhall/tests/import/failure/unit/HashMismatch2.txt diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index a24c274..29dd16b 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -17,7 +17,7 @@ pub type CyclesStack = Vec; /// Environment for resolving imports #[derive(Debug)] pub struct ImportEnv { - file_cache: Option, + disk_cache: Option, // Missing if it failed to initialize mem_cache: MemCache, stack: CyclesStack, } @@ -69,37 +69,42 @@ impl NameEnv { impl ImportEnv { pub fn new() -> Self { ImportEnv { - file_cache: Cache::new().ok(), + disk_cache: Cache::new().ok(), mem_cache: Default::default(), stack: Default::default(), } } - pub fn get_from_cache( + pub fn get_from_mem_cache( &mut self, location: &ImportLocation, - hash: Option<&Hash>, ) -> Option { - if let Some(expr) = self.mem_cache.get(location) { - return Some(expr.clone()); - } + Some(self.mem_cache.get(location)?.clone()) + } + + pub fn get_from_disk_cache( + &mut self, + hash: &Option, + ) -> Option { let hash = hash.as_ref()?; - let expr = self.file_cache.as_ref()?.get(hash).ok()?; + let expr = self.disk_cache.as_ref()?.get(hash).ok()?; Some(expr) } - pub fn set_cache( + pub fn write_to_mem_cache( &mut self, location: ImportLocation, - hash: Option<&Hash>, expr: Typed, ) { - if let Some(file_cache) = self.file_cache.as_ref() { + self.mem_cache.insert(location, expr); + } + + pub fn write_to_disk_cache(&mut self, hash: &Option, expr: &Typed) { + if let Some(disk_cache) = self.disk_cache.as_ref() { if let Some(hash) = hash { - let _ = file_cache.insert(hash, &expr); + let _ = disk_cache.insert(hash, &expr); } } - self.mem_cache.insert(location, expr); } pub fn with_cycle_detection( diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 002490b..614ea22 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -390,19 +390,35 @@ fn resolve_with_env( let do_sanity_check = import.mode != ImportMode::Location; let location = base_location.chain(&import.location, do_sanity_check)?; + // If the import is in the in-memory cache, or the hash is in the on-disk cache, return // the cached contents. - if let Some(resolved) = - env.get_from_cache(&location, import.hash.as_ref()) - { - return Ok(resolved); + if let Some(typed) = env.get_from_mem_cache(&location) { + // The same location may be used with different or no hashes. Thus we need to check + // the hashes every time. + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + return Ok(typed); + } + if let Some(typed) = env.get_from_disk_cache(&import.hash) { + // No need to check the hash, it was checked before reading the file. We also don't + // write to the in-memory cache, because the location might be completely unrelated + // to the cached file (e.g. `missing sha256:...` is valid). + // This actually means that importing many times a same hashed import will take + // longer than importing many times a same non-hashed import. + return Ok(typed); } + + // Resolve this import, making sure that recursive imports don't cycle back to the + // current one. let typed = env.with_cycle_detection(location.clone(), |env| { resolve_one_import(env, &import, location.clone(), span.clone()) })?; - check_hash(&import, &typed, span)?; + // Add the resolved import to the caches - env.set_cache(location, import.hash.as_ref(), typed.clone()); + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + env.write_to_mem_cache(location, typed.clone()); Ok(typed) }, )?; diff --git a/dhall/tests/import/data/simple.dhall b/dhall/tests/import/data/simple.dhall new file mode 100644 index 0000000..00750ed --- /dev/null +++ b/dhall/tests/import/data/simple.dhall @@ -0,0 +1 @@ +3 diff --git a/dhall/tests/import/failure/unit/HashMismatch2.dhall b/dhall/tests/import/failure/unit/HashMismatch2.dhall new file mode 100644 index 0000000..5fae772 --- /dev/null +++ b/dhall/tests/import/failure/unit/HashMismatch2.dhall @@ -0,0 +1,2 @@ +-- This ensures that even if the file gets imported without hash first, the hash check is not skipped later +../../data/simple.dhall + ../../data/simple.dhall sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ../../data/simple.dhall diff --git a/dhall/tests/import/failure/unit/HashMismatch2.txt b/dhall/tests/import/failure/unit/HashMismatch2.txt new file mode 100644 index 0000000..f03ab24 --- /dev/null +++ b/dhall/tests/import/failure/unit/HashMismatch2.txt @@ -0,0 +1,9 @@ +Type error: error: hash mismatch + --> :2:27 + | +1 | -- This ensures that even if the file gets imported without hash first, the hash check is not skipped later +2 | ../../data/simple.dhall + ../../data/simple.dhall sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ../../data/simple.dhall + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ hash mismatch + | + = note: Expected sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + = note: Found sha256:15f52ecf91c94c1baac02d5a4964b2ed8fa401641a2c8a95e8306ec7c1e3b8d2 -- cgit v1.2.3