Return of BrotliInputStream.Read(...) Does not work with CopyTo #384

Open
opened 2026-01-29 20:43:03 +00:00 by claunia · 3 comments
Owner

Originally created by @solamanhuq on GitHub (Apr 27, 2021).

Not sure what/if there is an intended target, but at least for my case, the csharp code fails when performing a BrotliInputStream.CopyTo.

The reason I have identified has to do with this line, which apparently does not conform with a valid output as per the online docs.

Looking further into it, this is necessarily the case, as CopyTo will use a given buffer to continue reading input and eventually copying to destination stream, expecting a 0 when there is nothing written to the buffer (source code).

Given that the current code does not confirm with this, can we change the -1 to 0?
Fwiw I'd argue this is a safe change; there shouldn't be a particular reason any dependent code would treat a no-op differently depending on an offset or start of buffer write.

Originally created by @solamanhuq on GitHub (Apr 27, 2021). Not sure what/if there is an intended target, but at least for my case, the csharp code fails when performing a `BrotliInputStream.CopyTo`. The reason I have identified has to do with this [line](https://github.com/google/brotli/blob/2a51a85aa86abb4c294c65fab57f3d9c69f10080/csharp/org/brotli/dec/BrotliInputStream.cs#L181), which apparently does not conform with a valid output as per the [online docs](https://docs.microsoft.com/en-us/dotnet/api/system.io.stream.read?view=net-5.0#System_IO_Stream_Read_System_Byte___System_Int32_System_Int32_). Looking further into it, this is necessarily the case, as CopyTo will use a given buffer to continue reading input and eventually copying to destination stream, expecting a 0 when there is nothing written to the buffer ([source code](https://referencesource.microsoft.com/#mscorlib/system/io/stream.cs,218)). Given that the current code does not confirm with this, can we change the `-1` to `0`? Fwiw I'd argue this is a safe change; there shouldn't be a particular reason any dependent code would treat a no-op differently depending on an offset or start of buffer write.
claunia added the release-v1.1.1 label 2026-01-29 20:43:03 +00:00
Author
Owner

@paulushub commented on GitHub (Sep 14, 2021):

Just came here to report the same issue. Since the C# version is a translation of the Java version, someone thought Java return value of -1 will work for C#/.NET.
Fixed it in my local copy.

@paulushub commented on GitHub (Sep 14, 2021): Just came here to report the same issue. Since the C# version is a translation of the Java version, someone thought Java return value of `-1` will work for C#/.NET. Fixed it in my local copy.
Author
Owner

@eustas commented on GitHub (Dec 15, 2021):

Hello. Thanks for the heads-up.
I have plans for refreshing the C# code (and code-generation). Will include this fix in the update.

@eustas commented on GitHub (Dec 15, 2021): Hello. Thanks for the heads-up. I have plans for refreshing the C# code (and code-generation). Will include this fix in the update.
Author
Owner

@TheRealNem commented on GitHub (Sep 19, 2022):

This bit me too. Some .Net API code checks explicitly for 0, so things crash in confusing ways when you pass in a BrotliInputStream (in my case it was a XmlReader). If you change the return value, be aware that BrotliInputStream.ReadByte also compares the return value of BrotliInputStream.Read to -1.

@TheRealNem commented on GitHub (Sep 19, 2022): This bit me too. Some .Net API code checks explicitly for 0, so things crash in confusing ways when you pass in a BrotliInputStream (in my case it was a XmlReader). If you change the return value, be aware that BrotliInputStream.ReadByte also compares the return value of BrotliInputStream.Read to -1.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/brotli#384