diff options
author | Ian Manske <ian.manske@pm.me> | 2024-05-27 02:03:06 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-27 10:03:06 +0800 |
commit | 6012af24124f5b49c6f1d885a765f1c31e0a8631 (patch) | |
tree | e815863212c33862f5800c1c7e1e2a2578c40ce4 | |
parent | f74dd33ba9213e85e08ee96ba25558d1a89f1fb2 (diff) |
Fix panic when redirecting nothing (#12970)
# Description
Fixes #12969 where the parser can panic if a redirection is applied to
nothing / an empty command.
# Tests + Formatting
Added a test.
-rw-r--r-- | crates/nu-parser/src/lite_parser.rs | 148 | ||||
-rw-r--r-- | crates/nu-parser/tests/test_parser.rs | 39 | ||||
-rw-r--r-- | crates/nu-protocol/src/errors/parse_error.rs | 8 |
3 files changed, 111 insertions, 84 deletions
diff --git a/crates/nu-parser/src/lite_parser.rs b/crates/nu-parser/src/lite_parser.rs index 65bdc4763..f04b8befd 100644 --- a/crates/nu-parser/src/lite_parser.rs +++ b/crates/nu-parser/src/lite_parser.rs @@ -51,12 +51,21 @@ impl LiteCommand { self.parts.push(span); } + fn check_accepts_redirection(&self, span: Span) -> Option<ParseError> { + self.parts + .is_empty() + .then_some(ParseError::UnexpectedRedirection { span }) + } + fn try_add_redirection( &mut self, source: RedirectionSource, target: LiteRedirectionTarget, ) -> Result<(), ParseError> { let redirection = match (self.redirection.take(), source) { + (None, _) if self.parts.is_empty() => Err(ParseError::UnexpectedRedirection { + span: target.connector(), + }), (None, source) => Ok(LiteRedirection::Single { source, target }), ( Some(LiteRedirection::Single { @@ -162,94 +171,59 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option<ParseError>) { for (idx, token) in tokens.iter().enumerate() { if let Some((source, append, span)) = file_redirection.take() { - if command.parts.is_empty() { - error = error.or(Some(ParseError::LabeledError( - "Redirection without command or expression".into(), - "there is nothing to redirect".into(), - span, - ))); - - command.push(span); - - match token.contents { - TokenContents::Comment => { - command.comments.push(token.span); - curr_comment = None; - } - TokenContents::Pipe - | TokenContents::ErrGreaterPipe - | TokenContents::OutErrGreaterPipe => { - pipeline.push(&mut command); - command.pipe = Some(token.span); - } - TokenContents::Semicolon => { - pipeline.push(&mut command); - block.push(&mut pipeline); - } - TokenContents::Eol => { - pipeline.push(&mut command); - } - _ => command.push(token.span), + match &token.contents { + TokenContents::PipePipe => { + error = error.or(Some(ParseError::ShellOrOr(token.span))); + command.push(span); + command.push(token.span); } - } else { - match &token.contents { - TokenContents::PipePipe => { - error = error.or(Some(ParseError::ShellOrOr(token.span))); - command.push(span); - command.push(token.span); - } - TokenContents::Item => { - let target = LiteRedirectionTarget::File { - connector: span, - file: token.span, - append, - }; - if let Err(err) = command.try_add_redirection(source, target) { - error = error.or(Some(err)); - command.push(span); - command.push(token.span) - } - } - TokenContents::OutGreaterThan - | TokenContents::OutGreaterGreaterThan - | TokenContents::ErrGreaterThan - | TokenContents::ErrGreaterGreaterThan - | TokenContents::OutErrGreaterThan - | TokenContents::OutErrGreaterGreaterThan => { - error = - error.or(Some(ParseError::Expected("redirection target", token.span))); - command.push(span); - command.push(token.span); - } - TokenContents::Pipe - | TokenContents::ErrGreaterPipe - | TokenContents::OutErrGreaterPipe => { - error = - error.or(Some(ParseError::Expected("redirection target", token.span))); - command.push(span); - pipeline.push(&mut command); - command.pipe = Some(token.span); - } - TokenContents::Eol => { - error = - error.or(Some(ParseError::Expected("redirection target", token.span))); - command.push(span); - pipeline.push(&mut command); - } - TokenContents::Semicolon => { - error = - error.or(Some(ParseError::Expected("redirection target", token.span))); - command.push(span); - pipeline.push(&mut command); - block.push(&mut pipeline); - } - TokenContents::Comment => { - error = error.or(Some(ParseError::Expected("redirection target", span))); + TokenContents::Item => { + let target = LiteRedirectionTarget::File { + connector: span, + file: token.span, + append, + }; + if let Err(err) = command.try_add_redirection(source, target) { + error = error.or(Some(err)); command.push(span); - command.comments.push(token.span); - curr_comment = None; + command.push(token.span) } } + TokenContents::OutGreaterThan + | TokenContents::OutGreaterGreaterThan + | TokenContents::ErrGreaterThan + | TokenContents::ErrGreaterGreaterThan + | TokenContents::OutErrGreaterThan + | TokenContents::OutErrGreaterGreaterThan => { + error = error.or(Some(ParseError::Expected("redirection target", token.span))); + command.push(span); + command.push(token.span); + } + TokenContents::Pipe + | TokenContents::ErrGreaterPipe + | TokenContents::OutErrGreaterPipe => { + error = error.or(Some(ParseError::Expected("redirection target", token.span))); + command.push(span); + pipeline.push(&mut command); + command.pipe = Some(token.span); + } + TokenContents::Eol => { + error = error.or(Some(ParseError::Expected("redirection target", token.span))); + command.push(span); + pipeline.push(&mut command); + } + TokenContents::Semicolon => { + error = error.or(Some(ParseError::Expected("redirection target", token.span))); + command.push(span); + pipeline.push(&mut command); + block.push(&mut pipeline); + } + TokenContents::Comment => { + error = error.or(Some(ParseError::Expected("redirection target", span))); + command.push(span); + command.comments.push(token.span); + curr_comment = None; + } } } else { match &token.contents { @@ -278,22 +252,28 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option<ParseError>) { command.push(token.span); } TokenContents::OutGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::Stdout, false, token.span)); } TokenContents::OutGreaterGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::Stdout, true, token.span)); } TokenContents::ErrGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::Stderr, false, token.span)); } TokenContents::ErrGreaterGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::Stderr, true, token.span)); } TokenContents::OutErrGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::StdoutAndStderr, false, token.span)); } TokenContents::OutErrGreaterGreaterThan => { + error = error.or(command.check_accepts_redirection(token.span)); file_redirection = Some((RedirectionSource::StdoutAndStderr, true, token.span)); } TokenContents::ErrGreaterPipe => { diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 9b8ae9431..ef5d96828 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -727,6 +727,45 @@ fn test_redirection_with_letmut(#[case] phase: &[u8]) { )); } +#[rstest] +#[case(b"o>")] +#[case(b"o>>")] +#[case(b"e>")] +#[case(b"e>>")] +#[case(b"o+e>")] +#[case(b"o+e>>")] +#[case(b"e>|")] +#[case(b"o+e>|")] +#[case(b"|o>")] +#[case(b"|o>>")] +#[case(b"|e>")] +#[case(b"|e>>")] +#[case(b"|o+e>")] +#[case(b"|o+e>>")] +#[case(b"|e>|")] +#[case(b"|o+e>|")] +#[case(b"e> file")] +#[case(b"e>> file")] +#[case(b"o> file")] +#[case(b"o>> file")] +#[case(b"o+e> file")] +#[case(b"o+e>> file")] +#[case(b"|e> file")] +#[case(b"|e>> file")] +#[case(b"|o> file")] +#[case(b"|o>> file")] +#[case(b"|o+e> file")] +#[case(b"|o+e>> file")] +fn test_redirecting_nothing(#[case] text: &[u8]) { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + let _ = parse(&mut working_set, None, text, true); + assert!(matches!( + working_set.parse_errors.first(), + Some(ParseError::UnexpectedRedirection { .. }) + )); +} + #[test] fn test_nothing_comparison_neq() { let engine_state = EngineState::new(); diff --git a/crates/nu-protocol/src/errors/parse_error.rs b/crates/nu-protocol/src/errors/parse_error.rs index 7e39fe1ef..28c865b84 100644 --- a/crates/nu-protocol/src/errors/parse_error.rs +++ b/crates/nu-protocol/src/errors/parse_error.rs @@ -93,6 +93,13 @@ pub enum ParseError { #[label = "second redirection"] Span, ), + #[error("Unexpected redirection.")] + #[diagnostic(code(nu::parser::unexpected_redirection))] + UnexpectedRedirection { + #[label = "redirecting nothing"] + span: Span, + }, + #[error("{0} is not supported on values of type {3}")] #[diagnostic(code(nu::parser::unsupported_operation))] UnsupportedOperationLHS( @@ -564,6 +571,7 @@ impl ParseError { ParseError::ShellErrRedirect(s) => *s, ParseError::ShellOutErrRedirect(s) => *s, ParseError::MultipleRedirections(_, _, s) => *s, + ParseError::UnexpectedRedirection { span } => *span, ParseError::UnknownOperator(_, _, s) => *s, ParseError::InvalidLiteral(_, _, s) => *s, ParseError::LabeledErrorWithHelp { span: s, .. } => *s, |