matthiasbeyer/imag
1
0
Fork 0
Code Releases Activity

Revert "Check whether the StoreId is inside the store, before doing anything on the FS"

Browse source 
This reverts commit 373502217e
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.
This commit is contained in:
Matthias Beyer 2016-03-10 18:14:53 +01:00
parent 7dde6c2a56
commit 3413646934
2 changed files with 1 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
libimagstore/src/error.rs
View file

@ -19,7 +19,6 @@ pub enum StoreErrorKind {
IoError, IoError,
StorePathExists, StorePathExists,
StorePathCreate, StorePathCreate,
StorePathOutsideStore,
LockPoisoned, LockPoisoned,
EntryAlreadyBorrowed, EntryAlreadyBorrowed,
EntryAlreadyExists, EntryAlreadyExists,
@ -42,7 +41,6 @@ fn store_error_type_as_str(e: &StoreErrorKind) -> &'static str {
&StoreErrorKind::IoError => "File Error", &StoreErrorKind::IoError => "File Error",
&StoreErrorKind::StorePathExists => "Store path exists", &StoreErrorKind::StorePathExists => "Store path exists",
&StoreErrorKind::StorePathCreate => "Store path create", &StoreErrorKind::StorePathCreate => "Store path create",
&StoreErrorKind::StorePathOutsideStore => "Store path would be outside of store",
&StoreErrorKind::LockPoisoned &StoreErrorKind::LockPoisoned
=> "The internal Store Lock has been poisoned", => "The internal Store Lock has been poisoned",
&StoreErrorKind::EntryAlreadyBorrowed => "Entry is already borrowed", &StoreErrorKind::EntryAlreadyBorrowed => "Entry is already borrowed",

25
libimagstore/src/store.rs
View file

@ -131,11 +131,6 @@ impl Store {
/// Creates the Entry at the given location (inside the entry) /// Creates the Entry at the given location (inside the entry)
pub fn create<'a>(&'a self, id: StoreId) -> Result<FileLockEntry<'a>> { pub fn create<'a>(&'a self, id: StoreId) -> Result<FileLockEntry<'a>> {
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(); let hsmap = self.entries.write();
if hsmap.is_err() { if hsmap.is_err() {
return Err(StoreError::new(StoreErrorKind::LockPoisoned, None)) 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 /// Borrow a given Entry. When the `FileLockEntry` is either `update`d or
/// dropped, the new Entry is written to disk /// dropped, the new Entry is written to disk
pub fn retrieve<'a>(&'a self, id: StoreId) -> Result<FileLockEntry<'a>> { pub fn retrieve<'a>(&'a self, id: StoreId) -> Result<FileLockEntry<'a>> {
if !self.id_in_store(&id) {
debug!("'{:?}' seems not to be in '{:?}'", id, self.location);
return Err(StoreError::new(StoreErrorKind::StorePathOutsideStore, None));
}
self.entries self.entries
.write() .write()
.map_err(|_| StoreError::new(StoreErrorKind::LockPoisoned, None)) .map_err(|_| StoreError::new(StoreErrorKind::LockPoisoned, None))
@ -228,11 +218,6 @@ impl Store {
/// Delete an entry /// Delete an entry
pub fn delete(&self, id: StoreId) -> Result<()> { 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(); let entries_lock = self.entries.write();
if entries_lock.is_err() { if entries_lock.is_err() {
return Err(StoreError::new(StoreErrorKind::LockPoisoned, None)) 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)))) 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 /// Gets the path where this store is on the disk
pub fn path(&self) -> &PathBuf { pub fn path(&self) -> &PathBuf {
&self.location &self.location
} }
} }
impl Drop for Store { impl Drop for Store {