diff --git a/lender/src/adapters/chunky.rs b/lender/src/adapters/chunky.rs index 9e4ad8a..8d70e0c 100644 --- a/lender/src/adapters/chunky.rs +++ b/lender/src/adapters/chunky.rs @@ -130,10 +130,7 @@ where } else { // Exhaust if self.len > 0 { - let skip = self - .len - .checked_mul(self.chunk_size) - .expect("overflow in Chunky::nth"); + let skip = self.len.saturating_mul(self.chunk_size); let _ = self.lender.advance_by(skip); self.len = 0; } @@ -197,3 +194,36 @@ impl FusedLender for Chunky where L: FusedLender {} // `next()` will actually produce. This violates the ExactSizeLender // contract, which requires `len()` to match the actual remaining // count. The chunk count is still available through `size_hint()`. + +#[cfg(test)] +mod test { + use crate::prelude::*; + + // A lender reporting len() == usize::MAX with O(1) advance_by, to exercise the + // exhaust branch's chunk-count * chunk_size multiplication near usize::MAX. + struct Huge; + impl<'l> crate::Lending<'l> for Huge { + type Lend = i32; + } + impl crate::Lender for Huge { + crate::check_covariance!(); + fn next(&mut self) -> Option> { + Some(0) + } + fn advance_by(&mut self, _n: usize) -> Result<(), core::num::NonZeroUsize> { + Ok(()) + } + } + impl crate::ExactSizeLender for Huge { + fn len(&self) -> usize { + usize::MAX + } + } + + #[test] + fn test_nth_exhaust_no_overflow_panic() { + // len = usize::MAX.div_ceil(2); nth past the end must not panic on len * 2 overflow + let mut c = Huge.chunky(2); + assert!(c.nth(usize::MAX).is_none()); + } +} diff --git a/lender/src/adapters/flatten.rs b/lender/src/adapters/flatten.rs index 23a1b38..d9268a8 100644 --- a/lender/src/adapters/flatten.rs +++ b/lender/src/adapters/flatten.rs @@ -30,7 +30,8 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.inner.inner = None; *AliasableBox::into_unique(self.inner.lender) } } @@ -133,13 +134,15 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.inner.inner = None; (*AliasableBox::into_unique(self.inner.lender)).into_inner() } /// Returns the inner lender and the mapping function. #[inline] - pub fn into_parts(self) -> (L, Covar) { + pub fn into_parts(mut self) -> (L, Covar) { + *self.inner.inner = None; (*AliasableBox::into_unique(self.inner.lender)).into_parts() } } diff --git a/lender/src/adapters/fuse.rs b/lender/src/adapters/fuse.rs index 303eccd..4c92faf 100644 --- a/lender/src/adapters/fuse.rs +++ b/lender/src/adapters/fuse.rs @@ -77,12 +77,11 @@ where Self: Sized, { if !self.flag { - if let x @ Some(_) = self.lender.last() { - return x; - } self.flag = true; + self.lender.last() + } else { + None } - None } #[inline] @@ -244,3 +243,36 @@ impl Default for Fuse { Fuse::new(L::default()) } } + +#[cfg(test)] +mod test { + use crate::prelude::*; + + // A non-fused lender: yields 10, then None, then 30 if polled again. + struct Resurrect { + step: usize, + } + impl<'l> crate::Lending<'l> for Resurrect { + type Lend = i32; + } + impl crate::Lender for Resurrect { + crate::check_covariance!(); + fn next(&mut self) -> Option> { + let s = self.step; + self.step += 1; + match s { + 0 => Some(10), + 2 => Some(30), + _ => None, + } + } + } + + #[test] + fn test_last_fuses() { + let mut f = (Resurrect { step: 0 }).fuse(); + assert_eq!(f.last(), Some(10)); // drives the inner lender to None + assert_eq!(f.next(), None); // must stay fused, not resurrect 30 + assert_eq!(f.size_hint(), (0, Some(0))); + } +} diff --git a/lender/src/adapters/peekable.rs b/lender/src/adapters/peekable.rs index 0d8a444..6813c60 100644 --- a/lender/src/adapters/peekable.rs +++ b/lender/src/adapters/peekable.rs @@ -41,7 +41,8 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.peeked = None; *AliasableBox::into_unique(self.lender) } @@ -97,15 +98,25 @@ where /// # use lender::prelude::*; /// let mut lender = [1, 2, 3].iter().into_lender().peekable(); /// - /// if let Some(p) = lender.peek_mut() { + /// // SAFETY: `p` is used and dropped within this scope; the lend never escapes the borrow. + /// if let Some(p) = unsafe { lender.peek_mut() } { /// // p is &mut &i32, so we replace the reference /// *p = &10; /// } /// assert_eq!(lender.next(), Some(&10)); /// assert_eq!(lender.next(), Some(&2)); /// ``` + /// + /// # Safety + /// + /// The returned reference exposes the cached lend with the lender's full + /// storage lifetime. The caller must not, through the returned reference, + /// (a) move or copy out a lend that outlives this `&mut self` borrow, nor + /// (b) overwrite the referent with a lend borrowing data that does not + /// outlive this `Peekable`. Either lets the lend escape its borrow, causing + /// a use-after-free. #[inline] - pub fn peek_mut(&mut self) -> Option<&'_ mut Lend<'this, L>> { + pub unsafe fn peek_mut(&mut self) -> Option<&'_ mut Lend<'this, L>> { let lender = &mut self.lender; self.peeked .get_or_insert_with(|| { @@ -232,12 +243,13 @@ where #[inline] fn count(mut self) -> usize { + let extra = match self.peeked.take() { + Some(None) => return 0, + Some(Some(_)) => 1, + None => 0, + }; let lender = *AliasableBox::into_unique(self.lender); - match self.peeked.take() { - Some(None) => 0, - Some(Some(_)) => 1 + lender.count(), - None => lender.count(), - } + extra + lender.count() } #[inline] @@ -437,4 +449,17 @@ mod test { "Peeked element pointer should point to the first element of the array" ); } + + #[test] + fn test_into_inner_and_count_after_peek() { + use crate::prelude::*; + let mut p = [1, 2, 3].iter().into_lender().peekable(); + assert_eq!(p.peek(), Some(&&1)); + assert_eq!(p.count(), 3); // peeked element counted, no panic/UB + + let mut p = [1, 2, 3].iter().into_lender().peekable(); + assert_eq!(p.peek(), Some(&&1)); + let mut inner = p.into_inner(); // must not free-then-drop the cache + assert_eq!(inner.next(), Some(&2)); // peeked &1 consumed into the (now-dropped) cache + } } diff --git a/lender/src/adapters/step_by.rs b/lender/src/adapters/step_by.rs index 2355941..7fbae8f 100644 --- a/lender/src/adapters/step_by.rs +++ b/lender/src/adapters/step_by.rs @@ -86,6 +86,7 @@ where if n == 0 { return self.lender.next(); } + let _ = self.lender.next(); n -= 1; } let mut step = self.step + 1; @@ -248,3 +249,16 @@ where impl ExactSizeLender for StepBy where L: ExactSizeLender {} impl FusedLender for StepBy where L: FusedLender {} + +#[cfg(test)] +mod test { + use crate::prelude::*; + #[test] + fn test_nth_fresh() { + // yields &0,&2,&4; nth(1) is the SECOND yielded element = &2 (was &1 before the fix) + let mut s = [0, 1, 2, 3, 4].iter().into_lender().step_by(2); + assert_eq!(s.nth(1), Some(&2)); + // matches std::iter::StepBy + assert_eq!((0..5).step_by(2).nth(1), Some(2)); + } +} diff --git a/lender/src/adapters/take.rs b/lender/src/adapters/take.rs index 80fff04..48073f9 100644 --- a/lender/src/adapters/take.rs +++ b/lender/src/adapters/take.rs @@ -180,7 +180,7 @@ where self.n -= n + 1; self.lender.nth_back(m) } else { - if len > 0 { + if self.n != 0 && len > 0 { self.lender.nth_back(len - 1); } None @@ -230,6 +230,9 @@ where #[inline] fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + if n == 0 { + return Ok(()); + } // Relies on ExactSizeLender: if len() is inaccurate, // advanced_by_inner - trim_inner may underflow. let trim_inner = self.lender.len().saturating_sub(self.n); @@ -248,3 +251,33 @@ where impl ExactSizeLender for Take where L: ExactSizeLender {} impl FusedLender for Take where L: FusedLender {} + +#[cfg(test)] +mod test { + use crate::prelude::*; + + #[test] + fn test_advance_back_by_zero_is_noop() { + let mut t = [0, 1, 2, 3].iter().into_lender().take(2); + assert!(t.advance_back_by(0).is_ok()); + // inner must be untouched: all four elements remain + let mut inner = t.into_inner(); + assert_eq!(inner.next(), Some(&0)); + assert_eq!(inner.next(), Some(&1)); + assert_eq!(inner.next(), Some(&2)); + assert_eq!(inner.next(), Some(&3)); + assert_eq!(inner.next(), None); + } + + #[test] + fn test_empty_take_nth_back_leaves_inner() { + let mut t = [0, 1, 2, 3].iter().into_lender().take(0); + assert_eq!(t.nth_back(0), None); + let mut inner = t.into_inner(); + assert_eq!(inner.next(), Some(&0)); + assert_eq!(inner.next(), Some(&1)); + assert_eq!(inner.next(), Some(&2)); + assert_eq!(inner.next(), Some(&3)); + assert_eq!(inner.next(), None); + } +} diff --git a/lender/src/adapters/take_while.rs b/lender/src/adapters/take_while.rs index 45047a6..6afb506 100644 --- a/lender/src/adapters/take_while.rs +++ b/lender/src/adapters/take_while.rs @@ -68,11 +68,10 @@ where #[inline] fn next(&mut self) -> Option> { if !self.flag { - let x = self.lender.next()?; - if (self.predicate)(&x) { - return Some(x); + match self.lender.next() { + Some(x) if (self.predicate)(&x) => return Some(x), + _ => self.flag = true, } - self.flag = true; } None } @@ -145,3 +144,36 @@ where L: Lender, { } + +#[cfg(test)] +mod test { + use crate::prelude::*; + + // A non-fused lender: yields 10, then None, then 30 if polled again. + struct Resurrect { + step: usize, + } + impl<'l> crate::Lending<'l> for Resurrect { + type Lend = i32; + } + impl crate::Lender for Resurrect { + crate::check_covariance!(); + fn next(&mut self) -> Option> { + let s = self.step; + self.step += 1; + match s { + 0 => Some(10), + 2 => Some(30), + _ => None, + } + } + } + + #[test] + fn test_none_is_terminal() { + let mut tw = (Resurrect { step: 0 }).take_while(|_| true); + assert_eq!(tw.next(), Some(10)); + assert_eq!(tw.next(), None); // inner yields None -> TakeWhile is done + assert_eq!(tw.next(), None); // must not resurrect 30 (TakeWhile is FusedLender) + } +} diff --git a/lender/src/fallible_adapters/chunky.rs b/lender/src/fallible_adapters/chunky.rs index dd7e272..bf07a12 100644 --- a/lender/src/fallible_adapters/chunky.rs +++ b/lender/src/fallible_adapters/chunky.rs @@ -44,10 +44,7 @@ where } else { // Exhaust if self.len > 0 { - let skip = self - .len - .checked_mul(self.chunk_size) - .expect("overflow in Chunky::nth"); + let skip = self.len.saturating_mul(self.chunk_size); let _ = self.lender.advance_by(skip)?; self.len = 0; } @@ -101,3 +98,42 @@ where } impl FusedFallibleLender for Chunky where L: FusedFallibleLender {} + +#[cfg(test)] +mod test { + use core::convert::Infallible; + + use crate::prelude::*; + + // A fallible lender reporting len() == usize::MAX with O(1) advance_by. + struct Huge; + impl<'l> crate::FallibleLending<'l> for Huge { + type Lend = i32; + } + impl crate::FallibleLender for Huge { + type Error = Infallible; + crate::check_covariance_fallible!(); + fn next(&mut self) -> Result>, Self::Error> { + Ok(Some(0)) + } + fn advance_by( + &mut self, + _n: usize, + ) -> Result, Self::Error> { + Ok(Ok(())) + } + } + impl crate::ExactSizeFallibleLender for Huge { + fn len(&self) -> usize { + usize::MAX + } + } + + #[test] + fn test_nth_exhaust_no_overflow_panic() -> Result<(), Infallible> { + // len = usize::MAX.div_ceil(2); nth past the end must not panic on len * 2 overflow + let mut c = Huge.chunky(2); + assert!(c.nth(usize::MAX)?.is_none()); + Ok(()) + } +} diff --git a/lender/src/fallible_adapters/flatten.rs b/lender/src/fallible_adapters/flatten.rs index 4cb4bae..5668ee3 100644 --- a/lender/src/fallible_adapters/flatten.rs +++ b/lender/src/fallible_adapters/flatten.rs @@ -33,7 +33,8 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.inner.inner = None; *AliasableBox::into_unique(self.inner.lender) } } @@ -141,13 +142,15 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.inner.inner = None; (*AliasableBox::into_unique(self.inner.lender)).into_inner() } /// Returns the inner lender and the mapping function. #[inline] - pub fn into_parts(self) -> (L, Covar) { + pub fn into_parts(mut self) -> (L, Covar) { + *self.inner.inner = None; (*AliasableBox::into_unique(self.inner.lender)).into_parts() } } @@ -484,4 +487,38 @@ mod test { assert_eq!(l.next(), Ok(None)); assert_eq!(l.next(), Ok(None)); } + + #[test] + fn test_flatten_into_inner() -> Result<(), Infallible> { + let mut flatten = Parent([0, 1, 2, 3]).into_fallible().flatten(); + let _ = flatten.next()?; // populate the sub-lender cache + let _inner = flatten.into_inner(); // must clear cache before freeing the box + Ok(()) + } + + #[test] + fn test_flat_map_into_inner_and_parts() { + use crate::traits::IteratorExt; + let mut l = [1, 2] + .into_iter() + .into_lender() + .into_fallible() + // SAFETY: closure returns an owned Result (trivially covariant). + .flat_map(unsafe { + crate::Covar::__new(|n: i32| Ok((0..n).into_lender().into_fallible())) + }); + let _ = l.next(); // populate the sub-lender cache + let _inner = l.into_inner(); // must clear cache before freeing the box + + let mut l = [1, 2] + .into_iter() + .into_lender() + .into_fallible() + // SAFETY: closure returns an owned Result (trivially covariant). + .flat_map(unsafe { + crate::Covar::__new(|n: i32| Ok((0..n).into_lender().into_fallible())) + }); + let _ = l.next(); + let (_inner, _f) = l.into_parts(); + } } diff --git a/lender/src/fallible_adapters/fuse.rs b/lender/src/fallible_adapters/fuse.rs index d1153de..4c6c6f2 100644 --- a/lender/src/fallible_adapters/fuse.rs +++ b/lender/src/fallible_adapters/fuse.rs @@ -60,12 +60,11 @@ where Self: Sized, { if !self.flag { - if let x @ Some(_) = self.lender.last()? { - return Ok(x); - } self.flag = true; + self.lender.last() + } else { + Ok(None) } - Ok(None) } #[inline] @@ -218,3 +217,40 @@ where } impl FusedFallibleLender for Fuse where L: FallibleLender {} + +#[cfg(test)] +mod test { + use core::convert::Infallible; + + use crate::prelude::*; + + // A non-fused fallible lender: yields 10, then None, then 30 if polled again. + struct Resurrect { + step: usize, + } + impl<'l> crate::FallibleLending<'l> for Resurrect { + type Lend = i32; + } + impl crate::FallibleLender for Resurrect { + type Error = Infallible; + crate::check_covariance_fallible!(); + fn next(&mut self) -> Result>, Self::Error> { + let s = self.step; + self.step += 1; + Ok(match s { + 0 => Some(10), + 2 => Some(30), + _ => None, + }) + } + } + + #[test] + fn test_last_fuses() -> Result<(), Infallible> { + let mut f = (Resurrect { step: 0 }).fuse(); + assert_eq!(f.last()?, Some(10)); // drives the inner lender to None + assert_eq!(f.next()?, None); // must stay fused, not resurrect 30 + assert_eq!(f.size_hint(), (0, Some(0))); + Ok(()) + } +} diff --git a/lender/src/fallible_adapters/peekable.rs b/lender/src/fallible_adapters/peekable.rs index 02e9568..7d97f55 100644 --- a/lender/src/fallible_adapters/peekable.rs +++ b/lender/src/fallible_adapters/peekable.rs @@ -44,7 +44,8 @@ where /// Returns the inner lender. #[inline] - pub fn into_inner(self) -> L { + pub fn into_inner(mut self) -> L { + *self.peeked = None; *AliasableBox::into_unique(self.lender) } @@ -121,7 +122,8 @@ where /// .into_fallible() /// .peekable(); /// - /// if let Some(p) = lender.peek_mut()? { + /// // SAFETY: `p` is used and dropped within this scope; the lend never escapes the borrow. + /// if let Some(p) = unsafe { lender.peek_mut() }? { /// // p is &mut &i32, so we replace the reference /// *p = &10; /// } @@ -130,8 +132,17 @@ where /// # Ok(()) /// # } /// ``` + /// + /// # Safety + /// + /// The returned reference exposes the cached lend with the fallible lender's + /// full storage lifetime. The caller must not, through the returned reference, + /// (a) move or copy out a lend that outlives this `&mut self` borrow, nor + /// (b) overwrite the referent with a lend borrowing data that does not + /// outlive this `FalliblePeekable`. Either lets the lend escape its borrow, + /// causing a use-after-free. #[inline] - pub fn peek_mut(&mut self) -> Result>, L::Error> { + pub unsafe fn peek_mut(&mut self) -> Result>, L::Error> { let lender = &mut self.lender; if self.peeked.is_none() { *self.peeked = Some( @@ -296,12 +307,13 @@ where #[inline] fn count(mut self) -> Result { + let extra = match self.peeked.take() { + Some(None) => return Ok(0), + Some(Some(_)) => 1, + None => 0, + }; let lender = *AliasableBox::into_unique(self.lender); - match self.peeked.take() { - Some(None) => Ok(0), - Some(Some(_)) => Ok(1 + lender.count()?), - None => lender.count(), - } + Ok(extra + lender.count()?) } #[inline] @@ -520,4 +532,18 @@ mod test { "Peeked element pointer should point to the first element of the array" ); } + + #[test] + fn test_into_inner_and_count_after_peek() -> Result<(), Infallible> { + use crate::prelude::*; + let mut p = [1, 2, 3].iter().into_lender().into_fallible().peekable(); + assert_eq!(p.peek()?, Some(&&1)); + assert_eq!(p.count()?, 3); // peeked element counted via the Ok/? path + + let mut p = [1, 2, 3].iter().into_lender().into_fallible().peekable(); + assert_eq!(p.peek()?, Some(&&1)); + let mut inner = p.into_inner(); // must clear the cache before freeing the box + assert_eq!(inner.next()?, Some(&2)); + Ok(()) + } } diff --git a/lender/src/fallible_adapters/step_by.rs b/lender/src/fallible_adapters/step_by.rs index 7516202..3966d71 100644 --- a/lender/src/fallible_adapters/step_by.rs +++ b/lender/src/fallible_adapters/step_by.rs @@ -63,6 +63,7 @@ where if n == 0 { return self.lender.next(); } + let _ = self.lender.next()?; n -= 1; } let mut step = self.step + 1; @@ -227,3 +228,21 @@ where impl ExactSizeFallibleLender for StepBy where L: ExactSizeFallibleLender {} impl FusedFallibleLender for StepBy where L: FusedFallibleLender {} + +#[cfg(test)] +mod test { + use core::convert::Infallible; + + use crate::prelude::*; + #[test] + fn test_nth_fresh() -> Result<(), Infallible> { + // yields &0,&2,&4; nth(1) is the SECOND yielded element = &2 (was &1 before the fix) + let mut s = [0, 1, 2, 3, 4] + .iter() + .into_lender() + .into_fallible() + .step_by(2); + assert_eq!(s.nth(1)?, Some(&2)); + Ok(()) + } +} diff --git a/lender/src/fallible_adapters/take.rs b/lender/src/fallible_adapters/take.rs index 69bac1d..d4d012b 100644 --- a/lender/src/fallible_adapters/take.rs +++ b/lender/src/fallible_adapters/take.rs @@ -160,7 +160,7 @@ where self.n -= n + 1; self.lender.nth_back(m) } else { - if len > 0 { + if self.n != 0 && len > 0 { self.lender.nth_back(len - 1)?; } Ok(None) @@ -210,6 +210,9 @@ where #[inline] fn advance_back_by(&mut self, n: usize) -> Result, Self::Error> { + if n == 0 { + return Ok(Ok(())); + } let trim_inner = self.lender.len().saturating_sub(self.n); let advance_by = trim_inner.saturating_add(n); let remainder = match self.lender.advance_back_by(advance_by)? { @@ -226,3 +229,37 @@ where impl ExactSizeFallibleLender for Take where L: ExactSizeFallibleLender {} impl FusedFallibleLender for Take where L: FusedFallibleLender {} + +#[cfg(test)] +mod test { + use core::convert::Infallible; + + use crate::prelude::*; + + #[test] + fn test_advance_back_by_zero_is_noop() -> Result<(), Infallible> { + let mut t = [0, 1, 2, 3].iter().into_lender().into_fallible().take(2); + assert!(t.advance_back_by(0)?.is_ok()); + // inner must be untouched: all four elements remain + let mut inner = t.into_inner(); + assert_eq!(inner.next()?, Some(&0)); + assert_eq!(inner.next()?, Some(&1)); + assert_eq!(inner.next()?, Some(&2)); + assert_eq!(inner.next()?, Some(&3)); + assert_eq!(inner.next()?, None); + Ok(()) + } + + #[test] + fn test_empty_take_nth_back_leaves_inner() -> Result<(), Infallible> { + let mut t = [0, 1, 2, 3].iter().into_lender().into_fallible().take(0); + assert_eq!(t.nth_back(0)?, None); + let mut inner = t.into_inner(); + assert_eq!(inner.next()?, Some(&0)); + assert_eq!(inner.next()?, Some(&1)); + assert_eq!(inner.next()?, Some(&2)); + assert_eq!(inner.next()?, Some(&3)); + assert_eq!(inner.next()?, None); + Ok(()) + } +} diff --git a/lender/src/fallible_adapters/take_while.rs b/lender/src/fallible_adapters/take_while.rs index ade08c0..20b3696 100644 --- a/lender/src/fallible_adapters/take_while.rs +++ b/lender/src/fallible_adapters/take_while.rs @@ -39,7 +39,10 @@ where if !self.flag { let x = match self.lender.next()? { Some(x) => x, - None => return Ok(None), + None => { + self.flag = true; + return Ok(None); + } }; if (self.predicate)(&x)? { return Ok(Some(x)); @@ -122,3 +125,40 @@ where L: FallibleLender, { } + +#[cfg(test)] +mod test { + use core::convert::Infallible; + + use crate::prelude::*; + + // A non-fused fallible lender: yields 10, then None, then 30 if polled again. + struct Resurrect { + step: usize, + } + impl<'l> crate::FallibleLending<'l> for Resurrect { + type Lend = i32; + } + impl crate::FallibleLender for Resurrect { + type Error = Infallible; + crate::check_covariance_fallible!(); + fn next(&mut self) -> Result>, Self::Error> { + let s = self.step; + self.step += 1; + Ok(match s { + 0 => Some(10), + 2 => Some(30), + _ => None, + }) + } + } + + #[test] + fn test_none_is_terminal() -> Result<(), Infallible> { + let mut tw = (Resurrect { step: 0 }).take_while(|_| Ok(true)); + assert_eq!(tw.next()?, Some(10)); + assert_eq!(tw.next()?, None); // inner yields None -> TakeWhile is done + assert_eq!(tw.next()?, None); // must not resurrect 30 (TakeWhile is FusedFallibleLender) + Ok(()) + } +} diff --git a/lender/src/fallible_sources/from_iter_ref.rs b/lender/src/fallible_sources/from_iter_ref.rs index f654bc0..9f35213 100644 --- a/lender/src/fallible_sources/from_iter_ref.rs +++ b/lender/src/fallible_sources/from_iter_ref.rs @@ -60,6 +60,7 @@ impl FallibleLender for FromIterRef { #[inline] fn next(&mut self) -> Result>, Self::Error> { + self.current = None; self.current = self.iter.next()?; Ok(self.current.as_ref()) } @@ -71,6 +72,7 @@ impl FallibleLender for FromIterRef { #[inline] fn nth(&mut self, n: usize) -> Result>, Self::Error> { + self.current = None; self.current = self.iter.nth(n)?; Ok(self.current.as_ref()) } @@ -134,6 +136,7 @@ impl FallibleLender for FromIterRef { impl DoubleEndedFallibleLender for FromIterRef { #[inline] fn next_back(&mut self) -> Result>, Self::Error> { + self.current = None; self.current = self.iter.next_back()?; Ok(self.current.as_ref()) } @@ -185,3 +188,42 @@ impl From for FromIterRef { from_iter_ref(iter) } } + +#[cfg(test)] +mod test { + use core::cell::{RefCell, RefMut}; + use core::convert::Infallible; + + use fallible_iterator::FallibleIterator; + + use super::from_iter_ref; + use crate::FallibleLender; + + // A fallible iterator whose items each hold an exclusive borrow of `cell`. + struct BorrowIter<'a> { + cell: &'a RefCell, + n: usize, + } + impl<'a> FallibleIterator for BorrowIter<'a> { + type Item = RefMut<'a, i32>; + type Error = Infallible; + fn next(&mut self) -> Result, Self::Error> { + if self.n == 0 { + return Ok(None); + } + self.n -= 1; + Ok(Some(self.cell.borrow_mut())) + } + } + + #[test] + fn test_drops_previous_item_before_fetch() -> Result<(), Infallible> { + let cell = RefCell::new(0); + let mut l = from_iter_ref(BorrowIter { cell: &cell, n: 3 }); + assert!(l.next()?.is_some()); + assert!(l.next()?.is_some()); // buggy code panics "already mutably borrowed" + assert!(l.next()?.is_some()); + assert!(l.next()?.is_none()); + Ok(()) + } +} diff --git a/lender/src/sources/from_iter_ref.rs b/lender/src/sources/from_iter_ref.rs index 493d8e2..6c2a6ee 100644 --- a/lender/src/sources/from_iter_ref.rs +++ b/lender/src/sources/from_iter_ref.rs @@ -54,6 +54,7 @@ impl Lender for FromIterRef { #[inline] fn next(&mut self) -> Option> { + self.current = None; self.current = self.iter.next(); self.current.as_ref() } @@ -65,6 +66,7 @@ impl Lender for FromIterRef { #[inline] fn nth(&mut self, n: usize) -> Option> { + self.current = None; self.current = self.iter.nth(n); self.current.as_ref() } @@ -129,12 +131,14 @@ impl Lender for FromIterRef { impl DoubleEndedLender for FromIterRef { #[inline] fn next_back(&mut self) -> Option> { + self.current = None; self.current = self.iter.next_back(); self.current.as_ref() } #[inline] fn nth_back(&mut self, n: usize) -> Option> { + self.current = None; self.current = self.iter.nth_back(n); self.current.as_ref() } @@ -191,3 +195,37 @@ impl From for FromIterRef { from_iter_ref(iter) } } + +#[cfg(test)] +mod test { + use core::cell::{RefCell, RefMut}; + + use super::from_iter_ref; + use crate::Lender; + + // An iterator whose items each hold an exclusive borrow of `cell`. + struct BorrowIter<'a> { + cell: &'a RefCell, + n: usize, + } + impl<'a> Iterator for BorrowIter<'a> { + type Item = RefMut<'a, i32>; + fn next(&mut self) -> Option { + if self.n == 0 { + return None; + } + self.n -= 1; + Some(self.cell.borrow_mut()) + } + } + + #[test] + fn test_drops_previous_item_before_fetch() { + let cell = RefCell::new(0); + let mut l = from_iter_ref(BorrowIter { cell: &cell, n: 3 }); + assert!(l.next().is_some()); + assert!(l.next().is_some()); // buggy code panics "already mutably borrowed" + assert!(l.next().is_some()); + assert!(l.next().is_none()); + } +} diff --git a/lender/src/traits/fallible_lender.rs b/lender/src/traits/fallible_lender.rs index 7d53da9..9e34ad4 100644 --- a/lender/src/traits/fallible_lender.rs +++ b/lender/src/traits/fallible_lender.rs @@ -140,8 +140,9 @@ pub trait FallibleLender: for<'all /* where Self: 'all */> FallibleLending<'all> fn next(&mut self) -> Result>, Self::Error>; /// Takes the next `chunk_size` lends of the lender with temporary lender - /// [`Chunk`]. This is equivalent to cloning the lender and calling - /// [`take(chunk_size)`](FallibleLender::take) on it. + /// [`Chunk`]. The returned [`Chunk`] borrows this fallible lender mutably + /// and advances it as the chunk is consumed; any elements left unconsumed + /// when the [`Chunk`] is dropped remain and are produced by later calls. /// /// # Examples /// diff --git a/lender/src/traits/lender.rs b/lender/src/traits/lender.rs index eed9d4f..bf66b9b 100644 --- a/lender/src/traits/lender.rs +++ b/lender/src/traits/lender.rs @@ -136,8 +136,9 @@ pub trait Lender: for<'all /* where Self: 'all */> Lending<'all> { /// ``` fn next(&mut self) -> Option>; /// Takes the next `chunk_size` lends of the lender with temporary lender - /// [`Chunk`]. This is equivalent to cloning the lender and calling - /// [`take(chunk_size)`](Lender::take) on it. + /// [`Chunk`]. The returned [`Chunk`] borrows this lender mutably and + /// advances it as the chunk is consumed; any elements left unconsumed when + /// the [`Chunk`] is dropped remain and are produced by later calls. /// /// # Examples /// diff --git a/lender/tests/fail/peekable_peek_mut_requires_unsafe.rs b/lender/tests/fail/peekable_peek_mut_requires_unsafe.rs new file mode 100644 index 0000000..150c73b --- /dev/null +++ b/lender/tests/fail/peekable_peek_mut_requires_unsafe.rs @@ -0,0 +1,10 @@ +// Test that Peekable::peek_mut cannot be called from safe code: it is `unsafe` +// because the returned reference exposes the cached lend with the lender's full +// storage lifetime, allowing a lend to escape its borrow (use-after-free). + +use lender::prelude::*; + +fn main() { + let mut p = [1, 2, 3].iter().into_lender().peekable(); + let _ = p.peek_mut(); // ERROR: call to unsafe function is unsafe and requires unsafe block +} diff --git a/lender/tests/fail/peekable_peek_mut_requires_unsafe.stderr b/lender/tests/fail/peekable_peek_mut_requires_unsafe.stderr new file mode 100644 index 0000000..85d2a12 --- /dev/null +++ b/lender/tests/fail/peekable_peek_mut_requires_unsafe.stderr @@ -0,0 +1,7 @@ +error[E0133]: call to unsafe function `lender::Peekable::<'this, L>::peek_mut` is unsafe and requires unsafe block + --> tests/fail/peekable_peek_mut_requires_unsafe.rs:9:13 + | +9 | let _ = p.peek_mut(); // ERROR: call to unsafe function is unsafe and requires unsafe block + | ^^^^^^^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior diff --git a/lender/tests/test_adapters_chunk.rs b/lender/tests/test_adapters_chunk.rs index 2f55b1c..6adc95b 100644 --- a/lender/tests/test_adapters_chunk.rs +++ b/lender/tests/test_adapters_chunk.rs @@ -42,7 +42,8 @@ fn test_peekable_peek_mut() { let mut peekable = VecLender::new(vec![1, 2, 3]).peekable(); // peek_mut returns a mutable reference to the lend - assert!(peekable.peek_mut().is_some()); + // SAFETY: the peeked reference is only inspected, never moved out or overwritten to escape. + assert!(unsafe { peekable.peek_mut() }.is_some()); assert_eq!(peekable.next(), Some(&1)); assert_eq!(peekable.next(), Some(&2)); } diff --git a/lender/tests/test_fallible_adapters.rs b/lender/tests/test_fallible_adapters.rs index 799f6d7..57f7739 100644 --- a/lender/tests/test_fallible_adapters.rs +++ b/lender/tests/test_fallible_adapters.rs @@ -80,7 +80,8 @@ fn test_fallible_peekable_adapter() { assert_eq!(peekable.next().unwrap(), Some(2)); // Test peek_mut - if let Some(val) = peekable.peek_mut().unwrap() { + // SAFETY: `val` is written through and dropped within this scope; the lend never escapes. + if let Some(val) = unsafe { peekable.peek_mut() }.unwrap() { *val = 100; } assert_eq!(peekable.next().unwrap(), Some(100)); diff --git a/lender/tests/test_fallible_methods.rs b/lender/tests/test_fallible_methods.rs index 39df000..d556b7b 100644 --- a/lender/tests/test_fallible_methods.rs +++ b/lender/tests/test_fallible_methods.rs @@ -405,7 +405,8 @@ fn test_fallible_peekable_peek_mut() { let fallible: lender::IntoFallible<_> = VecLender::new(vec![1, 2, 3]).into_fallible(); let mut peekable = fallible.peekable(); // peek_mut to store a value and get mutable reference - let peeked = peekable.peek_mut().unwrap(); + // SAFETY: `peeked` is only compared below and dropped before `peekable` is used again. + let peeked = unsafe { peekable.peek_mut() }.unwrap(); assert_eq!(peeked, Some(&mut &1)); }