From 1dd776ec990a5317326dea9b408f0975db596c23 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Sat, 9 Jan 2021 08:43:07 +0100
Subject: [PATCH] echo: Don't interpret and print options

A weird interaction between grouped short options and our weird option
parsing that puts unknown options back:

```
echo "-n foo"
```

would see the `-n`, turn off printing newlines, interpret the " " as
another grouped short option, see that there is no short option for
space and put the entire token back on the arguments pile.

So it would print "-n foo" *without a newline*.

Fix this by keeping an old state of the options around and reverting
it when putting options back.

The alternative is *probably* to forbid the " " short option in
wgetopt, then check if an option group contains it and error out, but
this should only really be a problem in `echo` because that is,
AFAICT, the only thing that puts the options back.

Fixes #7614
---
 src/builtin_echo.cpp    | 14 ++++++++++++++
 tests/checks/basic.fish |  9 +++++++++
 2 files changed, 23 insertions(+)

diff --git a/src/builtin_echo.cpp b/src/builtin_echo.cpp
index 9d2c78e71..55051f689 100644
--- a/src/builtin_echo.cpp
+++ b/src/builtin_echo.cpp
@@ -28,6 +28,8 @@ static int parse_cmd_opts(echo_cmd_opts_t &opts, int *optind, int argc, wchar_t
     wchar_t *cmd = argv[0];
     int opt;
     wgetopter_t w;
+    echo_cmd_opts_t oldopts = opts;
+    int oldoptind = 0;
     while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) {
         switch (opt) {
             case 'n': {
@@ -51,6 +53,7 @@ static int parse_cmd_opts(echo_cmd_opts_t &opts, int *optind, int argc, wchar_t
                 return STATUS_INVALID_ARGS;
             }
             case '?': {
+                opts = oldopts;
                 *optind = w.woptind - 1;
                 return STATUS_CMD_OK;
             }
@@ -58,6 +61,17 @@ static int parse_cmd_opts(echo_cmd_opts_t &opts, int *optind, int argc, wchar_t
                 DIE("unexpected retval from wgetopt_long");
             }
         }
+
+        // Super cheesy: We keep an old copy of the option state around,
+        // so we can revert it in case we get an argument like
+        // "-n foo".
+        // We need to keep it one out-of-date so we can ignore the *last* option.
+        // (this might be an issue in wgetopt, but that's a whole other can of worms
+        //  and really only occurs with our weird "put it back" option parsing)
+        if (w.woptind == oldoptind + 2) {
+            oldopts = opts;
+            oldoptind = w.woptind;
+        }
     }
 
     *optind = w.woptind;
diff --git a/tests/checks/basic.fish b/tests/checks/basic.fish
index 2c231040b..c16ab5d7e 100644
--- a/tests/checks/basic.fish
+++ b/tests/checks/basic.fish
@@ -475,3 +475,12 @@ echo $status
 builtin --query echo
 echo $status
 #CHECK: 0
+
+# Check that echo doesn't interpret options *and print them*
+# at the start of quoted args:
+echo '-ne \tart'
+# CHECK: -ne \tart
+echo '-n art'
+echo banana
+# CHECK: -n art
+# CHECK: banana