paritytech / parity-common

Collection of crates used in Parity projects
https://www.paritytech.io/
Apache License 2.0
282 stars 213 forks source link

[rlp] `RlpIterator::len()` does not update when the iterator is consumed. #761

Closed achillelamb closed 1 year ago

achillelamb commented 1 year ago

Issue

The RlpIterator (and the ExactSizeIterator) documentation states that the len function

Returns the exact remaining length of the iterator.

So it is expected that the value returned is decreased by 1 each time next() is invoked. However, this does not happen, and len() always returns the initial size of the iterator.

Failing test

By slightly modifying the test rlp_iter in rlp/tests/tests.rs you can trigger this misbehavior.

#[test]
fn rlp_iter() {
    let data = vec![0xc8, 0x83, b'c', b'a', b't', 0x83, b'd', b'o', b'g'];
    {
        let rlp = Rlp::new(&data);
        let mut iter = rlp.iter();

        assert_eq!(iter.len(), 2); // passes

        let cat = iter.next().unwrap();
        assert!(cat.is_data());
        assert_eq!(cat.as_raw(), &[0x83, b'c', b'a', b't']);

        assert_eq!(iter.len(), 1); // fails

        let dog = iter.next().unwrap();
        assert!(dog.is_data());
        assert_eq!(dog.as_raw(), &[0x83, b'd', b'o', b'g']);

        assert_eq!(iter.len(), 0); // unreachable

        let none = iter.next();
        assert!(none.is_none());

        let cat_again = rlp.at(0).unwrap();
        assert!(cat_again.is_data());
        assert_eq!(cat_again.as_raw(), &[0x83, b'c', b'a', b't']);
    }
}

Proposed solution

Modify the implementation of the len() function such that it subtracts self.index to self.rlp.item_count().

I can create the PR if it is ok for you.

ordian commented 1 year ago

Good find and yes PRs are welcome! :)