Re: Rare issue in ignoring padding

[Jim Meyering]

Daniel Barkalow wrote:
> The code to ignore trailing zeros from:
>
>         * gzip.c (get_method): Don't complain about trailing zeros at
>         the end of a gzipped file, as they're commonly appended to fill
>         out a block (e.g. by GNU tar).
>
> has a rare bug: if there is exactly one trailing zero (because, for
> example, your compressed tar file was one byte short of a block), gzip
> gets only the first byte of the magic, and it's zero, but then it hits the
> end of the file on the second byte of the magic.
>
> I think this should affect only that case, and make it not fail:

Thank you!
I've applied your patch (small enough to get by without a copyright assignment)

  http://git.sv.gnu.org/cgit/gzip.git/commit/?id=118a107f2d3ae5267b

and added tests to exercise that bit of code:

  http://git.sv.gnu.org/cgit/gzip.git/commit/?id=8842154f6e88c37af3

>From 118a107f2d3ae5267b42e1aaac58a8b5ce9d1925 Mon Sep 17 00:00:00 2001
From: Daniel Barkalow <address@hidden>
Date: Sat, 10 Oct 2009 11:35:13 +0200
Subject: [PATCH 1/2] gzip: don't fail when there is exactly one trailing NUL 
byte

* gzip.c (get_method): Require the second byte of magic only if
the first byte was nonzero.
---
 gzip.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gzip.c b/gzip.c
index 8543128..b0f792a 100644
--- a/gzip.c
+++ b/gzip.c
@@ -1266,8 +1266,13 @@ local int get_method(in)
        /* If try_byte returned EOF, magic[1] == (char) EOF.  */
     } else {
        magic[0] = (char)get_byte();
-       magic[1] = (char)get_byte();
-       imagic1 = 0; /* avoid lint warning */
+       if (magic[0]) {
+           magic[1] = (char)get_byte();
+           imagic1 = 0; /* avoid lint warning */
+       } else {
+           imagic1 = try_byte ();
+           magic[1] = (char) imagic1;
+       }
     }
     method = -1;                 /* unknown yet */
     part_nb++;                   /* number of parts in gzip file */
--
1.6.5.rc3.227.g2ff1c


>From 8842154f6e88c37af3b62a4b5d11fdcb10aeb395 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 10 Oct 2009 11:29:33 +0200
Subject: [PATCH 2/2] gzip: add tests for today's bug fix

* tests/trailing-nul: New file.  Test for today's fix.
* Makefile.am (TESTS): Add new script.
* NEWS (Bug fixes): Mention it.
---
 Makefile.am        |    6 ++++--
 NEWS               |    3 +++
 tests/trailing-nul |   42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)
 create mode 100755 tests/trailing-nul

diff --git a/Makefile.am b/Makefile.am
index 206d5bf..3bca0ee 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -100,9 +100,11 @@ check-local: $(FILES_TO_CHECK) $(bin_PROGRAMS) gzip.doc.gz
          test "`cat $$k | ./gzip -dc $$k - $$k`" = aaa && rm $$k
        @echo 'Test succeeded.'

-TESTS =                \
-  tests/zdiff  \
+TESTS =                                                \
+  tests/trailing-nul                           \
+  tests/zdiff                                  \
   tests/zgrep-f
+
 EXTRA_DIST += $(TESTS)

 install-exec-hook: remove-installed-links
diff --git a/NEWS b/NEWS
index 5071241..5aeee71 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU gzip NEWS                                    -*- outline -*-

 ** Bug fixes

+  gzip no longer fails when there is exactly one trailing NUL byte
+  gzip has always accepted trailing NUL bytes.  Note the plural.
+
   zdiff would exit with status 2 (indicating an error) rather than 1 to
   indicate differences when both inputs were compressed and different.

diff --git a/tests/trailing-nul b/tests/trailing-nul
new file mode 100755
index 0000000..08b6c02
--- /dev/null
+++ b/tests/trailing-nul
@@ -0,0 +1,42 @@
+#!/bin/sh
+# gzip accepts trailing NUL bytes; don't fail if there is exactly one.
+# Before gzip-1.4, this would fail.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program 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.
+
+# This program 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/>.
+# limit so don't run it by default.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  gzip --version
+fi
+
+. $srcdir/tests/test-lib.sh
+
+(echo 0 | gzip; printf '\0') > 0.gz || framework_failure
+(echo 00 | gzip; printf '\0\0') > 00.gz || framework_failure
+(echo 1 | gzip; printf '\1') > 1.gz || framework_failure
+
+fail=0
+
+for i in 0 00 1; do
+  gzip -d $i.gz; ret=$?
+  test $ret -eq $i || fail=1
+  test $ret = 1 && continue
+  echo $i > exp || fail=1
+  compare $i exp || fail=1
+done
+
+Exit $fail
--
1.6.5.rc3.227.g2ff1c