diff --git a/src/access.rs b/src/access.rs index 9cf00245..dc9106f0 100644 --- a/src/access.rs +++ b/src/access.rs @@ -52,34 +52,36 @@ impl TryCompat for BitFlags where A: Access, { - fn try_compat_inner(self, abi: ABI) -> Result, CompatError> { + fn try_compat_inner(&mut self, abi: ABI) -> Result, CompatError> { if self.is_empty() { // Empty access-rights would result to a runtime error. Err(AccessError::Empty.into()) - } else if !Self::all().contains(self) { + } else if !Self::all().contains(*self) { // Unknown access-rights (at build time) would result to a runtime error. // This can only be reached by using the unsafe BitFlags::from_bits_unchecked(). Err(AccessError::Unknown { - access: self, - unknown: self & full_negation(Self::all()), + access: *self, + unknown: *self & full_negation(Self::all()), } .into()) } else { - let compat = self & A::from_all(abi); - if compat.is_empty() { + let compat = *self & A::from_all(abi); + let ret = if compat.is_empty() { Ok(CompatResult::No( - AccessError::Incompatible { access: self }.into(), + AccessError::Incompatible { access: *self }.into(), )) - } else if compat != self { + } else if compat != *self { let error = AccessError::PartiallyCompatible { - access: self, - incompatible: self & full_negation(compat), + access: *self, + incompatible: *self & full_negation(compat), } .into(); - Ok(CompatResult::Partial(compat, error)) + Ok(CompatResult::Partial(error)) } else { - Ok(CompatResult::Full(self)) - } + Ok(CompatResult::Full) + }; + *self = compat; + ret } } } diff --git a/src/compat.rs b/src/compat.rs index 3842c918..47efb16b 100644 --- a/src/compat.rs +++ b/src/compat.rs @@ -558,20 +558,15 @@ fn tailored_compat_level() { } } -// CompatResult is useful because we don't want to duplicate objects (potentially wrapping a file -// descriptor), and we may not have compatibility errors for some objects. TryCompat::try_compat() -// is responsible to either take T or CompatError according to the compatibility level. -// // CompatResult is not public outside this crate. -pub enum CompatResult +pub enum CompatResult where - T: TryCompat, A: Access, { // Fully matches the request. - Full(T), + Full, // Partially matches the request. - Partial(T, CompatError), + Partial(CompatError), // Doesn't matches the request. No(CompatError), } @@ -582,7 +577,7 @@ where Self: Sized + TailoredCompatLevel, A: Access, { - fn try_compat_inner(self, abi: ABI) -> Result, CompatError>; + fn try_compat_inner(&mut self, abi: ABI) -> Result, CompatError>; // Default implementation for objects without children. // @@ -590,6 +585,11 @@ where // compatibility level, if any, with self.tailored_compat_level(default_compat_level), and pass // it with the abi and compat_state to each child.try_compat(). See PathBeneath implementation // and the self.allowed_access.try_compat() call. + // + // # Warning + // + // Errors must be prioritized over incompatibility (i.e. return Err(e) over Ok(None)) for all + // children. fn try_compat_children( self, _abi: ABI, @@ -614,48 +614,51 @@ where L: Into, { let compat_level = self.tailored_compat_level(parent_level); - let new_self = match self.try_compat_children(abi, compat_level, compat_state)? { - Some(n) => n, - None => return Ok(None), - }; - match new_self.try_compat_inner(abi) { - Ok(CompatResult::Full(new_self)) => { + let some_inner = match self.try_compat_inner(abi) { + Ok(CompatResult::Full) => { compat_state.update(CompatState::Full); - Ok(Some(new_self)) + true } - Ok(CompatResult::Partial(new_self, error)) => match compat_level { + Ok(CompatResult::Partial(error)) => match compat_level { CompatLevel::BestEffort => { compat_state.update(CompatState::Partial); - Ok(Some(new_self)) + true } CompatLevel::SoftRequirement => { compat_state.update(CompatState::Dummy); - Ok(None) + false } CompatLevel::HardRequirement => { compat_state.update(CompatState::Dummy); - Err(error) + return Err(error); } }, Ok(CompatResult::No(error)) => match compat_level { CompatLevel::BestEffort => { compat_state.update(CompatState::No); - Ok(None) + false } CompatLevel::SoftRequirement => { compat_state.update(CompatState::Dummy); - Ok(None) + false } CompatLevel::HardRequirement => { compat_state.update(CompatState::Dummy); - Err(error) + return Err(error); } }, - Err(e) => { + Err(error) => { // Safeguard to help for test consistency. compat_state.update(CompatState::Dummy); - Err(e) + return Err(error); } + }; + + // At this point, any inner error have been returned, so we can proceed with + // try_compat_children()?. + match self.try_compat_children(abi, compat_level, compat_state)? { + Some(n) if some_inner => Ok(Some(n)), + _ => Ok(None), } } } diff --git a/src/fs.rs b/src/fs.rs index 258781e7..6086f777 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -269,11 +269,9 @@ where } fn try_compat_inner( - mut self, + &mut self, _abi: ABI, - ) -> Result, CompatError> { - // self.allowed_access was updated with try_compat_children(), called by try_compat(). - + ) -> Result, CompatError> { // Gets subset of valid accesses according the FD type. let valid_access = if is_file(&self.parent_fd).map_err(|e| PathBeneathError::StatCall { source: e })? { @@ -290,9 +288,9 @@ where .into(); self.allowed_access = valid_access; // Linux would return EINVAL. - Ok(CompatResult::Partial(self, error)) + Ok(CompatResult::Partial(error)) } else { - Ok(CompatResult::Full(self)) + Ok(CompatResult::Full) } } } @@ -314,8 +312,8 @@ fn path_beneath_try_compat_children() { .add_rule(PathBeneath::new(PathFd::new("/dev/null").unwrap(), access_file)) .unwrap_err(), RulesetError::AddRules(AddRulesError::Fs(AddRuleError::Compat( - CompatError::Access(AccessError::PartiallyCompatible { access, incompatible })) - )) if access == access_file && incompatible == AccessFs::Refer + CompatError::PathBeneath(PathBeneathError::DirectoryAccess { access, incompatible }) + ))) if access == access_file && incompatible == AccessFs::Refer )); // Test error ordering with ABI::V2