From 44f2f37bd45ad5c933d49247530226b41e0c74ca Mon Sep 17 00:00:00 2001 From: Fabian Homborg Date: Fri, 7 Apr 2017 21:48:44 +0200 Subject: [PATCH] Remove "Array index out of bounds" errors This just removes every invalid index. That means with `set foo a b c` and the "show" function from tests/expand.in: - `show $foo[-5..-1]` prints "3 a b c" - `show $foo[-10..1]` prints "1 a" - `show $foo[2..5]` prints "2 b c" - `show $foo[1 3 7 2]` prints "3 a c b" and similar for command substitutions. Fixes #826. --- src/expand.cpp | 46 +++++++++++++-------------------------------- tests/expansion.err | 21 --------------------- tests/expansion.in | 6 ++++++ tests/expansion.out | 13 +++++++++++++ 4 files changed, 32 insertions(+), 54 deletions(-) diff --git a/src/expand.cpp b/src/expand.cpp index 74e0f8008..837d90eba 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -796,29 +796,22 @@ static int expand_variables(const wcstring &instr, std::vector *ou if (!all_vars) { wcstring_list_t string_values(var_idx_list.size()); + size_t k = 0; for (size_t j = 0; j < var_idx_list.size(); j++) { long tmp = var_idx_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, - ARRAY_BOUNDS_ERR); - is_ok = false; - var_idx_list.resize(j); - break; - } else { - // Replace each index in var_idx_list inplace with the string value - // at the specified index. - // al_set( var_idx_list, j, wcsdup((const wchar_t *)al_get( - // &var_item_list, tmp-1 ) ) ); - string_values.at(j) = var_item_list.at(tmp - 1); + // Check that we are within array bounds. If not, skip the element. Note: + // Negative indices (`echo $foo[-1]`) are already converted to positive ones + // here, So tmp < 1 means it's definitely not in. + if ((size_t)tmp > var_item_list.size() || tmp < 1) { + continue; } + // Replace each index in var_idx_list inplace with the string value + // at the specified index. + string_values.at(k++) = var_item_list.at(tmp - 1); } - // string_values is the new var_item_list. + // string_values is the new var_item_list. Resize to remove invalid elements. + string_values.resize(k); var_item_list = std::move(string_values); } } @@ -892,17 +885,6 @@ static int expand_variables(const wcstring &instr, std::vector *ou 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. @@ -1090,10 +1072,8 @@ static int expand_cmdsubst(const wcstring &input, std::vector *out tail_begin = slice_end; for (i = 0; i < slice_idx.size(); i++) { long idx = slice_idx.at(i); - if (idx < 1 || (size_t)idx > sub_res.size()) { - size_t pos = slice_source_positions.at(i); - append_syntax_error(errors, slice_begin - in + pos, ARRAY_BOUNDS_ERR); - return 0; + if ((size_t)idx > sub_res.size() || idx < 1) { + continue; } idx = idx - 1; diff --git a/tests/expansion.err b/tests/expansion.err index 5f2751b1b..468f394d8 100644 --- a/tests/expansion.err +++ b/tests/expansion.err @@ -1,30 +1,9 @@ -fish: Array index out of bounds -show "$foo[2]" - ^ -fish: Array index out of bounds -show $foo[2] - ^ -fish: Array index out of bounds -show "$foo[1 2]" - ^ -fish: Array index out of bounds -show $foo[1 2] - ^ -fish: Array index out of bounds -show "$foo[2 1]" - ^ -fish: Array index out of bounds -show $foo[2 1] - ^ fish: Invalid index value echo "$foo[d]" ^ fish: Invalid index value echo $foo[d] ^ -fish: Array index out of bounds -echo ()[1] - ^ fish: Invalid index value echo ()[d] ^ diff --git a/tests/expansion.in b/tests/expansion.in index 6439871f0..f1df97d57 100644 --- a/tests/expansion.in +++ b/tests/expansion.in @@ -62,6 +62,12 @@ show "$$foo" show $$foo show "prefix$$foo" show prefix$$foo +show $foo[-5..2] +show $foo[-2..-1] +show $foo[-10..-5] +show (printf '%s\n' $foo)[-5..2] +show (printf '%s\n' $foo)[-2..-1] +show (printf '%s\n' $foo)[-10..-5] set -l foo show "$foo[1]" diff --git a/tests/expansion.out b/tests/expansion.out index 9fd68a041..3025cda9d 100644 --- a/tests/expansion.out +++ b/tests/expansion.out @@ -36,8 +36,21 @@ 2 baz quux 1 prefixbaz quux fooest 2 prefixbaz prefixquux +2 bar +2 fooest +0 +2 bar +2 fooest +0 +1 +0 1 0 1 0 +1 +0 +1 +0 + Catch your breath