go: Flush() does not completely process all data following a large Write() #415

New Issue

claunia · 2026-01-29T20:43:30Z

claunia commented

2026-01-29 20:43:30 +00:00

Originally created by @hochhaus on GitHub (Mar 11, 2022).

Thanks for providing the brotli library and corresponding cgo bindings.

I am seeing the Flush() calls not completely process all data following a call to a large Write(). Modifying your unit test TestEncoderFlush to use an input size of 32766 instead of 1000 demonstrates the behavior.

func TestEncoderFlush(t *testing.T) {
  input := make([]byte, 32766)  // MODIFIED
  rand.Read(input)
  out := bytes.Buffer{}
  e := NewWriter(&out, WriterOptions{Quality: 5})
  in := bytes.NewReader(input)
  _, err := io.Copy(e, in)
  if err != nil {
    t.Fatalf("Copy Error: %v", err)
  }
  if err := e.Flush(); err != nil {
    t.Fatalf("Flush(): %v", err)
  }
  if out.Len() == 0 {
    t.Fatalf("0 bytes written after Flush()")
  }
  decompressed := make([]byte, 32766)  // MODIFIED
  reader := NewReader(bytes.NewReader(out.Bytes()))
  n, err := reader.Read(decompressed)
  if n != len(decompressed) || err != nil {
    t.Errorf("Expected <%v, nil>, but <%v, %v>", len(decompressed), n, err)
  }
  reader.Close()
  if !bytes.Equal(decompressed, input) {
    t.Errorf(""+
      "Decompress after flush: %v\n"+
      "%q\n"+
      "want:\n%q",
      err, decompressed, input)
  }
  if err := e.Close(); err != nil {
    t.Errorf("Close(): %v", err)
  }
}

results in

    cbrotli_test.go:162: Expected <32766, nil>, but <32765, <nil>>
    cbrotli_test.go:166: Decompress after flush: <nil>
    ...

Is it expected that large Writes() are supported? Is the unit test somehow using the API incorrectly to ensure all data is both written and flushed?

Originally created by @hochhaus on GitHub (Mar 11, 2022). Thanks for providing the brotli library and corresponding cgo bindings. I am seeing the Flush() calls not completely process all data following a call to a large Write(). Modifying your unit test `TestEncoderFlush` to use an input size of `32766` instead of `1000` demonstrates the behavior. ``` func TestEncoderFlush(t *testing.T) { input := make([]byte, 32766) // MODIFIED rand.Read(input) out := bytes.Buffer{} e := NewWriter(&out, WriterOptions{Quality: 5}) in := bytes.NewReader(input) _, err := io.Copy(e, in) if err != nil { t.Fatalf("Copy Error: %v", err) } if err := e.Flush(); err != nil { t.Fatalf("Flush(): %v", err) } if out.Len() == 0 { t.Fatalf("0 bytes written after Flush()") } decompressed := make([]byte, 32766) // MODIFIED reader := NewReader(bytes.NewReader(out.Bytes())) n, err := reader.Read(decompressed) if n != len(decompressed) || err != nil { t.Errorf("Expected <%v, nil>, but <%v, %v>", len(decompressed), n, err) } reader.Close() if !bytes.Equal(decompressed, input) { t.Errorf(""+ "Decompress after flush: %v\n"+ "%q\n"+ "want:\n%q", err, decompressed, input) } if err := e.Close(); err != nil { t.Errorf("Close(): %v", err) } } ``` results in ``` cbrotli_test.go:162: Expected <32766, nil>, but <32765, <nil>> cbrotli_test.go:166: Decompress after flush: <nil> ... ``` Is it expected that large Writes() are supported? Is the unit test somehow using the API incorrectly to ensure all data is both written and flushed?

claunia closed this issue

2026-01-29 20:43:31 +00:00

claunia commented

2026-01-29 20:43:31 +00:00

@eustas commented on GitHub (Mar 11, 2022):

Hello. Thanks for the report. Will investigate on Monday.

@eustas commented on GitHub (Mar 11, 2022): Hello. Thanks for the report. Will investigate on Monday.

claunia commented

2026-01-29 20:43:31 +00:00

@hochhaus commented on GitHub (Mar 11, 2022):

Thanks @eustas.

@hochhaus commented on GitHub (Mar 11, 2022): Thanks @eustas.

claunia commented

2026-01-29 20:43:32 +00:00

@eustas commented on GitHub (Mar 14, 2022):

Reproducible. Digging in.

@eustas commented on GitHub (Mar 14, 2022): Reproducible. Digging in.

claunia commented

2026-01-29 20:43:32 +00:00

@eustas commented on GitHub (Mar 14, 2022):

Hoi. Well, it is not an issue.

io.Reader interface allows partial reads, if decoder decides it would be blocked to read further.
This is the case. If you add the second read in the test (or read in the loop, until all compressed input is consumed), the test will succeed.

TestEncoderFlush case checks different aspects of the codec behaviour, that is why payload length is chosen to be 1000.

From debugging output I've seen that all data passed to encoder is processed.

There are two checks that ensure correct behaviour:

io.Copy does not return error -> all input taken from source is consumed by encoder
out is not empty after Flush -> encoder was forced to process all consumed input

Overall: try to Read in the loop, until all compressed input is consumed. If it does not help, feel free to reopen this isssue.

@eustas commented on GitHub (Mar 14, 2022): Hoi. Well, it is not an issue. `io.Reader` interface allows partial reads, if decoder decides it would be blocked to read further. This is the case. If you add the second read in the test (or read in the loop, until all compressed input is consumed), the test will succeed. `TestEncoderFlush` case checks different aspects of the codec behaviour, that is why payload length is chosen to be 1000. From debugging output I've seen that all data passed to encoder is processed. There are two checks that ensure correct behaviour: * `io.Copy` does not return error -> all input taken from source is consumed by encoder * `out` is not empty after `Flush` -> encoder was forced to process all consumed input Overall: try to `Read` in the loop, until all compressed input is consumed. If it does not help, feel free to reopen this isssue.

claunia commented

2026-01-29 20:43:32 +00:00

@hochhaus commented on GitHub (Mar 15, 2022):

Thanks @eustas. As you say, looping the Read() fixes what I reported here.

It appears the real bug I'm chasing might actually be in firefox instead. I'll work on filing that upstream.

@hochhaus commented on GitHub (Mar 15, 2022): Thanks @eustas. As you say, looping the `Read()` fixes what I reported here. It appears the real bug I'm chasing might actually be in firefox instead. I'll work on filing that upstream.

claunia commented

2026-01-29 20:43:32 +00:00

@hochhaus commented on GitHub (Mar 15, 2022):

In case a future reader hits a similar issue as I did, I found that brotli compressed chunks great than 128k are not eagerly processed by FF. I filed an upstream issue.

@hochhaus commented on GitHub (Mar 15, 2022): In case a future reader hits a similar issue as I did, I found that brotli compressed chunks great than 128k are not eagerly processed by FF. I filed an [upstream issue](https://bugzilla.mozilla.org/show_bug.cgi?id=1759745).

claunia referenced this issue

2026-01-29 20:52:07 +00:00

[PR #415] Use version from common/version.h #1020

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#415