On Thursday, March 13, 2014 09:35:26 Paul Eggert wrote: > On 03/13/2014 01:21 AM, Pavel Raiskup wrote: > >> We could detect that archive contents are coming from terminal input and > >> fail immediately (IIRC tar waits indefinitely ATM until it's input is cut > >> by ctrl+d or it is killed). > > I meant something like the patch attached, could you please consider? > > > > I tried to write testcase for this and I failed to find proper 'script' > > alternative tool among coreutils, is there some? I could rewrite it then. > > This sort of thing would seem to run counter to the GNU coding > standards, which say that the behavior a program should not depend on > the type of device it is used with. > > http://www.gnu.org/prep/standards/html_node/User-Interfaces.html
Ah, truth. Could I convince you to push some fix which would refuse just *reading* archive from terminal (updated patch attached)? Strictly speaking, GNU Coding Guidelines are talking about program _output_, not about input (IIRC the standard, this is intentional and I very agree with that part). Anyway, the 'tar -x BIG_FILE' usecase is the most serious one I tried to avoid - it may cost users very expensive *seconds* of their lives during trying to recognize what is (not) happening in background. Thanks for considering, Pavel
>From e6e98c91b94f8bde4bd4b9a7ebc0a670695a1e42 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup <prais...@redhat.com> Date: Thu, 13 Mar 2014 08:58:42 +0100 Subject: [PATCH] tar: fail if archive comes directly from terminal References: http://lists.gnu.org/archive/html/bug-tar/2014-03/msg00037.html * src/buffer.c (_open_archive): Call avoid_terminal_io (). (avoid_terminal_io): New function; fails if isatty(archive). * tests/iotty.at: New testcase. * tests/Makefile.am: Handle new testcase. * tests/testsuite.at: Handle new testcase and define new macro AT_SCRIPT_PREREQ. --- src/buffer.c | 18 ++++++++++++++++++ tests/Makefile.am | 1 + tests/iotty.at | 46 ++++++++++++++++++++++++++++++++++++++++++++++ tests/testsuite.at | 7 +++++++ 4 files changed, 72 insertions(+) create mode 100644 tests/iotty.at diff --git a/src/buffer.c b/src/buffer.c index 95e8a26..f2f56cb 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -668,6 +668,22 @@ init_buffer (void) record_end = record_start + blocking_factor; } +static void +avoid_terminal_io (enum access_mode wanted_access) +{ + if (0 != strcmp (archive_name_array[0], "-")) + return; + + /* do not fail if the tar's output goes directly to tty because such + behavior would go against GNU Coding Standards: + http://lists.gnu.org/archive/html/bug-tar/2014-03/msg00042.html */ + if (wanted_access == ACCESS_READ + && isatty (STDIN_FILENO)) + FATAL_ERROR ((0, 0, + _("I won't read archive contents from terminal " + "(missing -f option?)"))); +} + /* Open an archive file. The argument specifies whether we are reading or writing, or both. */ static void @@ -689,6 +705,8 @@ _open_archive (enum access_mode wanted_access) /* When updating the archive, we start with reading. */ access_mode = wanted_access == ACCESS_UPDATE ? ACCESS_READ : wanted_access; + avoid_terminal_io (access_mode); + read_full_records = read_full_records_option; records_read = 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index d4c9362..0ab28c8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -116,6 +116,7 @@ TESTSUITE_AT = \ incr09.at\ indexfile.at\ ignfail.at\ + iotty.at\ label01.at\ label02.at\ label03.at\ diff --git a/tests/iotty.at b/tests/iotty.at new file mode 100644 index 0000000..5aae1d3 --- /dev/null +++ b/tests/iotty.at @@ -0,0 +1,46 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- + +# Test suite for GNU tar. +# Copyright 2014 Free Software Foundation, Inc. + +# This file is part of GNU tar. + +# GNU tar is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. + +# GNU tar is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Ensure that we may not put/read archive contents to/from terminal +# output/input. +# +# Original report: +# http://lists.gnu.org/archive/html/bug-tar/2014-03/msg00033.html + +AT_SETUP([terminal input/output]) +AT_KEYWORDS([options iotty]) + +AT_TAR_CHECK([ +AT_SCRIPT_PREREQ +AT_GZIP_PREREQ +genfile --file file +script -q -c "tar -x" /dev/null >> out +script -q -c "tar -xz" /dev/null >> out +cat out | sed 's|\r||' +], +[0], +[tar: I won't read archive contents from terminal (missing -f option?) +tar: Error is not recoverable: exiting now +tar: I won't read archive contents from terminal (missing -f option?) +tar: Error is not recoverable: exiting now +], +[],[],[],[posix, gnu, oldgnu]) + +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index c52890b..2011e4a 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -124,6 +124,12 @@ m4_define([AT_CHECK_UTIL],[ fi ]) +m4_define([AT_SCRIPT_PREREQ],[ + echo "sth" > expected + script -q -c "echo sth" /dev/null | sed 's|\r||' > output + rm -rf output expected +]) + m4_define([AT_XATTRS_UTILS_PREREQ],[ file=$(TMPDIR=. mktemp fiXXXXXX) AT_CHECK_UTIL(setfattr -n user.test -v test $file,0) @@ -213,6 +219,7 @@ m4_include([gzip.at]) m4_include([recurse.at]) m4_include([recurs02.at]) m4_include([shortrec.at]) +m4_include([iotty.at]) AT_BANNER([The --same-order option]) m4_include([same-order01.at]) -- 1.8.5.3
