Ensure that the hash always gets checked

5 files changed, 52 insertions, 19 deletions

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<ImportLocation>;
 /// Environment for resolving imports
 #[derive(Debug)]
 pub struct ImportEnv {
-    file_cache: Option<Cache>,
+    disk_cache: Option<Cache>, // 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<Typed> {
-        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<Hash>,
+    ) -> Option<Typed> {
         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<Hash>, 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
+ --> <current file>: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