diff --git a/crates/nu-command/tests/commands/bytes/at.rs b/crates/nu-command/tests/commands/bytes/at.rs index 32514b36fe..4a8d034b3f 100644 --- a/crates/nu-command/tests/commands/bytes/at.rs +++ b/crates/nu-command/tests/commands/bytes/at.rs @@ -5,18 +5,27 @@ pub fn returns_error_for_relative_range_on_infinite_stream() { let actual = nu!("nu --testbin iecho 3 | bytes at ..-3"); assert!( actual.err.contains( - "Relative range values cannot be used with streams that don't specify a length" + "Relative range values cannot be used with streams that don't have a known length" ), "Expected error message for negative range with infinite stream" ); } #[test] -pub fn returns_bytes_for_fixed_range_on_infinite_stream() { +pub fn returns_bytes_for_fixed_range_on_infinite_stream_including_end() { let actual = nu!("nu --testbin iecho 3 | bytes at ..10 | decode"); assert_eq!( actual.out, "33333", - "Expected bytes from index 1 to 10, but got different output" + "Expected bytes from index 0 to 10, but got different output" + ); +} + +#[test] +pub fn returns_bytes_for_fixed_range_on_infinite_stream_excluding_end() { + let actual = nu!("nu --testbin iecho 3 | bytes at ..<9 | decode"); + assert_eq!( + actual.out, "3333", + "Expected bytes from index 0 to 8, but got different output" ); } @@ -28,7 +37,7 @@ pub fn test_string_returns_correct_slice_for_simple_positive_slice() { #[test] pub fn test_string_returns_correct_slice_for_negative_start() { - let actual = nu!("\"Hello World\" | encode utf8 | bytes at 6..11 | decode"); + let actual = nu!("\"Hello World\" | encode utf8 | bytes at (-5)..10 | decode"); assert_eq!(actual.out, "World"); } @@ -46,7 +55,7 @@ pub fn test_string_returns_correct_slice_for_empty_slice() { #[test] pub fn test_string_returns_correct_slice_for_out_of_bounds() { - let actual = nu!("\"Hello World\" | encode utf8 | bytes at 0..20 | decode"); + let actual = nu!("\"Hello World\" | encode utf8 | bytes at ..20 | decode"); assert_eq!(actual.out, "Hello World"); } diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index 96678daef5..9590987a0d 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -653,6 +653,18 @@ pub enum ShellError { creation_site: Span, }, + /// Attempted to us a relative range on an infinite stream + /// + /// ## Resolution + /// + /// Ensure that either the range is absolute or the stream has a known length. + #[error("Relative range values cannot be used with streams that don't have a known length")] + #[diagnostic(code(nu::shell::relative_range_on_infinite_stream))] + RelativeRangeOnInfiniteStream { + #[label = "Relative range values cannot be used with streams that don't have a known length"] + span: Span, + }, + /// An error happened while performing an external command. /// /// ## Resolution diff --git a/crates/nu-protocol/src/pipeline/byte_stream.rs b/crates/nu-protocol/src/pipeline/byte_stream.rs index e30b788666..159a270418 100644 --- a/crates/nu-protocol/src/pipeline/byte_stream.rs +++ b/crates/nu-protocol/src/pipeline/byte_stream.rs @@ -213,15 +213,14 @@ impl ByteStream { } pub fn skip(self, span: Span, n: u64) -> Result { + let known_size = self.known_size.map(|len| len.saturating_sub(n)); if let Some(mut reader) = self.reader() { // Copy the number of skipped bytes into the sink before proceeding io::copy(&mut (&mut reader).take(n), &mut io::sink()).err_span(span)?; - Ok(ByteStream::read( - reader, - span, - Signals::empty(), - ByteStreamType::Binary, - )) + Ok( + ByteStream::read(reader, span, Signals::empty(), ByteStreamType::Binary) + .with_known_size(known_size), + ) } else { Err(ShellError::TypeMismatch { err_message: "expected readable stream".into(), @@ -231,13 +230,15 @@ impl ByteStream { } pub fn take(self, span: Span, n: u64) -> Result { + let known_size = self.known_size.map(|s| s.min(n)); if let Some(reader) = self.reader() { Ok(ByteStream::read( reader.take(n), span, Signals::empty(), ByteStreamType::Binary, - )) + ) + .with_known_size(known_size)) } else { Err(ShellError::TypeMismatch { err_message: "expected readable stream".into(), @@ -248,45 +249,38 @@ impl ByteStream { pub fn slice( self, - range_span: Span, + val_span: Span, call_span: Span, range: IntRange, ) -> Result { - match self.known_size { - Some(len) => { - let absolute_start = range.absolute_start(len); - match range.absolute_end(len) { - Bound::Unbounded => self.skip(range_span, absolute_start), - Bound::Included(end) => self.skip(range_span, absolute_start) - .and_then(|stream| stream.take(range_span, end.saturating_sub(absolute_start) + 1)), - Bound::Excluded(end) => self.skip(range_span, absolute_start) - .and_then(|stream| stream.take(range_span, end.saturating_sub(absolute_start))), + if let Some(len) = self.known_size { + let start = range.absolute_start(len); + let stream = self.skip(val_span, start); + + match range.absolute_end(len) { + Bound::Unbounded => stream, + Bound::Included(end) | Bound::Excluded(end) if end < start => { + stream.and_then(|s| s.take(val_span, 0)) + } + Bound::Included(end) => { + let distance = end - start + 1; + stream.and_then(|s| s.take(val_span, distance.min(len))) + } + Bound::Excluded(end) => { + let distance = end - start; + stream.and_then(|s| s.take(val_span, distance.min(len))) } } - None => match range.is_relative() { - true => Err(ShellError::IncorrectValue { - msg: - "Relative range values cannot be used with streams that don't specify a length" - .into(), - val_span: range_span, - call_span, - }), - false => { - let skip = range.start(); - match range.end() { - std::ops::Bound::Unbounded => self.skip(range_span, skip as u64), - std::ops::Bound::Included(end) => match end - skip { - take if take < 0 => self.take(range_span, 0), - take => self.skip(range_span, skip as u64) - .and_then(|stream| stream.take(range_span, take as u64 + 1)), - }, - std::ops::Bound::Excluded(end) => match end - skip { - take if take < 0 => self.take(range_span, 0), - take => self.skip(range_span, skip as u64) - .and_then(|stream| stream.take(range_span, take as u64)), - }, - } - } + } else if range.is_relative() { + Err(ShellError::RelativeRangeOnInfiniteStream { span: call_span }) + } else { + let start = range.start() as u64; + let stream = self.skip(val_span, start); + + match range.distance() { + Bound::Unbounded => stream, + Bound::Included(distance) => stream.and_then(|s| s.take(val_span, distance)), + Bound::Excluded(distance) => stream.and_then(|s| s.take(val_span, distance - 1)), } } } diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 4a55e5c832..9e9c252fdd 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -82,9 +82,22 @@ mod int_range { } pub fn absolute_start(&self, len: u64) -> u64 { + let max_index = len - 1; match self.start { start if start < 0 => len.saturating_sub(start.unsigned_abs().into()), - start => start as u64, + start => max_index.min(start as u64), + } + } + + /// Returns the distance between the start and end of the range + /// The result will always be 0 or positive + pub fn distance(&self) -> Bound { + match self.end { + Bound::Unbounded => Bound::Unbounded, + Bound::Included(end) if self.start > end => Bound::Included(0), + Bound::Excluded(end) if self.start > end => Bound::Excluded(0), + Bound::Included(end) => Bound::Included((end - self.start) as u64), + Bound::Excluded(end) => Bound::Excluded((end - self.start) as u64), } } @@ -93,15 +106,16 @@ mod int_range { } pub fn absolute_end(&self, len: u64) -> Bound { + let max_index = len - 1; match self.end { Bound::Unbounded => Bound::Unbounded, Bound::Included(i) => Bound::Included(match i { i if i < 0 => len.saturating_sub(i.unsigned_abs() as u64), - i => i as u64, + i => max_index.min(i as u64), }), Bound::Excluded(i) => Bound::Excluded(match i { i if i < 0 => len.saturating_sub(i.unsigned_abs() as u64), - i => i as u64, + i => len.min(i as u64), }), } } diff --git a/crates/nu-protocol/tests/pipeline/byte_stream.rs b/crates/nu-protocol/tests/pipeline/byte_stream.rs index a659188cdc..3d06573461 100644 --- a/crates/nu-protocol/tests/pipeline/byte_stream.rs +++ b/crates/nu-protocol/tests/pipeline/byte_stream.rs @@ -12,7 +12,8 @@ pub fn test_simple_positive_slice_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello"); } #[test] @@ -27,7 +28,8 @@ pub fn test_negative_start_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "World"); } #[test] @@ -42,7 +44,8 @@ pub fn test_negative_end_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello"); } #[test] @@ -57,7 +60,8 @@ pub fn test_negative_start_and_end_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Wor"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Wor"); } #[test] @@ -72,7 +76,8 @@ pub fn test_empty_slice_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, Vec::::new()); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, ""); } #[test] @@ -87,7 +92,8 @@ pub fn test_out_of_bounds_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello World"); } #[test] @@ -102,7 +108,8 @@ pub fn test_invalid_range_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, Vec::::new()); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, ""); } #[test] @@ -117,7 +124,8 @@ pub fn test_max_end_exclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "World"); } #[test] @@ -132,7 +140,8 @@ pub fn test_simple_positive_slice_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello"); } #[test] @@ -147,7 +156,8 @@ pub fn test_negative_start_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "World"); } #[test] @@ -162,7 +172,8 @@ pub fn test_negative_end_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello"); } #[test] @@ -177,7 +188,8 @@ pub fn test_negative_start_and_end_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "World"); } #[test] @@ -192,7 +204,8 @@ pub fn test_empty_slice_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b" "); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, " "); } #[test] @@ -207,7 +220,8 @@ pub fn test_out_of_bounds_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"Hello World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "Hello World"); } #[test] @@ -222,7 +236,8 @@ pub fn test_invalid_range_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, Vec::::new()); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, ""); } #[test] @@ -237,7 +252,8 @@ pub fn test_max_end_inclusive() { ) .unwrap(); let result = sliced.into_bytes().unwrap(); - assert_eq!(result, b"World"); + let result = String::from_utf8(result).unwrap(); + assert_eq!(result, "World"); } fn create_range(start: i64, end: i64, inclusion: RangeInclusion) -> IntRange {