From 1d5e715008a45f06f62c9161de6246d47361cf30 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Mon, 22 Oct 2018 20:50:31 +0200
Subject: [PATCH] source: Return error instead of implicitly reading from tty

For things like

    source $undefined

or
    source (nooutput)

it was quite annoying that it read from tty.

Instead we now require a "-" as the filename to read from the tty.

This does not apply to reading from stdin if it's redirected, so

    something | source

still works.

Fixes #2633.
---
 CHANGELOG.md           | 1 +
 doc_src/source.txt     | 3 ++-
 src/builtin_source.cpp | 4 ++++
 tests/test1.in         | 3 +++
 tests/test1.out        | 1 +
 5 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2b0e7c00..218e17a0c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ fish 3.0 is a major release which brings with it both improvements in functional
 - Background jobs not first `disown`'d will be reaped upon `exec`, bringing the behavior in line with that of `exit`.
 - `read` now uses `-s` as short for `--silent` (à la `bash`); `--shell`'s abbreviation (formerly `-s`) is now `-S` instead (#4490).
 - `cd` no longer resolves symlinks. fish now maintains a virtual path, matching other shells. (#3350).
+- `source` now requires an explicit `-` as the filename to read from the terminal (#2633).
 
 ## Notable fixes and improvements
 ### Syntax/semantic changes and new builtins
diff --git a/doc_src/source.txt b/doc_src/source.txt
index abb99af5f..7b32654ee 100644
--- a/doc_src/source.txt
+++ b/doc_src/source.txt
@@ -3,13 +3,14 @@
 \subsection source-synopsis Synopsis
 \fish{synopsis}
 source FILENAME [ARGUMENTS...]
+somecommand | source
 \endfish
 
 \subsection source-description Description
 
 `source` evaluates the commands of the specified file in the current shell. This is different from starting a new process to perform the commands (i.e. `fish < FILENAME`) since the commands will be evaluated by the current shell, which means that changes in shell variables will affect the current shell. If additional arguments are specified after the file name, they will be inserted into the `$argv` variable. The `$argv` variable will not include the name of the sourced file.
 
-If no file is specified, or if the file name '`-`' is used, stdin will be read.
+If no file is specified and stdin is not the terminal, or if the file name '`-`' is used, stdin will be read.
 
 The return status of `source` is the return status of the last job to execute. If something goes wrong while opening or reading the file, `source` exits with a non-zero status.
 
diff --git a/src/builtin_source.cpp b/src/builtin_source.cpp
index e33a4a188..655111e92 100644
--- a/src/builtin_source.cpp
+++ b/src/builtin_source.cpp
@@ -41,6 +41,10 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (argc == optind || wcscmp(argv[optind], L"-") == 0) {
         // Either a bare `source` which means to implicitly read from stdin or an explicit `-`.
+        if (argc == optind && !streams.stdin_is_directly_redirected) {
+            // Don't implicitly read from the terminal.
+            return STATUS_CMD_ERROR;
+        }
         fn = L"-";
         fn_intern = fn;
         fd = dup(streams.stdin_fd);
diff --git a/tests/test1.in b/tests/test1.in
index d2910b6f4..e747f3143 100644
--- a/tests/test1.in
+++ b/tests/test1.in
@@ -213,6 +213,9 @@ echo 'echo "source argv {$argv}"' | source
 echo 'echo "source argv {$argv}"' | source -
 echo 'echo "source argv {$argv}"' | source - abc
 echo 'echo "source argv {$argv}"' | source - abc def
+# This hangs if it fails!
+source
+echo $status
 
 always_fails
 echo $status
diff --git a/tests/test1.out b/tests/test1.out
index 51ab3d525..56fd4f64d 100644
--- a/tests/test1.out
+++ b/tests/test1.out
@@ -140,3 +140,4 @@ source argv {}
 source argv {abc}
 source argv {abc def}
 1
+1