diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index df9b89e..7601ec6 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -38,8 +38,8 @@ doc = false bench = false [[bin]] -name = "generic_sysex_inserting_payloads" -path = "./fuzz_targets/generic_sysex_inserting_payloads.rs" +name = "generic_sysex_splicing_payloads" +path = "./fuzz_targets/generic_sysex_splicing_payloads.rs" test = false doc = false bench = false diff --git a/fuzz/fuzz_targets/generic_sysex_inserting_payloads.rs b/fuzz/fuzz_targets/generic_sysex_splicing_payloads.rs similarity index 79% rename from fuzz/fuzz_targets/generic_sysex_inserting_payloads.rs rename to fuzz/fuzz_targets/generic_sysex_splicing_payloads.rs index fa91db9..45400a6 100644 --- a/fuzz/fuzz_targets/generic_sysex_inserting_payloads.rs +++ b/fuzz/fuzz_targets/generic_sysex_splicing_payloads.rs @@ -59,12 +59,13 @@ impl IntoByte for u8 { } } -fn test_case(data: &InputData, mut message: M, index: usize) +fn test_case(data: &InputData, mut message: M, range: R) where B: midi2::buffer::Buffer + midi2::buffer::BufferTryResize + midi2::buffer::BufferMut, M: midi2::Sysex, >::Byte: Eq + core::fmt::Debug, u8: IntoByte<>::Byte>, + R: core::ops::RangeBounds + Clone, { let Ok(()) = message.try_set_payload(data.initial_data.iter().map(u8::byte)) else { return; @@ -78,14 +79,16 @@ where ); } - let Ok(()) = message.try_insert_payload(data.data_to_insert.iter().map(u8::byte), index) else { + let Ok(()) = + message.try_splice_payload(data.data_to_insert.iter().map(u8::byte), range.clone()) + else { return; }; let actual = message.payload().collect::>(); let expected = { let mut ret = data.initial_data.clone(); - ret.splice(index..index, data.data_to_insert.clone()); + ret.splice(range, data.data_to_insert.clone()); ret.iter().map(u8::byte).collect::>() }; assert_eq!(actual, expected); @@ -94,10 +97,18 @@ where fuzz_target!(|data: InputData| { let mut rng = rand::rngs::StdRng::seed_from_u64(data.seed); let fized_size_buffer_size = rng.random_range(4..MAX_BUFFER_SIZE); - let index = if data.initial_data.is_empty() { - 0 - } else { - rng.random_range(0..data.initial_data.len()) + let range = { + if data.initial_data.is_empty() { + 0..0 + } else { + let lower = rng.random_range(0..data.initial_data.len()); + if lower == data.initial_data.len() { + lower..lower + } else { + let upper = rng.random_range(lower..data.initial_data.len()); + lower..upper + } + } }; test_case( &data, @@ -105,7 +116,7 @@ fuzz_target!(|data: InputData| { FixedSizeBuffer::::new(fized_size_buffer_size), ) .unwrap(), - index, + range.clone(), ); test_case( &data, @@ -113,7 +124,7 @@ fuzz_target!(|data: InputData| { FixedSizeBuffer::::new(fized_size_buffer_size), ) .unwrap(), - index, + range.clone(), ); test_case( &data, @@ -121,6 +132,6 @@ fuzz_target!(|data: InputData| { FixedSizeBuffer::::new(fized_size_buffer_size), ) .unwrap(), - index, + range.clone(), ); }); diff --git a/midi2/src/detail/helpers.rs b/midi2/src/detail/helpers.rs index cc0eddf..148bfb1 100644 --- a/midi2/src/detail/helpers.rs +++ b/midi2/src/detail/helpers.rs @@ -89,16 +89,17 @@ pub fn validate_sysex_group_statuses< Ok(()) } -pub fn try_insert_sysex_data< +pub fn try_splice_sysex_data< B: crate::buffer::Buffer + crate::buffer::BufferMut + crate::buffer::BufferTryResize, S: SysexInternal, D: core::iter::Iterator>::Byte>, + R: core::ops::RangeBounds, >( sysex: &mut S, data: D, - before: usize, + range: R, ) -> core::result::Result<(), crate::error::BufferOverflow> { - match detail::try_insert_sysex_data(sysex, data, |s, sz| s.try_resize(sz), before) { + match detail::try_splice_sysex_data(sysex, data, |s, sz| s.try_resize(sz), range) { Err(e) => { // if the write failed we reset the message // back to zero data @@ -111,23 +112,24 @@ pub fn try_insert_sysex_data< } } -pub fn insert_sysex_data< +pub fn splice_sysex_data< B: crate::buffer::Buffer + crate::buffer::BufferMut + crate::buffer::BufferResize, S: SysexInternal, D: core::iter::Iterator>::Byte>, + R: core::ops::RangeBounds, >( sysex: &mut S, data: D, - before: usize, + range: R, ) { - detail::try_insert_sysex_data( + detail::try_splice_sysex_data( sysex, data, |s, sz| { s.resize(sz); Ok(()) }, - before, + range, ) .expect("Resizable buffers should not fail here") } @@ -137,16 +139,17 @@ mod detail { use super::*; - pub fn try_insert_sysex_data< + pub fn try_splice_sysex_data< B: crate::buffer::Buffer + crate::buffer::BufferMut, S: crate::traits::SysexInternal, D: core::iter::Iterator>::Byte>, R: Fn(&mut S, usize) -> core::result::Result<(), SysexTryResizeError>, + Rg: core::ops::RangeBounds, >( sysex: &mut S, data: D, resize: R, - before: usize, + range: Rg, ) -> core::result::Result<(), crate::error::BufferOverflow> { // reformat first to ensure data is optimally filling the // underlying buffer @@ -154,6 +157,19 @@ mod detail { // get an initial estimate for the size of the data let initial_size = sysex.payload_size(); + + let splice_begin = match range.start_bound() { + core::ops::Bound::Included(&v) => v, + core::ops::Bound::Excluded(&v) => v + 1, + core::ops::Bound::Unbounded => 0, + }; + let splice_end = match range.end_bound() { + core::ops::Bound::Included(&v) => v + 1, + core::ops::Bound::Excluded(&v) => v, + core::ops::Bound::Unbounded => initial_size, + }; + let splice_size = splice_end - splice_begin; + let mut running_data_size_estimate = match data.size_hint() { (_, Some(upper)) => upper, // not the optimal case - could lead to additional copying @@ -163,27 +179,26 @@ mod detail { let mut additional_size_for_overflow = 1; let mut data = data.peekable(); - // initial buffer resize - if let Err(SysexTryResizeError(sz)) = - resize(sysex, running_data_size_estimate + initial_size) - { - // failed. we'll work with what we've got - running_data_size_estimate = sz.saturating_sub(initial_size); - }; - - debug_assert_eq!( - sysex.payload_size(), - running_data_size_estimate + initial_size - ); + if splice_end < splice_end + running_data_size_estimate - splice_size { + // we need to grow + // initial buffer resize + if let Err(SysexTryResizeError(sz)) = resize( + sysex, + running_data_size_estimate + initial_size - splice_size, + ) { + // failed. we'll work with what we've got + running_data_size_estimate = sz.saturating_sub(initial_size - splice_size); + }; + } - let mut tail = before + running_data_size_estimate; - sysex.move_payload_tail(before, tail); + let mut tail = splice_end + running_data_size_estimate - splice_size; + sysex.move_payload_tail(splice_end, tail); 'main: loop { while written < running_data_size_estimate { match data.next() { Some(v) => { - sysex.write_datum(v, before + written); + sysex.write_datum(v, splice_begin + written); written += 1; } None => { @@ -199,16 +214,29 @@ mod detail { } // we underestimated. - // resize to make more space running_data_size_estimate += additional_size_for_overflow; - if let Err(SysexTryResizeError(sz)) = - resize(sysex, running_data_size_estimate + initial_size) + { - // failed. we'll work with what we've got - running_data_size_estimate = sz.saturating_sub(initial_size); - }; - sysex.move_payload_tail(tail, before + running_data_size_estimate); - tail = before + running_data_size_estimate; + let mut to = splice_begin + running_data_size_estimate; + if tail < to { + // we need to grow + // resize to make more space + // and move tail + + if let Err(SysexTryResizeError(sz)) = resize( + sysex, + running_data_size_estimate + initial_size - splice_size, + ) { + // failed. we'll work with what we've got + running_data_size_estimate = sz.saturating_sub(initial_size - splice_size); + to = splice_begin + running_data_size_estimate - splice_size; + }; + } + + sysex.move_payload_tail(tail, to); + tail = splice_begin + running_data_size_estimate; + } + additional_size_for_overflow *= 2; if written >= running_data_size_estimate { @@ -216,11 +244,11 @@ mod detail { } } - if written < running_data_size_estimate { - // we shrink the buffer back down to the correct size - sysex.move_payload_tail(tail, before + written); - resize(sysex, written + initial_size).map_err(|_| crate::error::BufferOverflow)?; - } + // we ensure the buffer is the correct size and move the tail + // to the final position + sysex.move_payload_tail(tail, splice_begin + written); + resize(sysex, written + initial_size - splice_size) + .map_err(|_| crate::error::BufferOverflow)?; Ok(()) } diff --git a/midi2/src/detail/test_support/rubbish_payload_iterator.rs b/midi2/src/detail/test_support/rubbish_payload_iterator.rs index d70f116..b79954e 100644 --- a/midi2/src/detail/test_support/rubbish_payload_iterator.rs +++ b/midi2/src/detail/test_support/rubbish_payload_iterator.rs @@ -1,22 +1,37 @@ /// This is for testing payload insertion implementation on sysex messages. /// The iterator returns no size hints so the optimisation case for these /// payload insertion implementations will hit their worst case for mem-copying. -pub struct RubbishPayloadIterator(u8); +pub struct RubbishPayloadIterator>(I); -impl RubbishPayloadIterator { +const DEFAULT_DATA: [u8; 50] = [ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, + 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, +]; + +impl RubbishPayloadIterator>> { pub fn new() -> Self { - RubbishPayloadIterator(0) + RubbishPayloadIterator(DEFAULT_DATA.iter().cloned()) + } +} + +impl> core::convert::From for RubbishPayloadIterator { + fn from(iter: I) -> Self { + RubbishPayloadIterator(iter) } } -impl core::iter::Iterator for RubbishPayloadIterator { +impl> core::iter::Iterator for RubbishPayloadIterator { type Item = u8; fn next(&mut self) -> Option { - if self.0 == 50 { - return None; - } - let ret = Some(self.0); - self.0 += 1; - ret + self.0.next() + } +} + +mod tests { + use super::*; + + #[test] + fn rubbish_iterator_should_give_worst_case_bounds() { + assert_eq!(RubbishPayloadIterator::new().size_hint(), (0, None)); } } diff --git a/midi2/src/sysex7.rs b/midi2/src/sysex7.rs index 51ceeb5..e754d17 100644 --- a/midi2/src/sysex7.rs +++ b/midi2/src/sysex7.rs @@ -496,24 +496,26 @@ impl Sysex for Sysex7 { } } - fn insert_payload(&mut self, data: D, index: usize) + fn splice_payload(&mut self, data: D, range: R) where D: core::iter::Iterator, B: crate::buffer::BufferMut + crate::buffer::BufferResize, + R: core::ops::RangeBounds, { - message_helpers::insert_sysex_data(self, data, index) + message_helpers::splice_sysex_data(self, data, range) } - fn try_insert_payload( + fn try_splice_payload( &mut self, data: D, - index: usize, + range: R, ) -> core::result::Result<(), crate::error::BufferOverflow> where D: core::iter::Iterator, B: crate::buffer::BufferMut + crate::buffer::BufferTryResize, + R: core::ops::RangeBounds, { - message_helpers::try_insert_sysex_data(self, data, index) + message_helpers::try_splice_sysex_data(self, data, range) } fn payload_size(&self) -> usize { @@ -2419,4 +2421,121 @@ mod tests { fn append_payload_bytes() { append_payload::(); } + + fn splice_payload() { + let mut message = Sysex7::>::new(); + message.set_payload((0..20).map(u7::new)); + message.splice_payload(core::iter::repeat_n(u7::new(0x7F), 30), 5..10); + assert_eq!( + message.payload().collect::>(), + [ + 0, 1, 2, 3, 4, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + ] + .iter() + .cloned() + .map(u7::new) + .collect::>(), + ); + } + + #[test] + fn splice_payload_ump() { + splice_payload::(); + } + + #[test] + fn splice_payload_bytes() { + splice_payload::(); + } + + fn splice_rubbish_payload() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex7::>::new(); + message.set_payload((0..20).map(u7::new)); + message.splice_payload(RubbishPayloadIterator::new().map(u7::new), 7..16); + assert_eq!( + message.payload().collect::>(), + [ + 0, 1, 2, 3, 4, 5, 6, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, + 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, + 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 16, 17, 18, 19, + ] + .iter() + .cloned() + .map(u7::new) + .collect::>(), + ); + } + + #[test] + fn splice_rubbish_payload_ump() { + splice_rubbish_payload::(); + } + + #[test] + fn splice_rubbish_payload_bytes() { + splice_rubbish_payload::(); + } + + fn splice_rubbish_payload_front() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex7::>::new(); + message.set_payload((0..20).map(u7::new)); + message.splice_payload(RubbishPayloadIterator::new().map(u7::new), 0..2); + assert_eq!( + message.payload().collect::>(), + [ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, + 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, + 44, 45, 46, 47, 48, 49, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, + 19, + ] + .iter() + .cloned() + .map(u7::new) + .collect::>(), + ); + } + + #[test] + fn splice_rubbish_payload_front_ump() { + splice_rubbish_payload_front::(); + } + + #[test] + fn splice_rubbish_payload_front_bytes() { + splice_rubbish_payload_front::(); + } + + fn splice_rubbish_payload_end() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex7::>::new(); + message.set_payload((0..20).map(u7::new)); + message.splice_payload(RubbishPayloadIterator::new().map(u7::new), 19..20); + assert_eq!( + message.payload().collect::>(), + [ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 0, 1, 2, 3, 4, 5, + 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, + 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, + 49, + ] + .iter() + .cloned() + .map(u7::new) + .collect::>(), + ); + } + + #[test] + fn splice_rubbish_payload_end_ump() { + splice_rubbish_payload_end::(); + } + + #[test] + fn splice_rubbish_payload_end_bytes() { + splice_rubbish_payload_end::(); + } } diff --git a/midi2/src/sysex8.rs b/midi2/src/sysex8.rs index 9637a33..2b25302 100644 --- a/midi2/src/sysex8.rs +++ b/midi2/src/sysex8.rs @@ -392,23 +392,26 @@ impl Sysex for Sysex8 { packet_index += 1; } } - fn insert_payload(&mut self, data: D, index: usize) + fn splice_payload(&mut self, data: D, range: R) where D: core::iter::Iterator, B: crate::buffer::BufferMut + crate::buffer::BufferResize, + R: core::ops::RangeBounds, { - message_helpers::insert_sysex_data(self, data, index) + message_helpers::splice_sysex_data(self, data, range) } - fn try_insert_payload( + + fn try_splice_payload( &mut self, data: D, - index: usize, + range: R, ) -> core::result::Result<(), crate::error::BufferOverflow> where D: core::iter::Iterator, B: crate::buffer::BufferMut + crate::buffer::BufferTryResize, + R: core::ops::RangeBounds, { - message_helpers::try_insert_sysex_data(self, data, index) + message_helpers::try_splice_sysex_data(self, data, range) } } @@ -1983,4 +1986,88 @@ mod tests { message.try_set_payload(core::iter::empty()).unwrap(); assert_eq!(message.payload_size(), 0); } + + #[test] + fn splice_payload_inserted_data_exceeds_replaced() { + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(core::iter::repeat(0x6_u8).take(10), 5..10); + assert_eq!( + message.payload().collect::>(), + std::vec![ + 0, 1, 2, 3, 4, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 10, 11, 12, 13, 14, 15, 16, 17, 18, + 19, + ], + ); + } + + #[test] + fn splice_payload_inserted_data_less_than_replaced() { + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(core::iter::repeat(0x6_u8).take(3), 5..10); + assert_eq!( + message.payload().collect::>(), + std::vec![0, 1, 2, 3, 4, 6, 6, 6, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19,], + ); + } + + #[test] + fn splice_payload_inserted_data_equal_in_length_to_replaced() { + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(core::iter::repeat(0x6_u8).take(5), 5..10); + assert_eq!( + message.payload().collect::>(), + std::vec![0, 1, 2, 3, 4, 6, 6, 6, 6, 6, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19,], + ); + } + + #[test] + fn splice_rubbish_payload() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(RubbishPayloadIterator::new(), 5..10); + assert_eq!( + message.payload().collect::>(), + std::vec![ + 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, + 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + ], + ); + } + + #[test] + fn splice_rubbish_payload_at_the_end() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(RubbishPayloadIterator::new(), 15..20); + assert_eq!( + message.payload().collect::>(), + std::vec![ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, + 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, + 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + ], + ); + } + + #[test] + fn splice_rubbish_payload_at_the_start() { + use crate::detail::test_support::rubbish_payload_iterator::RubbishPayloadIterator; + let mut message = Sysex8::>::new(); + message.set_payload(0..20); + message.splice_payload(RubbishPayloadIterator::new(), 0..5); + assert_eq!( + message.payload().collect::>(), + std::vec![ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, + 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, + 44, 45, 46, 47, 48, 49, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + ], + ); + } } diff --git a/midi2/src/traits.rs b/midi2/src/traits.rs index fae4d8a..348a5f9 100644 --- a/midi2/src/traits.rs +++ b/midi2/src/traits.rs @@ -561,7 +561,11 @@ pub trait Sysex { fn insert_payload(&mut self, data: D, index: usize) where D: core::iter::Iterator, - B: crate::buffer::BufferMut + crate::buffer::BufferResize; + B: crate::buffer::BufferMut + crate::buffer::BufferResize, + { + self.splice_payload(data, index..index) + } + /// Insert the provided byte data before position `index` /// /// # Fails @@ -578,7 +582,10 @@ pub trait Sysex { ) -> core::result::Result<(), crate::error::BufferOverflow> where D: core::iter::Iterator, - B: crate::buffer::BufferMut + crate::buffer::BufferTryResize; + B: crate::buffer::BufferMut + crate::buffer::BufferTryResize, + { + self.try_splice_payload(data, index..index) + } /// Pushes the provided payload data iterator into the back of the /// existing message payload. @@ -607,6 +614,27 @@ pub trait Sysex { self.try_insert_payload(data, self.payload_size()) } + /// Replaces the specified payload range with the given `data` iterator. + /// `data` does not need to be the same length as range. + fn splice_payload(&mut self, data: D, range: R) + where + D: core::iter::Iterator, + B: crate::buffer::BufferMut + crate::buffer::BufferResize, + R: core::ops::RangeBounds; + + /// Attempt to replace the specified payload range with the given `data` iterator. + /// `data` does not need to be the same length as range. + /// Fails if the underlying buffer cannot resize to accommodate the new data. + fn try_splice_payload( + &mut self, + data: D, + range: R, + ) -> core::result::Result<(), crate::error::BufferOverflow> + where + D: core::iter::Iterator, + B: crate::buffer::BufferMut + crate::buffer::BufferTryResize, + R: core::ops::RangeBounds; + /// Pushes the provided byte into the back of the /// existing message payload. fn append_byte(&mut self, byte: Self::Byte)