[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Rare issue in ignoring padding
From: |
Jim Meyering |
Subject: |
Re: Rare issue in ignoring padding |
Date: |
Sat, 10 Oct 2009 11:41:46 +0200 |
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