From 3413646934384d76cba47e171a271bcae10e06e2 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 10 Mar 2016 18:14:53 +0100 Subject: [PATCH] Revert "Check whether the StoreId is inside the store, before doing anything on the FS" This reverts commit 373502217e62ad07f76af4579680ad7156bbe390 as this commit was introducing a bug. The StoreId type says `/test/example` for a store id path, which is completely valid, as the root (`/`) is the store itself. The id_in_store() function assumed that the store-id is the full (file-system) path to the store entry, which is false. This commit does not introduce a fix but revert the check. --- libimagstore/src/error.rs | 2 -- libimagstore/src/store.rs | 25 +------------------------ 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/libimagstore/src/error.rs b/libimagstore/src/error.rs index be1375f0..39b47034 100644 --- a/libimagstore/src/error.rs +++ b/libimagstore/src/error.rs @@ -19,7 +19,6 @@ pub enum StoreErrorKind { IoError, StorePathExists, StorePathCreate, - StorePathOutsideStore, LockPoisoned, EntryAlreadyBorrowed, EntryAlreadyExists, @@ -42,7 +41,6 @@ fn store_error_type_as_str(e: &StoreErrorKind) -> &'static str { &StoreErrorKind::IoError => "File Error", &StoreErrorKind::StorePathExists => "Store path exists", &StoreErrorKind::StorePathCreate => "Store path create", - &StoreErrorKind::StorePathOutsideStore => "Store path would be outside of store", &StoreErrorKind::LockPoisoned => "The internal Store Lock has been poisoned", &StoreErrorKind::EntryAlreadyBorrowed => "Entry is already borrowed", diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs index f1018d8a..bb4e6f0d 100644 --- a/libimagstore/src/store.rs +++ b/libimagstore/src/store.rs @@ -131,11 +131,6 @@ impl Store { /// Creates the Entry at the given location (inside the entry) pub fn create<'a>(&'a self, id: StoreId) -> Result> { - if !self.id_in_store(&id) { - debug!("'{:?}' seems not to be in '{:?}'", id, self.location); - return Err(StoreError::new(StoreErrorKind::StorePathOutsideStore, None)); - } - let hsmap = self.entries.write(); if hsmap.is_err() { return Err(StoreError::new(StoreErrorKind::LockPoisoned, None)) @@ -155,11 +150,6 @@ impl Store { /// Borrow a given Entry. When the `FileLockEntry` is either `update`d or /// dropped, the new Entry is written to disk pub fn retrieve<'a>(&'a self, id: StoreId) -> Result> { - if !self.id_in_store(&id) { - debug!("'{:?}' seems not to be in '{:?}'", id, self.location); - return Err(StoreError::new(StoreErrorKind::StorePathOutsideStore, None)); - } - self.entries .write() .map_err(|_| StoreError::new(StoreErrorKind::LockPoisoned, None)) @@ -228,11 +218,6 @@ impl Store { /// Delete an entry pub fn delete(&self, id: StoreId) -> Result<()> { - if !self.id_in_store(&id) { - debug!("'{:?}' seems not to be in '{:?}'", id, self.location); - return Err(StoreError::new(StoreErrorKind::StorePathOutsideStore, None)); - } - let entries_lock = self.entries.write(); if entries_lock.is_err() { return Err(StoreError::new(StoreErrorKind::LockPoisoned, None)) @@ -250,19 +235,11 @@ impl Store { remove_file(&id).map_err(|e| StoreError::new(StoreErrorKind::FileError, Some(Box::new(e)))) } - fn id_in_store(&self, path: &StoreId) -> bool { - path.canonicalize() - .map(|can| { - can.starts_with(&self.location) - }) - .unwrap_or(path.starts_with(&self.location)) - // we return false, as fs::canonicalize() returns an Err(..) on filesystem errors - } - /// Gets the path where this store is on the disk pub fn path(&self) -> &PathBuf { &self.location } + } impl Drop for Store {