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

Multiple Calls to zstd.(*Encoder).Close() Write Duplicate CRC Checksums #1016

Closed
Fusl opened this issue Oct 8, 2024 · 1 comment · Fixed by #1017
Closed

Multiple Calls to zstd.(*Encoder).Close() Write Duplicate CRC Checksums #1016

Fusl opened this issue Oct 8, 2024 · 1 comment · Fixed by #1017

Comments

@Fusl
Copy link

Fusl commented Oct 8, 2024

When invoking .Close() multiple times on a zstd.(*Encoder) instance, the CRC checksum is written to the underlying stream multiple times. This behavior prevents reliable use of defer zstdWr.Close() in conjunction with an explicit zstdWr.Close() mid-function, as it results in duplicate checksum entries.

Other io.WriteCloser implementations, such as Go's standard library gzip.(*Writer).Close(), prevent this issue by setting a closed flag internally, ensuring the Close() method is idempotent.

Steps to Reproduce:

package main

import (
	"bytes"
	"crypto/rand"
	"github.com/klauspost/compress/zstd"
)

func main() {
	buf := bytes.NewBuffer(nil)
	encoder, _ := zstd.NewWriter(buf)
	b := make([]byte, 131072)
	rand.Read(b)
	encoder.Write(b)
	encoder.Close()  // First close
	encoder.Close()  // Second close (shouldn't write checksum again)
	b = buf.Bytes()

	// Compare the last two 4-byte sequences in the buffer (which are the checksums)
	c1 := b[len(b)-4:]
	c2 := b[len(b)-8 : len(b)-4]
	if string(c1) == string(c2) {
		panic("Duplicate CRC checksums written")
	}
}
@klauspost
Copy link
Owner

Thanks for the report! Fix in #1017

klauspost added a commit that referenced this issue Oct 8, 2024
* zstd: Fix extra CRC written with multiple Close calls
* Also check write/flush after close.

Fixes #1016
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