Skip to content

Commit

Permalink
Fix panic when redirecting nothing (#12970)
Browse files Browse the repository at this point in the history 
# Description
Fixes #12969 where the parser can panic if a redirection is applied to
nothing / an empty command.

# Tests + Formatting
Added a test.
  • Loading branch information
@IanManske
IanManske committed May 27, 2024
1 parent f74dd33 commit 6012af2
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 84 deletions.
148 changes: 64 additions & 84 deletions crates/nu-parser/src/lite_parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 => {
Expand Down
39 changes: 39 additions & 0 deletions crates/nu-parser/tests/test_parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 8 additions & 0 deletions crates/nu-protocol/src/errors/parse_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 6012af2

Please sign in to comment.