Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zstd.Decoder omitting errors for short invalid data #381

Closed
motiejus opened this issue May 28, 2021 · 1 comment · Fixed by #382
Closed

zstd.Decoder omitting errors for short invalid data #381

motiejus opened this issue May 28, 2021 · 1 comment · Fixed by #382

Comments

@motiejus
Copy link

When input data is 0-3 bytes long, I would expect zstd reader to emit an error, but doesn't. I found this when writing a test case for invalid data ("foo").

Barring other safeguards, this could have been a problem, when incoming data would be corrupted (e.g. empty file) and then successfully "decompressed" to an empty file. It would be good to emit an error when the input is wrong in any way, including short or empty, to prevent problems downstream.

zstd emits an error when the input is empty or too short:

$ echo -n | zstd -cd; echo $?
zstd: /*stdin*\: unexpected end of file 
1
$ echo -n foo | zstd -cd; echo $?
zstd: /*stdin*\: unknown header 
1

Test case:

diff --git a/zstd/decoder_test.go b/zstd/decoder_test.go
index 0476751..9cdf416 100644
--- a/zstd/decoder_test.go
+++ b/zstd/decoder_test.go
@@ -165,6 +165,31 @@ func TestErrorReader(t *testing.T) {
 	}
 }
 
+func TestShort(t *testing.T) {
+	for _, in := range []string{"", "f", "fo", "foo"} {
+		inb := []byte(in)
+		dec, err := NewReader(nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer dec.Close()
+
+		t.Run(fmt.Sprintf("DecodeAll-%d", len(in)), func(t *testing.T) {
+			_, err := dec.DecodeAll(inb, nil)
+			if err == nil {
+				t.Error("want error, got nil")
+			}
+		})
+		t.Run(fmt.Sprintf("Reader-%d", len(in)), func(t *testing.T) {
+			dec.Reset(bytes.NewReader(inb))
+			_, err := io.Copy(ioutil.Discard, dec)
+			if err == nil {
+				t.Error("want error, got nil")
+			}
+		})
+	}
+}
+
 func TestNewDecoder(t *testing.T) {
 	defer timeout(60 * time.Second)()
 	testDecoderFile(t, "testdata/decoder.zip")
klauspost added a commit that referenced this issue May 28, 2021
Detect short frame signatures.

Fixes #381
@klauspost
Copy link
Owner

@motiejus 0 bytes in -> 0 bytes out is a valid roundtrip.

If you want this, use WithZeroFrames and check the input before sending.

Sizes 1-3 fixed in #382

klauspost added a commit that referenced this issue May 28, 2021
Detect short frame signatures.

Fixes #381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants