From cc49042294aceb3ab30396eb5731e4ac0cf601bc Mon Sep 17 00:00:00 2001
From: Kevin Ballard <kevin@sb.org>
Date: Wed, 20 Aug 2014 22:01:24 -0700
Subject: [PATCH] Parse slices even for empty variables

When a variable is parsed as being empty, parse out the slice and
validate the indexes anyway, behaving for slicing purposes as if the
variable had a single empty value.

Besides providing errors when expected, this also fixes the following:

    set -l foo
    echo "$foo[1]"

This used to print "[1]", now it properly prints nothing.
---
 expand.cpp          | 35 ++++++++++++++++++++++++++++++++++-
 tests/expansion.err | 18 ++++++++++++++++++
 tests/expansion.in  | 12 ++++++++++++
 tests/expansion.out |  4 ++++
 4 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/expand.cpp b/expand.cpp
index d75c65723..7388cb18b 100644
--- a/expand.cpp
+++ b/expand.cpp
@@ -1155,10 +1155,10 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
                         for (size_t j=0; j<var_idx_list.size(); j++)
                         {
                             long tmp = var_idx_list.at(j);
-                            size_t var_src_pos = var_pos_list.at(j);
                             /* Check that we are within array bounds. If not, truncate the list to exit. */
                             if (tmp < 1 || (size_t)tmp > var_item_list.size())
                             {
+                                size_t var_src_pos = var_pos_list.at(j);
                                 /* The slice was parsed starting at stop_pos, so we have to add that to the error position */
                                 append_syntax_error(errors,
                                                     slice_start + var_src_pos,
@@ -1255,6 +1255,39 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::
             }
             else
             {
+                // even with no value, we still need to parse out slice syntax
+                // Behave as though we had 1 value, so $foo[1] always works.
+                const size_t slice_start = stop_pos;
+                if (in[slice_start] == L'[')
+                {
+                    wchar_t *slice_end;
+
+                    if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, 1))
+                    {
+                        append_syntax_error(errors,
+                                            stop_pos,
+                                            L"Invalid index value");
+                        is_ok = 0;
+                        return is_ok;
+                    }
+                    stop_pos = (slice_end-in);
+
+                    // validate that the parsed indexes are valid
+                    for (size_t j=0; j<var_idx_list.size(); j++)
+                    {
+                        long tmp = var_idx_list.at(j);
+                        if (tmp != 1)
+                        {
+                            size_t var_src_pos = var_pos_list.at(j);
+                            append_syntax_error(errors,
+                                                slice_start + var_src_pos,
+                                                ARRAY_BOUNDS_ERR);
+                            is_ok = 0;
+                            return is_ok;
+                        }
+                    }
+                }
+
                 /*
                          Expand a non-existing variable
                          */
diff --git a/tests/expansion.err b/tests/expansion.err
index e69de29bb..6346f23fb 100644
--- a/tests/expansion.err
+++ b/tests/expansion.err
@@ -0,0 +1,18 @@
+Array index out of bounds
+fish: show "$foo[2]"
+                 ^
+Array index out of bounds
+fish: show $foo[2]
+                ^
+Array index out of bounds
+fish: show "$foo[1 2]"
+                   ^
+Array index out of bounds
+fish: show $foo[1 2]
+                  ^
+Array index out of bounds
+fish: show "$foo[2 1]"
+                 ^
+Array index out of bounds
+fish: show $foo[2 1]
+                ^
diff --git a/tests/expansion.in b/tests/expansion.in
index d362e7895..8b48772ef 100644
--- a/tests/expansion.in
+++ b/tests/expansion.in
@@ -62,3 +62,15 @@ show "$$foo"
 show $$foo
 show "prefix$$foo"
 show prefix$$foo
+
+set -l foo
+show "$foo[1]"
+show $foo[1]
+show "$foo[-1]"
+show $foo[-1]
+show "$foo[2]"
+show $foo[2]
+show "$foo[1 2]"
+show $foo[1 2]
+show "$foo[2 1]"
+show $foo[2 1]
diff --git a/tests/expansion.out b/tests/expansion.out
index 3e7631804..fe594539c 100644
--- a/tests/expansion.out
+++ b/tests/expansion.out
@@ -36,3 +36,7 @@
 2 baz quux
 1 prefixbaz quux  fooest
 2 prefixbaz prefixquux
+1 
+0
+1 
+0