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

Encoder.ReadFrom() (added in v4) is buggy #183

Closed
lizthegrey opened this issue May 18, 2022 · 2 comments · Fixed by #184
Closed

Encoder.ReadFrom() (added in v4) is buggy #183

lizthegrey opened this issue May 18, 2022 · 2 comments · Fixed by #184

Comments

@lizthegrey
Copy link
Contributor

lizthegrey commented May 18, 2022

I appear to have truncated output (corresponding to partial input), and err.EOF when io.CopyN() followed by some followup Write() on v4, whereas v3 works fine and doesn't operate on only partial input.

when I buffer the output (which makes a copy for safety) in a bytes.Buffer() first before then using io.Copy() to send it all to lz4 at the same time, the output is not corrupted, because the copy is done with a single write and thus the Close() I manually trigger after is idempotent.

3d212bc (and all subsequent commits) fail, bb3370a passes my (non-minimal) testcase

@lizthegrey lizthegrey changed the title v4 does not adequately guard against post-facto mutation of buffer passed in via Write() ReadFrom() (added in v4) is buggy May 18, 2022
@lizthegrey lizthegrey changed the title ReadFrom() (added in v4) is buggy Encoder.ReadFrom() (added in v4) is buggy May 18, 2022
@lizthegrey
Copy link
Contributor Author

Verified my fixed branch works with go mod edit -replace

@lizthegrey
Copy link
Contributor Author

Repro code:

package main

import (
	"bytes"
	"encoding/hex"
	"fmt"
	"io"
	"os"
	"testing"

	lzv3 "github.com/pierrec/lz4/v3"
	lzv4 "github.com/pierrec/lz4/v4"
)

type Generator func(io.WriteCloser) io.WriteCloser

func TestFoo(t *testing.T) {
	for i, gen := range []Generator{initV3, initV4} {
		fmt.Printf("\n===Generator %d, piecemeal===\n", i)
		dumper := hex.Dumper(os.Stdout)
		coder := gen(dumper)
		for _, v := range inStream {
			n, err := io.CopyN(coder, bytes.NewReader(v), int64(len(v)))
			if n != int64(len(v)) || err != nil {
				fmt.Printf("Failed to write %v\n", err)
			}
		}
		coder.Close()
		dumper.Close()

		fmt.Printf("\n===Generator %d, clean buffered===\n", i)
		var allAtOnce bytes.Buffer
		for _, v := range inStream {
			n, err := io.CopyN(&allAtOnce, bytes.NewReader(v), int64(len(v)))
			if n != int64(len(v)) || err != nil {
				fmt.Printf("Failed to write: %v\n", err)
				break
			}
		}

		dumper2 := hex.Dumper(os.Stdout)
		coder2 := gen(dumper2)
		io.Copy(coder2, bytes.NewReader(allAtOnce.Bytes()))
		coder2.Close()
		dumper2.Close()
	}
	fmt.Println()
}

func initV3(out io.WriteCloser) io.WriteCloser {
	lz4Writer := lzv3.NewWriter(out)
	return lz4Writer
}

func initV4(out io.WriteCloser) io.WriteCloser {
	lz4Writer := lzv4.NewWriter(out)
	return lz4Writer
}

var inStream = [][]byte{
	{5, 0, 4, 0, 0, 0, 116, 104, 105, 115, 2, 0, 0, 0, 105, 115, 1, 0, 0, 0, 97, 4, 0, 0, 0, 116, 101, 115, 116, 8, 0, 0, 0, 112, 114, 111, 98, 97, 98, 108, 121},
	{1, 0, 0, 0, 13, 0, 0, 0},
	{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 15},
}

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.

1 participant