On 2025-02-12 05:58, Lasse Collin wrote:
the removal makes gzip one
of the few compression tools that don't support setting even the
compression level via an environment variable.
Thanks for the review. On further thought I agree that, for backward
compatibility reasons at least, the gzip command should continue to
examine the GZIP environment variable for options like -9 that affect
only performance (including compression performance). These options
shouldn't cause the bigger glitches (and perhaps even security hazards)
that behavior-oriented options can cause. This would be more in line
with zstd, which takes this relatively-cautious approach to environment
variables.
You mentioned --synchronous as being innocuous even though gzip
currently doesn't allow it in GZIP, and that also is a good suggestion.
So I installed the attached patch instead into Savannah master gzip. It
means gzip will pay attention to -1..-9, --rsyncable, and --synchronous
but will silently ignore everything else in GZIP. The idea is that a
garbage GZIP value shouldn't disrupt normal operations.
From 02cde43286da3b47182daac61352fa26ed35e2ea Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 19 Feb 2025 22:33:34 -0800
Subject: [PATCH] Ignore GZIP envvar except for innocuous flags
This is slimmed down from the originally proposed patch, which
ignored GZIP entirely. Thanks to Lasse Collin for thoughtful comments.
* gzip.c (ENV_OPTION): Remove. All uses removed.
(main): Parse GZIP contents with error diagnostics disabled,
and simply ignore invalid contents. Ignore all options
except -1 thru -9, --rsyncable, --synchronous. Remove
unnecessary longind local; all uses removed.
* tests/gzip-env (GZIP): Adjust to match current behavior.
Also test --rsyncable, --synchronous.
---
NEWS | 9 +++++++
doc/gzip.texi | 13 +++++-----
gzip.1 | 17 ++++++-------
gzip.c | 69 ++++++++++++--------------------------------------
tests/gzip-env | 16 +++++++-----
znew.in | 3 ++-
6 files changed, 52 insertions(+), 75 deletions(-)
diff --git a/NEWS b/NEWS
index e348355..9341ef8 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,15 @@ GNU gzip NEWS -*- outline -*-
'gzip -S' now rejects suffixes containing '/'.
[bug present since the beginning]
+** Changes in behavior
+
+ The GZIP environment variable is now silently ignored except for the
+ options -1 (--fast) through -9 (--best), --rsyncable, and --synchronous.
+ This brings gzip into line with more-cautious compressors like zstd
+ that limit environment variables' effect to relatively innocuous
+ performance issues. You can continue to use scripts to specify
+ whatever gzip options you like.
+
** Performance improvements
gzip now decompresses significantly faster by computing CRCs via a
diff --git a/doc/gzip.texi b/doc/gzip.texi
index cc05b60..ac03950 100644
--- a/doc/gzip.texi
+++ b/doc/gzip.texi
@@ -487,15 +487,16 @@ complement to @command{tar}, not as a replacement.
@chapter Environment
@cindex Environment
-The obsolescent environment variable @env{GZIP} can hold a set of
+The environment variable @env{GZIP} can hold a set of
default options for @command{gzip}. These options are interpreted
-first and can be overwritten by explicit command line parameters. As
+first and can be overridden by explicit command line parameters. As
this can cause problems when using scripts, this feature is supported
-only for options that are reasonably likely to not cause too much
-harm, and @command{gzip} warns if it is used. This feature will be
-removed in a future release of @command{gzip}.
+only for @option{--rsyncable}, @option{--synchronous}, and
+options like @option{-9} that set the compression level;
+any other options or operands in @env{GZIP} are silently ignored.
-You can use an alias or script instead. For example, if
+You can use an alias or script instead. For example,
+instead of setting @samp{GZIP="-9"} in the environment, if
@command{gzip} is in the directory @samp{/usr/bin} you can prepend
@file{$HOME/bin} to your @env{PATH} and create an executable script
@file{$HOME/bin/gzip} containing the following:
diff --git a/gzip.1 b/gzip.1
index 0efe399..8ab131b 100644
--- a/gzip.1
+++ b/gzip.1
@@ -416,22 +416,21 @@ such as tar or zip.
GNU tar supports the \-z option to invoke gzip transparently.
gzip is designed as a complement to tar, not as a replacement.
.SH "ENVIRONMENT"
-The obsolescent environment variable
+The environment variable
.B GZIP
can hold a set of default options for
.BR gzip .
-These options are interpreted first and can be overwritten by explicit
+These options are interpreted first and can be overridden by explicit
command line parameters.
As this can cause problems when using scripts,
-this feature is supported only for options that are
-reasonably likely to not cause too much harm, and
-.B gzip
-warns if it is used.
-This feature will be removed in a future release of
-.BR gzip .
+this feature is supported only for \-\-rsyncable, \-\-synchronous,
+and options like \-9 that set the compression level;
+any other options or operands in
+.B GZIP
+are silently ignored.
.PP
You can use an alias or script instead.
-For example, if
+For example, instead of setting GZIP="\-9" in the environment, if
.B gzip
is in the directory
.B /usr/bin
diff --git a/gzip.c b/gzip.c
index a33841a..913fafe 100644
--- a/gzip.c
+++ b/gzip.c
@@ -249,10 +249,6 @@ enum
PRESUME_INPUT_TTY_OPTION = CHAR_MAX + 1,
RSYNCABLE_OPTION,
SYNCHRONOUS_OPTION,
-
- /* A value greater than all valid long options, used as a flag to
- distinguish options derived from the GZIP environment variable. */
- ENV_OPTION
};
static char const shortopts[] = "ab:cdfhH?klLmMnNqrS:tvVZ123456789";
@@ -277,7 +273,7 @@ static const struct option longopts[] =
{"-presume-input-tty", no_argument, NULL, PRESUME_INPUT_TTY_OPTION},
{"quiet", 0, 0, 'q'}, /* quiet mode */
{"silent", 0, 0, 'q'}, /* quiet mode */
- {"synchronous",0, 0, SYNCHRONOUS_OPTION},
+ {"synchronous",0, 0, SYNCHRONOUS_OPTION}, /* output data synchronously */
{"recursive", 0, 0, 'r'}, /* recurse through directories */
{"suffix", 1, 0, 'S'}, /* use given suffix instead of .gz */
{"test", 0, 0, 't'}, /* test compressed file integrity */
@@ -430,6 +426,7 @@ int main (int argc, char **argv)
argv_copy = argv;
env = add_envopt (&env_argc, &argv_copy, OPTIONS_VAR);
env_argv = env ? argv_copy : NULL;
+ opterr = !env;
#ifndef GNU_STANDARD
# define GNU_STANDARD 1
@@ -455,48 +452,35 @@ int main (int argc, char **argv)
while (true) {
int optc;
- int longind = -1;
if (env_argv)
{
- if (env_argv[optind] && strequ (env_argv[optind], "--"))
- optc = ENV_OPTION + '-';
- else
+ /* Get the next option taken from the GZIP environment variable
+ that affects only performance (including compression level).
+ Silently ignore other options, non-options, invalid options,
+ and option syntax errors, so that bad GZIP values do
+ not unduly interfere with normal gzip operation. */
+ do
{
optc = getopt_long (env_argc, env_argv, shortopts, longopts,
- &longind);
- if (0 <= optc)
- optc += ENV_OPTION;
- else
+ NULL);
+ if (optc < 0)
{
- if (optind != env_argc)
- {
- fprintf (stderr,
- ("%s: %s: non-option in "OPTIONS_VAR
- " environment variable\n"),
- program_name, env_argv[optind]);
- try_help ();
- }
-
- /* Wait until here before warning, so that GZIP='-q'
- doesn't warn. */
- if (env_argc != 1 && !quiet)
- fprintf (stderr,
- ("%s: warning: "OPTIONS_VAR" environment variable"
- " is deprecated; use an alias or script\n"),
- program_name);
-
/* Start processing ARGC and ARGV instead. */
free (env_argv);
env_argv = NULL;
+ opterr = 1;
optind = 1;
- longind = -1;
+ break;
}
}
+ while (! (('1' <= optc && optc <= '9')
+ || optc == RSYNCABLE_OPTION
+ || optc == SYNCHRONOUS_OPTION));
}
if (!env_argv)
- optc = getopt_long (argc, argv, shortopts, longopts, &longind);
+ optc = getopt_long (argc, argv, shortopts, longopts, NULL);
if (optc < 0)
break;
@@ -532,15 +516,12 @@ int main (int argc, char **argv)
case 'M': /* undocumented, may change later */
no_time = 0; break;
case 'n':
- case 'n' + ENV_OPTION:
no_name = no_time = 1; break;
case 'N':
- case 'N' + ENV_OPTION:
no_name = no_time = 0; break;
case PRESUME_INPUT_TTY_OPTION:
presume_input_tty = true; break;
case 'q':
- case 'q' + ENV_OPTION:
quiet = 1; verbose = 0; break;
case 'r':
#if NO_DIR
@@ -553,7 +534,6 @@ int main (int argc, char **argv)
break;
case RSYNCABLE_OPTION:
- case RSYNCABLE_OPTION + ENV_OPTION:
rsync = 1;
break;
case 'S':
@@ -575,7 +555,6 @@ int main (int argc, char **argv)
test = decompress = to_stdout = 1;
break;
case 'v':
- case 'v' + ENV_OPTION:
verbose++; quiet = 0; break;
case 'V':
version (); finish_out (); break;
@@ -584,28 +563,12 @@ int main (int argc, char **argv)
program_name);
try_help ();
break;
- case '1' + ENV_OPTION: case '2' + ENV_OPTION: case '3' + ENV_OPTION:
- case '4' + ENV_OPTION: case '5' + ENV_OPTION: case '6' + ENV_OPTION:
- case '7' + ENV_OPTION: case '8' + ENV_OPTION: case '9' + ENV_OPTION:
- optc -= ENV_OPTION;
- FALLTHROUGH;
case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
level = optc - '0';
break;
default:
- if (ENV_OPTION <= optc && optc != ENV_OPTION + '?')
- {
- /* Output a diagnostic, since getopt_long didn't. */
- fprintf (stderr, "%s: ", program_name);
- if (longind < 0)
- fprintf (stderr, "-%c: ", optc - ENV_OPTION);
- else
- fprintf (stderr, "--%s: ", longopts[longind].name);
- fprintf (stderr, ("option not valid in "OPTIONS_VAR
- " environment variable\n"));
- }
try_help ();
}
} /* loop on all arguments */
diff --git a/tests/gzip-env b/tests/gzip-env
index b7382d9..44123c9 100755
--- a/tests/gzip-env
+++ b/tests/gzip-env
@@ -1,5 +1,5 @@
#!/bin/sh
-# Test the obsolescent GZIP environment variable.
+# Test the GZIP environment variable.
# Copyright 2015-2025 Free Software Foundation, Inc.
@@ -27,14 +27,18 @@ GZIP=-qv gzip -d <in >out 2>err || fail=1
compare exp out || fail=1
for badopt in -- -c --stdout -d --decompress -f --force -h --help -k --keep \
- -l --list -L --license -r --recursive -Sxxx --suffix=xxx '--suffix xxx' \
- -t --test -V --version
+ -l --list -L --license -n --no-name -N --name -q --quiet \
+ -r --recursive -Sxxx --suffix=xxx '--suffix xxx' \
+ -t --test -v --verbose -V --version
do
- GZIP=$badopt gzip -d <in >out 2>err && fail=1
+ GZIP=$badopt gzip <exp >inagain 2>err || fail=1
+ compare in inagain
+ GZIP=$badopt gzip -d <in >out 2>err || fail=1
+ compare exp out || fail=1
done
-for goodopt in -n --no-name -N --name -q --quiet -v --verbose \
- -1 --fast -2 -3 -4 -5 -6 -7 -8 -9 --best
+for goodopt in \
+ -1 --fast -2 -3 -4 -5 -6 -7 -8 -9 --best --rsyncable --synchronous
do
GZIP=$goodopt gzip -d <in >out 2>err || fail=1
compare exp out || fail=1
diff --git a/znew.in b/znew.in
index b09d967..ad13631 100644
--- a/znew.in
+++ b/znew.in
@@ -55,7 +55,8 @@ new=0
block=1024
# block is the disk block size (best guess, need not be exact)
-# Beware -s or --suffix in $GZIP.
+# Beware -s or --suffix in $GZIP, which could cause misbehavior
+# in gzip 1.13 and earlier.
unset GZIP
ext=.gz
--
2.45.2
