Repository: reubeno/brush (rust)

Issue:
`unterminated here document sequence` using `brush -c ....` argument from fish
I'm mainly driving Fish, but now and then I need to run heredocs from the internet. I do it by running `bash -c "<heredoc>"` and decided to give it a go with brush, but it didn't work.

Here is the minimized (but still reasonable) example:
```
# NOTE: This works only when ran from Fish

❯ bash -c "cat <<EOF
  test
  EOF"
test

❯ brush -c "cat <<EOF
  test
  EOF"
ERROR main: unterminated here document sequence; tag(s) [EOF] found at: [line 1 col 10] (detected near line 3 column 4)
```

When I move the double quote to a new line, it works fine:
```
❯ bash -c "cat <<EOF
  test
  EOF
  "
test

❯ brush -c "cat <<EOF
  test
  EOF
  "
test
```

<details><summary>Tokenizer debug log:</summary>
<p>

```
❯ brush --debug tokenize -c "cat <<EOF
  test
  EOF
  "
DEBUG Tokenizing...
DEBUG TOKEN 0: Word("cat", TokenLocation { start: SourcePosition { index: 0, line: 1, column: 1 }, end: SourcePosition { index: 3, line: 1, column: 4 } }) NonNewLineBlank
DEBUG TOKEN 1: Operator("<<", TokenLocation { start: SourcePosition { index: 4, line: 1, column: 5 }, end: SourcePosition { index: 6, line: 1, column: 7 } }) OperatorEnd
DEBUG TOKEN 2: Word("EOF", TokenLocation { start: SourcePosition { index: 6, line: 1, column: 7 }, end: SourcePosition { index: 9, line: 1, column: 10 } }) OperatorStart
DEBUG TOKEN 3: Word("test\n", TokenLocation { start: SourcePosition { index: 10, line: 2, column: 1 }, end: SourcePosition { index: 19, line: 4, column: 1 } }) HereDocumentBodyEnd
DEBUG TOKEN 4: Word("EOF", TokenLocation { start: SourcePosition { index: 19, line: 4, column: 1 }, end: SourcePosition { index: 19, line: 4, column: 1 } }) HereDocumentEndTag
DEBUG TOKEN 5: Operator("\n", TokenLocation { start: SourcePosition { index: 9, line: 1, column: 10 }, end: SourcePosition { index: 10, line: 2, column: 1 } }) UnescapedNewLine
DEBUG   => 6 token(s)
test

~/tmp/export-dynamic
❯ brush --debug tokenize -c "cat <<EOF
  test
  EOF"
DEBUG Tokenizing...
DEBUG TOKEN 0: Word("cat", TokenLocation { start: SourcePosition { index: 0, line: 1, column: 1 }, end: SourcePosition { index: 3, line: 1, column: 4 } }) NonNewLineBlank
ERROR main: unterminated here document sequence; tag(s) [EOF] found at: [line 1 col 10] (detected near line 3 column 4)
```

</p>
</details> 

Pull request:
fix(tokenizer): correctly handle here docs terminated by EOF
This fixes the tokenizer to handle a here document being terminated by the final tag, immediately followed by an EOF (rather than a newline).

_Huge credit to @mati865 for finding the issue, identifying a targeted fix, and sharing it out._

Resolves #547 

Patch:
diff --git a/brush-parser/src/parser.rs b/brush-parser/src/parser.rs
index db2a5ca..5f22546 100644
--- a/brush-parser/src/parser.rs
+++ b/brush-parser/src/parser.rs
@@ -1043,6 +1043,29 @@ esac\
         Ok(())
     }
 
+    #[test]
+    fn parse_here_doc_with_no_trailing_newline() -> Result<()> {
+        let input = r"cat <<EOF
+Something
+EOF";
+
+        let tokens = tokenize_str(input)?;
+        let result = super::token_parser::program(
+            &Tokens {
+                tokens: tokens.as_slice(),
+            },
+            &ParserOptions::default(),
+            &SourceInfo::default(),
+        )?;
+
+        assert_ron_snapshot!(ParseResult {
+            input,
+            result: &result
+        });
+
+        Ok(())
+    }
+
     #[test]
     fn parse_function_with_pipe_redirection() -> Result<()> {
         let inputs = [r"foo() { echo 1; } 2>&1 | cat", r"foo() { echo 1; } |& cat"];
diff --git a/brush-parser/src/tokenizer.rs b/brush-parser/src/tokenizer.rs
index 29b6ae3..1f11e48 100644
--- a/brush-parser/src/tokenizer.rs
+++ b/brush-parser/src/tokenizer.rs
@@ -650,6 +650,11 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
 
                 // Verify we're not in a here document.
                 if !matches!(self.cross_state.here_state, HereState::None) {
+                    if self.remove_here_end_tag(&mut state, &mut result, false)? {
+                        // If we hit end tag without a trailing newline, try to get next token.
+                        continue;
+                    }
+
                     let tag_names = self
                         .cross_state
                         .current_here_tags
@@ -701,33 +706,7 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
 
                     // See if this was a newline character following the terminating here tag.
                     if c == '\n' {
-                        let next_here_tag = &self.cross_state.current_here_tags[0];
-                        let tag_str: Cow<'_, str> = if next_here_tag.tag_was_escaped_or_quoted {
-                            unquote_str(next_here_tag.tag.as_str()).into()
-                        } else {
-                            next_here_tag.tag.as_str().into()
-                        };
-
-                        if let Some(current_token_without_here_tag) =
-                            state.current_token().strip_suffix(tag_str.as_ref())
-                        {
-                            // Make sure that was either the start of the here document, or there
-                            // was a newline between the preceding part
-                            // and the tag.
-                            if current_token_without_here_tag.is_empty()
-                                || current_token_without_here_tag.ends_with('\n')
-                            {
-                                state.replace_with_here_doc(
-                                    current_token_without_here_tag.to_owned(),
-                                );
-
-                                // Delimit the end of the here-document body.
-                                result = state.delimit_current_token(
-                                    TokenEndReason::HereDocumentBodyEnd,
-                                    &mut self.cross_state,
-                                )?;
-                            }
-                        }
+                        self.remove_here_end_tag(&mut state, &mut result, true)?;
                     }
                 }
             } else if state.in_operator() {
@@ -1167,6 +1146,52 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
         Ok(result)
     }
 
+    fn remove_here_end_tag(
+        &mut self,
+        state: &mut TokenParseState,
+        result: &mut Option<TokenizeResult>,
+        ends_with_newline: bool,
+    ) -> Result<bool, TokenizerError> {
+        // Bail immediately if we don't even have a *starting* here tag.
+        if self.cross_state.current_here_tags.is_empty() {
+            return Ok(false);
+        }
+
+        let next_here_tag = &self.cross_state.current_here_tags[0];
+
+        let tag_str: Cow<'_, str> = if next_here_tag.tag_was_escaped_or_quoted {
+            unquote_str(next_here_tag.tag.as_str()).into()
+        } else {
+            next_here_tag.tag.as_str().into()
+        };
+
+        let tag_str = if !ends_with_newline {
+            tag_str.strip_suffix('\n').unwrap_or(tag_str.as_ref())
+        } else {
+            tag_str.as_ref()
+        };
+
+        if let Some(current_token_without_here_tag) = state.current_token().strip_suffix(tag_str) {
+            // Make sure that was either the start of the here document, or there
+            // was a newline between the preceding part
+            // and the tag.
+            if current_token_without_here_tag.is_empty()
+                || current_token_without_here_tag.ends_with('\n')
+            {
+                state.replace_with_here_doc(current_token_without_here_tag.to_owned());
+
+                // Delimit the end of the here-document body.
+                *result = state.delimit_current_token(
+                    TokenEndReason::HereDocumentBodyEnd,
+                    &mut self.cross_state,
+                )?;
+
+                return Ok(true);
+            }
+        }
+        Ok(false)
+    }
+
     fn can_start_extglob(c: char) -> bool {
         matches!(c, '@' | '!' | '?' | '+' | '*')
     }
@@ -1352,6 +1377,24 @@ SOMETHING
 HERE
 echo after
 "
+        )?);
+        assert_ron_snapshot!(test_tokenizer(
+            r"cat <<HERE
+SOMETHING
+HERE
+"
+        )?);
+        assert_ron_snapshot!(test_tokenizer(
+            r"cat <<HERE
+SOMETHING
+HERE
+
+"
+        )?);
+        assert_ron_snapshot!(test_tokenizer(
+            r"cat <<HERE
+SOMETHING
+HERE"
         )?);
         Ok(())
     }


Test patch:
diff --git a/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here.snap b/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here.snap
new file mode 100644
index 0000000..1ce0033
--- /dev/null
+++ b/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here.snap
@@ -0,0 +1,35 @@
+---
+source: brush-parser/src/parser.rs
+expression: "ParseResult { input, result: &result }"
+---
+ParseResult(
+  input: "cat <<EOF\nSomething\nEOF\n",
+  result: Program(
+    cmds: [
+      List([
+        Item(AndOr(
+          first: Pipeline(
+            seq: [
+              Simple(Simple(
+                w: Some(W(
+                  v: "cat",
+                )),
+                suffix: Some(Suffix([
+                  IoRedirect(HereDocument(None, IoHereDocument(
+                    requires_expansion: true,
+                    here_end: W(
+                      v: "EOF",
+                    ),
+                    doc: W(
+                      v: "Something\n",
+                    ),
+                  ))),
+                ])),
+              )),
+            ],
+          ),
+        ), Sequence),
+      ]),
+    ],
+  ),
+)
diff --git a/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here_doc_with_no_trailing_newline.snap b/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here_doc_with_no_trailing_newline.snap
new file mode 100644
index 0000000..083b98b
--- /dev/null
+++ b/brush-parser/src/snapshots/brush_parser__parser__tests__parse_here_doc_with_no_trailing_newline.snap
@@ -0,0 +1,35 @@
+---
+source: brush-parser/src/parser.rs
+expression: "ParseResult { input, result: &result }"
+---
+ParseResult(
+  input: "cat <<EOF\nSomething\nEOF",
+  result: Program(
+    cmds: [
+      List([
+        Item(AndOr(
+          first: Pipeline(
+            seq: [
+              Simple(Simple(
+                w: Some(W(
+                  v: "cat",
+                )),
+                suffix: Some(Suffix([
+                  IoRedirect(HereDocument(None, IoHereDocument(
+                    requires_expansion: true,
+                    here_end: W(
+                      v: "EOF",
+                    ),
+                    doc: W(
+                      v: "Something\n",
+                    ),
+                  ))),
+                ])),
+              )),
+            ],
+          ),
+        ), Sequence),
+      ]),
+    ],
+  ),
+)
diff --git a/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-2.snap b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-2.snap
new file mode 100644
index 0000000..7f169d9
--- /dev/null
+++ b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-2.snap
@@ -0,0 +1,81 @@
+---
+source: brush-parser/src/tokenizer.rs
+expression: "test_tokenizer(r\"cat <<HERE\nSOMETHING\nHERE\n\")?"
+---
+TokenizerResult(
+  input: "cat <<HERE\nSOMETHING\nHERE\n",
+  result: [
+    W("cat", Loc(
+      start: Pos(
+        idx: 0,
+        line: 1,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 3,
+        line: 1,
+        col: 4,
+      ),
+    )),
+    Op("<<", Loc(
+      start: Pos(
+        idx: 4,
+        line: 1,
+        col: 5,
+      ),
+      end: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+      end: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+    )),
+    W("SOMETHING\n", Loc(
+      start: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+    )),
+    Op("\n", Loc(
+      start: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+      end: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+    )),
+  ],
+)
diff --git a/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-3.snap b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-3.snap
new file mode 100644
index 0000000..84bfc47
--- /dev/null
+++ b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-3.snap
@@ -0,0 +1,93 @@
+---
+source: brush-parser/src/tokenizer.rs
+expression: "test_tokenizer(r\"cat <<HERE\nSOMETHING\nHERE\n\n\")?"
+---
+TokenizerResult(
+  input: "cat <<HERE\nSOMETHING\nHERE\n\n",
+  result: [
+    W("cat", Loc(
+      start: Pos(
+        idx: 0,
+        line: 1,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 3,
+        line: 1,
+        col: 4,
+      ),
+    )),
+    Op("<<", Loc(
+      start: Pos(
+        idx: 4,
+        line: 1,
+        col: 5,
+      ),
+      end: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+      end: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+    )),
+    W("SOMETHING\n", Loc(
+      start: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+    )),
+    Op("\n", Loc(
+      start: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+      end: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+    )),
+    Op("\n", Loc(
+      start: Pos(
+        idx: 26,
+        line: 4,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 27,
+        line: 5,
+        col: 1,
+      ),
+    )),
+  ],
+)
diff --git a/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-4.snap b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-4.snap
new file mode 100644
index 0000000..7ac2031
--- /dev/null
+++ b/brush-parser/src/snapshots/brush_parser__tokenizer__tests__tokenize_here_doc-4.snap
@@ -0,0 +1,81 @@
+---
+source: brush-parser/src/tokenizer.rs
+expression: "test_tokenizer(r\"cat <<HERE\nSOMETHING\nHERE\")?"
+---
+TokenizerResult(
+  input: "cat <<HERE\nSOMETHING\nHERE",
+  result: [
+    W("cat", Loc(
+      start: Pos(
+        idx: 0,
+        line: 1,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 3,
+        line: 1,
+        col: 4,
+      ),
+    )),
+    Op("<<", Loc(
+      start: Pos(
+        idx: 4,
+        line: 1,
+        col: 5,
+      ),
+      end: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 6,
+        line: 1,
+        col: 7,
+      ),
+      end: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+    )),
+    W("SOMETHING\n", Loc(
+      start: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+      end: Pos(
+        idx: 25,
+        line: 3,
+        col: 5,
+      ),
+    )),
+    W("HERE", Loc(
+      start: Pos(
+        idx: 25,
+        line: 3,
+        col: 5,
+      ),
+      end: Pos(
+        idx: 25,
+        line: 3,
+        col: 5,
+      ),
+    )),
+    Op("\n", Loc(
+      start: Pos(
+        idx: 10,
+        line: 1,
+        col: 11,
+      ),
+      end: Pos(
+        idx: 11,
+        line: 2,
+        col: 1,
+      ),
+    )),
+  ],
+)
diff --git a/brush-shell/tests/cases/here.yaml b/brush-shell/tests/cases/here.yaml
index d6d1665..ee6a712 100644
--- a/brush-shell/tests/cases/here.yaml
+++ b/brush-shell/tests/cases/here.yaml
@@ -53,6 +53,9 @@ cases:
       wc -l <<EOF
       EOF
 
+  - name: "Basic here doc"
+    stdin: "wc -l <<EOF\nSomething\nEOF"
+
   - name: "Here doc with other tokens after tag"
     stdin: |
       wc -l <<EOF | wc -l