From 4197420f3968093ec12075f1eac06ed1570a8e30 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Wed, 26 Jul 2017 20:17:04 -0700 Subject: [PATCH] implement limits on command substitution output This makes command substitutions impose the same limit on the amount of data they accept as the `read` builtin. It does not limit output of external commands or builtins in other contexts. Fixes #3822 --- CHANGELOG.md | 3 +- src/exec.cpp | 41 +++++++------ src/exec.h | 5 +- src/expand.cpp | 101 ++++++++++++++++++--------------- src/fish_tests.cpp | 6 +- src/io.cpp | 5 +- src/io.h | 99 +++++++++++++++++++++++++++----- src/parse_execution.cpp | 2 +- src/parser.cpp | 1 - tests/test8.in | 4 +- tests/test8.out | 2 +- tests/test_cmdsub.err | 22 +++++++ tests/test_cmdsub.in | 35 ++++++++++++ tests/test_cmdsub.out | 10 ++++ tests/test_functions/show.fish | 16 +++--- 15 files changed, 257 insertions(+), 95 deletions(-) create mode 100644 tests/test_cmdsub.err create mode 100644 tests/test_cmdsub.in create mode 100644 tests/test_cmdsub.out diff --git a/CHANGELOG.md b/CHANGELOG.md index 42c4d1be8..0688b7ec4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ This section is for changes merged to the `major` branch that are not also merged to 2.7. ## Deprecations -- The `IFS` variable (#4156). +- The `IFS` variable is deprecated and will be removed in fish 4.0 (#4156). ## Notable fixes and improvements - Local exported (`set -lx`) vars are now visible to functions (#1091). @@ -10,6 +10,7 @@ This section is for changes merged to the `major` branch that are not also merge - `complete` now has a `-k`/`--keep-order` flag to keep the order of the OPTION_ARGUMENTS (#361). - A "--delimiter" option to `read` as a better alternative to the `IFS` variable (#4256). - `set` has a new `--show` option to show lots of information about variables (#4265). +- Command substitution output is now limited to 10 MB by default (#3822). ## Other significant changes - `read` now requires at least one var name (#4220). diff --git a/src/exec.cpp b/src/exec.cpp index 462c2448e..44f41255c 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -382,13 +382,15 @@ void exec_job(parser_t &parser, job_t *j) { // Verify that all IO_BUFFERs are output. We used to support a (single, hacked-in) magical input // IO_BUFFER used by fish_pager, but now the claim is that there are no more clients and it is // removed. This assertion double-checks that. + size_t stdout_read_limit = 0; const io_chain_t all_ios = j->all_io_redirections(); for (size_t idx = 0; idx < all_ios.size(); idx++) { const shared_ptr &io = all_ios.at(idx); if ((io->io_mode == IO_BUFFER)) { io_buffer_t *io_buffer = static_cast(io.get()); - assert(!io_buffer->is_input); //!OCLINT(multiple unary operator) + assert(!io_buffer->is_input); + stdout_read_limit = io_buffer->get_buffer_limit(); } } @@ -605,7 +607,7 @@ void exec_job(parser_t &parser, job_t *j) { shared_ptr block_output_io_buffer; // This is the io_streams we pass to internal builtins. - std::unique_ptr builtin_io_streams; + std::unique_ptr builtin_io_streams(new io_streams_t(stdout_read_limit)); switch (p->type) { case INTERNAL_FUNCTION: { @@ -754,7 +756,6 @@ void exec_job(parser_t &parser, job_t *j) { stdin_is_directly_redirected = stdin_io && stdin_io->io_mode != IO_CLOSE; } - builtin_io_streams.reset(new io_streams_t()); builtin_io_streams->stdin_fd = local_builtin_stdin; builtin_io_streams->out_is_redirected = has_fd(process_net_io_chain, STDOUT_FILENO); @@ -882,8 +883,9 @@ void exec_job(parser_t &parser, job_t *j) { const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == IO_BUFFER; const bool no_stdout_output = stdout_buffer.empty(); const bool no_stderr_output = stderr_buffer.empty(); + const bool stdout_discarded = builtin_io_streams->out.output_discarded(); - if (no_stdout_output && no_stderr_output) { + if (!stdout_discarded && no_stdout_output && no_stderr_output) { // The builtin produced no output and is not inside of a pipeline. No // need to fork or even output anything. debug(3, L"Skipping fork: no output for internal builtin '%ls'", @@ -897,13 +899,15 @@ void exec_job(parser_t &parser, job_t *j) { p->argv0()); io_buffer_t *io_buffer = static_cast(stdout_io.get()); - const std::string res = wcs2string(builtin_io_streams->out.buffer()); - - io_buffer->out_buffer_append(res.data(), res.size()); + if (stdout_discarded) { + io_buffer->set_discard(); + } else { + const std::string res = wcs2string(builtin_io_streams->out.buffer()); + io_buffer->out_buffer_append(res.data(), res.size()); + } fork_was_skipped = true; } else if (stdout_io.get() == NULL && stderr_io.get() == NULL) { - // We are writing to normal stdout and stderr. Just do it - no need to - // fork. + // We are writing to normal stdout and stderr. Just do it - no need to fork. debug(3, L"Skipping fork: ordinary output for internal builtin '%ls'", p->argv0()); const std::string outbuff = wcs2string(stdout_buffer); @@ -915,6 +919,7 @@ void exec_job(parser_t &parser, job_t *j) { debug(0, "!builtin_io_done and errno != EPIPE"); show_stackframe(L'E'); } + if (stdout_discarded) p->status = STATUS_READ_TOO_MUCH; fork_was_skipped = true; } } @@ -1099,8 +1104,8 @@ void exec_job(parser_t &parser, job_t *j) { } } -static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, - bool apply_exit_status) { +static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, bool apply_exit_status, + bool is_subcmd) { ASSERT_IS_MAIN_THREAD(); bool prev_subshell = is_subshell; const int prev_status = proc_get_last_status(); @@ -1116,7 +1121,8 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, // IO buffer creation may fail (e.g. if we have too many open files to make a pipe), so this may // be null. - const shared_ptr io_buffer(io_buffer_t::create(STDOUT_FILENO, io_chain_t())); + const shared_ptr io_buffer( + io_buffer_t::create(STDOUT_FILENO, io_chain_t(), is_subcmd ? read_byte_limit : 0)); if (io_buffer.get() != NULL) { parser_t &parser = parser_t::principal_parser(); if (parser.eval(cmd, io_chain_t(io_buffer), SUBST) == 0) { @@ -1126,6 +1132,8 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, io_buffer->read(); } + if (io_buffer->output_discarded()) subcommand_status = STATUS_READ_TOO_MUCH; + // If the caller asked us to preserve the exit status, restore the old status. Otherwise set the // status of the subcommand. proc_set_last_status(apply_exit_status ? subcommand_status : prev_status); @@ -1166,12 +1174,13 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, return subcommand_status; } -int exec_subshell(const wcstring &cmd, std::vector &outputs, bool apply_exit_status) { +int exec_subshell(const wcstring &cmd, std::vector &outputs, bool apply_exit_status, + bool is_subcmd) { ASSERT_IS_MAIN_THREAD(); - return exec_subshell_internal(cmd, &outputs, apply_exit_status); + return exec_subshell_internal(cmd, &outputs, apply_exit_status, is_subcmd); } -int exec_subshell(const wcstring &cmd, bool apply_exit_status) { +int exec_subshell(const wcstring &cmd, bool apply_exit_status, bool is_subcmd) { ASSERT_IS_MAIN_THREAD(); - return exec_subshell_internal(cmd, NULL, apply_exit_status); + return exec_subshell_internal(cmd, NULL, apply_exit_status, is_subcmd); } diff --git a/src/exec.h b/src/exec.h index 7ce80fb6f..27a5110c8 100644 --- a/src/exec.h +++ b/src/exec.h @@ -35,8 +35,9 @@ void exec_job(parser_t &parser, job_t *j); /// \param outputs The list to insert output into. /// /// \return the status of the last job to exit, or -1 if en error was encountered. -int exec_subshell(const wcstring &cmd, std::vector &outputs, bool preserve_exit_status); -int exec_subshell(const wcstring &cmd, bool preserve_exit_status); +int exec_subshell(const wcstring &cmd, std::vector &outputs, bool preserve_exit_status, + bool is_subcmd = false); +int exec_subshell(const wcstring &cmd, bool preserve_exit_status, bool is_subcmd = false); /// Loops over close until the syscall was run without being interrupted. void exec_close(int fd); diff --git a/src/expand.cpp b/src/expand.cpp index 347f0c3c3..79f4cec7d 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -102,37 +102,43 @@ static bool expand_is_clean(const wcstring &in) { /// Append a syntax error to the given error list. static void append_syntax_error(parse_error_list_t *errors, size_t source_start, const wchar_t *fmt, ...) { - if (errors != NULL) { - parse_error_t error; - error.source_start = source_start; - error.source_length = 0; - error.code = parse_error_syntax; + if (!errors) return; - va_list va; - va_start(va, fmt); - error.text = vformat_string(fmt, va); - va_end(va); + parse_error_t error; + error.source_start = source_start; + error.source_length = 0; + error.code = parse_error_syntax; - errors->push_back(error); - } + va_list va; + va_start(va, fmt); + error.text = vformat_string(fmt, va); + va_end(va); + + errors->push_back(error); } -/// Append a cmdsub error to the given error list. +/// Append a cmdsub error to the given error list. But only do so if the error hasn't already been +/// recorded. This is needed because command substitution is a recursive process and some errors +/// could consequently be recorded more than once. static void append_cmdsub_error(parse_error_list_t *errors, size_t source_start, const wchar_t *fmt, ...) { - if (errors != NULL) { - parse_error_t error; - error.source_start = source_start; - error.source_length = 0; - error.code = parse_error_cmdsubst; + if (!errors) return; - va_list va; - va_start(va, fmt); - error.text = vformat_string(fmt, va); - va_end(va); + parse_error_t error; + error.source_start = source_start; + error.source_length = 0; + error.code = parse_error_cmdsubst; - errors->push_back(error); + va_list va; + va_start(va, fmt); + error.text = vformat_string(fmt, va); + va_end(va); + + for (auto it : *errors) { + if (error.text == it.text) return; } + + errors->push_back(error); } /// Return the environment variable value for the string starting at \c in. @@ -1024,24 +1030,23 @@ static expand_error_t expand_brackets(const wcstring &instr, expand_flags_t flag } /// Perform cmdsubst expansion. -static int expand_cmdsubst(const wcstring &input, std::vector *out_list, - parse_error_list_t *errors) { - wchar_t *paran_begin = 0, *paran_end = 0; - std::vector sub_res; +static bool expand_cmdsubst(const wcstring &input, std::vector *out_list, + parse_error_list_t *errors) { + wchar_t *paren_begin = nullptr, *paren_end = nullptr; + wchar_t *tail_begin = nullptr; size_t i, j; - wchar_t *tail_begin = 0; const wchar_t *const in = input.c_str(); int parse_ret; - switch (parse_ret = parse_util_locate_cmdsubst(in, ¶n_begin, ¶n_end, false)) { + switch (parse_ret = parse_util_locate_cmdsubst(in, &paren_begin, &paren_end, false)) { case -1: { append_syntax_error(errors, SOURCE_LOCATION_UNKNOWN, L"Mismatched parenthesis"); - return 0; + return false; } case 0: { append_completion(out_list, input); - return 1; + return true; } case 1: { break; @@ -1052,15 +1057,22 @@ static int expand_cmdsubst(const wcstring &input, std::vector *out } } - const wcstring subcmd(paran_begin + 1, paran_end - paran_begin - 1); - - if (exec_subshell(subcmd, sub_res, true /* do apply exit status */) == -1) { + wcstring_list_t sub_res; + const wcstring subcmd(paren_begin + 1, paren_end - paren_begin - 1); + if (exec_subshell(subcmd, sub_res, true /* apply_exit_status */, true /* is_subcmd */) == -1) { append_cmdsub_error(errors, SOURCE_LOCATION_UNKNOWN, L"Unknown error while evaulating command substitution"); - return 0; + return false; } - tail_begin = paran_end + 1; + if (proc_get_last_status() == STATUS_READ_TOO_MUCH) { + append_cmdsub_error( + errors, in - paren_begin, + _(L"Too much data emitted by command substitution so it was discarded\n")); + return false; + } + + tail_begin = paren_end + 1; if (*tail_begin == L'[') { std::vector slice_idx; std::vector slice_source_positions; @@ -1072,7 +1084,7 @@ static int expand_cmdsubst(const wcstring &input, std::vector *out parse_slice(slice_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size()); if (bad_pos != 0) { append_syntax_error(errors, slice_begin - in + bad_pos, L"Invalid index value"); - return 0; + return false; } wcstring_list_t sub_res2; @@ -1110,7 +1122,7 @@ static int expand_cmdsubst(const wcstring &input, std::vector *out const wcstring &tail_item = tail_expand.at(j).completion; // sb_append_substring( &whole_item, in, len1 ); - whole_item.append(in, paran_begin - in); + whole_item.append(in, paren_begin - in); // sb_append_char( &whole_item, INTERNAL_SEPARATOR ); whole_item.push_back(INTERNAL_SEPARATOR); @@ -1129,7 +1141,8 @@ static int expand_cmdsubst(const wcstring &input, std::vector *out } } - return 1; + if (proc_get_last_status() == STATUS_READ_TOO_MUCH) return false; + return true; } // Given that input[0] is HOME_DIRECTORY or tilde (ugh), return the user's name. Return the empty @@ -1304,7 +1317,6 @@ typedef expand_error_t (*expand_stage_t)(const wcstring &input, //!OCL static expand_error_t expand_stage_cmdsubst(const wcstring &input, std::vector *out, expand_flags_t flags, parse_error_list_t *errors) { - expand_error_t result = EXPAND_OK; if (EXPAND_SKIP_CMDSUBST & flags) { wchar_t *begin, *end; if (parse_util_locate_cmdsubst(input.c_str(), &begin, &end, true) == 0) { @@ -1312,15 +1324,14 @@ static expand_error_t expand_stage_cmdsubst(const wcstring &input, std::vector *out, diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 0702efb9c..09e57e091 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2013,7 +2013,7 @@ static bool run_one_test_test(int expected, wcstring_list_t &lst, bool bracket) i++; } argv[i + 1] = NULL; - io_streams_t streams; + io_streams_t streams(0); int result = builtin_test(parser, streams, argv); delete[] argv; return expected == result; @@ -2040,7 +2040,7 @@ static bool run_test_test(int expected, const wcstring &str) { static void test_test_brackets() { // Ensure [ knows it needs a ]. parser_t parser; - io_streams_t streams; + io_streams_t streams(0); const wchar_t *argv1[] = {L"[", L"foo", NULL}; do_test(builtin_test(parser, streams, (wchar_t **)argv1) != 0); @@ -3930,7 +3930,7 @@ int builtin_string(parser_t &parser, io_streams_t &streams, wchar_t **argv); static void run_one_string_test(const wchar_t **argv, int expected_rc, const wchar_t *expected_out) { parser_t parser; - io_streams_t streams; + io_streams_t streams(0); streams.stdin_is_directly_redirected = false; // read from argv instead of stdin int rc = builtin_string(parser, streams, const_cast(argv)); wcstring args; diff --git a/src/io.cpp b/src/io.cpp index f5b315430..b36a8d500 100644 --- a/src/io.cpp +++ b/src/io.cpp @@ -75,10 +75,11 @@ bool io_buffer_t::avoid_conflicts_with_io_chain(const io_chain_t &ios) { return result; } -shared_ptr io_buffer_t::create(int fd, const io_chain_t &conflicts) { +shared_ptr io_buffer_t::create(int fd, const io_chain_t &conflicts, + size_t buffer_limit) { bool success = true; assert(fd >= 0); - shared_ptr buffer_redirect(new io_buffer_t(fd)); + shared_ptr buffer_redirect(new io_buffer_t(fd, buffer_limit)); if (exec_pipe(buffer_redirect->pipe_fd) == -1) { debug(1, PIPE_ERROR); diff --git a/src/io.h b/src/io.h index a2340362e..4a24edee7 100644 --- a/src/io.h +++ b/src/io.h @@ -20,6 +20,7 @@ using std::tr1::shared_ptr; #endif #include "common.h" +#include "env.h" /// Describes what type of IO operation an io_data_t represents. enum io_mode_t { IO_FILE, IO_PIPE, IO_FD, IO_BUFFER, IO_CLOSE }; @@ -99,10 +100,18 @@ class io_pipe_t : public io_data_t { class io_chain_t; class io_buffer_t : public io_pipe_t { private: + /// True if we're discarding input. + bool discard; + /// Limit on how much data we'll buffer. Zero means no limit. + size_t buffer_limit; /// Buffer to save output in. std::vector out_buffer; - explicit io_buffer_t(int f) : io_pipe_t(IO_BUFFER, f, false /* not input */), out_buffer() {} + explicit io_buffer_t(int f, size_t limit) + : io_pipe_t(IO_BUFFER, f, false /* not input */), + discard(false), + buffer_limit(limit), + out_buffer() {} public: virtual void print() const; @@ -111,6 +120,12 @@ class io_buffer_t : public io_pipe_t { /// Function to append to the buffer. void out_buffer_append(const char *ptr, size_t count) { + if (discard) return; + if (buffer_limit && out_buffer.size() + count > buffer_limit) { + discard = true; + out_buffer.clear(); + return; + } out_buffer.insert(out_buffer.end(), ptr, ptr + count); } @@ -122,6 +137,19 @@ class io_buffer_t : public io_pipe_t { /// Function to get the size of the buffer. size_t out_buffer_size(void) const { return out_buffer.size(); } + /// Function that returns true if we discarded the input because there was too much data. + bool output_discarded(void) { return discard; } + + /// Function to explicitly put the object in discard mode. Meant to be used when moving + /// the results from an output_stream_t to an io_buffer_t. + void set_discard(void) { + discard = true; + out_buffer.clear(); + } + + /// This is used to transfer the buffer limit for this object to a output_stream_t object. + size_t get_buffer_limit(void) { return buffer_limit; } + /// Ensures that the pipes do not conflict with any fd redirections in the chain. bool avoid_conflicts_with_io_chain(const io_chain_t &ios); @@ -134,7 +162,8 @@ class io_buffer_t : public io_pipe_t { /// \param fd the fd that will be mapped in the child process, typically STDOUT_FILENO /// \param conflicts A set of IO redirections. The function ensures that any pipe it makes does /// not conflict with an fd redirection in this list. - static shared_ptr create(int fd, const io_chain_t &conflicts); + static shared_ptr create(int fd, const io_chain_t &conflicts, + size_t buffer_limit = 0); }; class io_chain_t : public std::vector > { @@ -164,37 +193,79 @@ bool pipe_avoid_conflicts_with_io_chain(int fds[2], const io_chain_t &ios); /// Class representing the output that a builtin can generate. class output_stream_t { private: + /// Limit on how much data we'll buffer. Zero means no limit. + size_t buffer_limit; + /// True if we're discarding input. + bool discard; // No copying. output_stream_t(const output_stream_t &s); void operator=(const output_stream_t &s); wcstring buffer_; + void check_for_overflow() { + if (buffer_limit && buffer_.size() > buffer_limit) { + discard = true; + buffer_.clear(); + } + } + public: - output_stream_t() {} + output_stream_t(size_t buffer_limit_) : buffer_limit(buffer_limit_), discard(false) {} - void append(const wcstring &s) { this->buffer_.append(s); } +#if 0 + void set_buffer_limit(size_t buffer_limit_) { buffer_limit = buffer_limit_; } +#endif - void append(const wchar_t *s) { this->buffer_.append(s); } + void append(const wcstring &s) { + if (discard) return; + buffer_.append(s); + check_for_overflow(); + } - void append(wchar_t s) { this->buffer_.push_back(s); } + void append(const wchar_t *s) { + if (discard) return; + buffer_.append(s); + check_for_overflow(); + } - void append(const wchar_t *s, size_t amt) { this->buffer_.append(s, amt); } + void append(wchar_t s) { + if (discard) return; + buffer_.push_back(s); + check_for_overflow(); + } - void push_back(wchar_t c) { this->buffer_.push_back(c); } + void append(const wchar_t *s, size_t amt) { + if (discard) return; + buffer_.append(s, amt); + check_for_overflow(); + } + + void push_back(wchar_t c) { + if (discard) return; + buffer_.push_back(c); + check_for_overflow(); + } void append_format(const wchar_t *format, ...) { + if (discard) return; va_list va; va_start(va, format); - ::append_formatv(this->buffer_, format, va); + ::append_formatv(buffer_, format, va); va_end(va); + check_for_overflow(); } void append_formatv(const wchar_t *format, va_list va_orig) { - ::append_formatv(this->buffer_, format, va_orig); + if (discard) return; + ::append_formatv(buffer_, format, va_orig); + check_for_overflow(); } - const wcstring &buffer() const { return this->buffer_; } + const wcstring &buffer() const { return buffer_; } + + /// Function that returns true if we discarded the input because there was too much data. + bool output_discarded(void) { return discard; } bool empty() const { return buffer_.empty(); } }; @@ -218,8 +289,10 @@ struct io_streams_t { // Actual IO redirections. This is only used by the source builtin. Unowned. const io_chain_t *io_chain; - io_streams_t() - : stdin_fd(-1), + io_streams_t(size_t read_limit) + : out(read_limit), + err(read_limit), + stdin_fd(-1), stdin_is_directly_redirected(false), out_is_redirected(false), err_is_redirected(false), diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 958a95ebc..f21327569 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -388,7 +388,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement( const wcstring contents_str = wcstring(this->src, contents_start, contents_end - contents_start); int definition_line_offset = this->line_offset_of_character_at_offset(contents_start); - io_streams_t streams; + io_streams_t streams(0); // no limit on the amount of output from builtin_function() int err = builtin_function(*parser, streams, argument_list, contents_str, definition_line_offset); proc_set_last_status(err); diff --git a/src/parser.cpp b/src/parser.cpp index 44edae94c..422bff2f6 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -724,7 +724,6 @@ int parser_t::eval_block_node(node_offset_t node_idx, const io_chain_t &io, this->pop_block(scope_block); job_reap(0); // reap again - return result; } diff --git a/tests/test8.in b/tests/test8.in index 5e918fbe8..97cc7d57d 100644 --- a/tests/test8.in +++ b/tests/test8.in @@ -1,4 +1,4 @@ -# Test index ranges +# Test index ranges echo Test variable expand set n 10 @@ -16,7 +16,7 @@ set test1[1..$n] $test; echo $test1 set test1[$n..1] $test; echo $test1 set test1[2..4 -2..-4] $test1[4..2 -4..-2]; echo $test1 -echo Test command substitution +echo Test using slices of command substitution echo (seq 5)[-1..1] echo (seq $n)[3..5 -2..2] diff --git a/tests/test8.out b/tests/test8.out index 1e9099f8e..f21615f25 100644 --- a/tests/test8.out +++ b/tests/test8.out @@ -9,7 +9,7 @@ Test variable set 1 2 3 4 5 6 7 8 9 10 10 9 8 7 6 5 4 3 2 1 10 7 8 9 6 5 2 3 4 1 -Test command substitution +Test using slices of command substitution 5 4 3 2 1 3 4 5 9 8 7 6 5 4 3 2 Test more diff --git a/tests/test_cmdsub.err b/tests/test_cmdsub.err new file mode 100644 index 000000000..c77650283 --- /dev/null +++ b/tests/test_cmdsub.err @@ -0,0 +1,22 @@ +# Command sub at the limit should fail. +fish: Too much data emitted by command substitution so it was discarded + +set b (string repeat -n 512 x) + ^ +# Command sub over the limit should fail. +fish: Too much data emitted by command substitution so it was discarded + +set -l x (string repeat -n $argv x) + ^ +in function 'subme' + called on standard input + with parameter list '513' + +in command substitution + called on standard input + +# Same builtin in a command substitution is affected +fish: Too much data emitted by command substitution so it was discarded + +echo this will fail (string repeat --max 513 b) to output anything + ^ diff --git a/tests/test_cmdsub.in b/tests/test_cmdsub.in new file mode 100644 index 000000000..7ec83e305 --- /dev/null +++ b/tests/test_cmdsub.in @@ -0,0 +1,35 @@ +# This tests various corner cases involving command substitution. Most +# importantly the limits on the amount of data we'll substitute. + +set FISH_READ_BYTE_LIMIT 512 + +function subme + set -l x (string repeat -n $argv x) + echo $x +end + +echo '# Command sub just under the limit should succeed.' +set a (subme 511) +show a + +echo '# Command sub at the limit should fail.' +echo '# Command sub at the limit should fail.' >&2 +set b (string repeat -n 512 x) +set saved_status $status +test $saved_status -eq 122 +or echo expected status 122, saw $saved_status >&2 +show b + +echo '# Command sub over the limit should fail.' +echo '# Command sub over the limit should fail.' >&2 +set c (subme 513) +show c + +echo '# Make sure output from builtins outside of command substitution is not affected' +string repeat --max 513 a + +echo '# Same builtin in a command substitution is affected' >&2 +echo this will fail (string repeat --max 513 b) to output anything +set saved_status $status +test $saved_status -eq 122 +or echo expected status 122, saw $saved_status >&2 diff --git a/tests/test_cmdsub.out b/tests/test_cmdsub.out new file mode 100644 index 000000000..442f922f0 --- /dev/null +++ b/tests/test_cmdsub.out @@ -0,0 +1,10 @@ +# Command sub just under the limit should succeed. +$a count=1 +$a[1]=|xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| +# Command sub at the limit should fail. +$b is not set +# Command sub over the limit should fail. +$c count=1 +$c[1]=|| +# Make sure output from builtins outside of command substitution is not affected +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/tests/test_functions/show.fish b/tests/test_functions/show.fish index bbf6c04cd..6cf6f4d54 100644 --- a/tests/test_functions/show.fish +++ b/tests/test_functions/show.fish @@ -1,18 +1,18 @@ # Show information about the named var(s) passed to us. function show --no-scope-shadowing - for v in $argv - if set -q $v - set -l c (count $$v) - printf '$%s count=%d\n' $v $c + for _v in $argv + if set -q $_v + set -l _c (count $$_v) + printf '$%s count=%d\n' $_v $_c # This test is to work around a bogosity of the BSD `seq` command. If called with # `seq 0` it emits `1`, `0`. Whereas that GNU version outputs nothing. - if test $c -gt 0 - for i in (seq $c) - printf '$%s[%d]=|%s|\n' $v $i $$v[1][$i] + if test $_c -gt 0 + for i in (seq $_c) + printf '$%s[%d]=|%s|\n' $_v $i $$_v[1][$i] end end else - echo \$$v is not set + echo \$$_v is not set end end end