Add strict validation to System.IO.Compression.GZipStream#47560
Add strict validation to System.IO.Compression.GZipStream#47560mfkl wants to merge 2 commits intodotnet:masterfrom
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
@mfkl thanks for your investigation and contribution. The owners of this area will take a look, but note that any changes to public API have to go through a proposal and approval process described here: https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md |
Indeed, sorry about that. Let's take the conversation there #47563 |
|
Hello @mfkl Big thanks for your work and contribution. As @danmosemsft mentioned, we have a dedicated process for adding new APIs to .NET. Let's take the conversation to the issue that you have created (#47563), get a good API proposal, API approval, and then reopen the PR. I hope that you don't mind that. At our scale, we need to follow the process to ensure that we meet all the quality requirements. Thank you, |
The current implementation of
System.IO.Compression.GzipStreamseems to not be as strict regarding error handling as it could be when it comes to corrupt data.For context, this is a good thread about this exact issue, I believe. There is likely even more info in the old connect ticket but it's been unaccessible for a while.
So I have compared the
System.IO.Compression.GzipStreamimplementation with its managed and bindings counterparts, namelyzlib.managed,ICSharpCode.SharpZipLibandSharpCompresswhen it comes to handling corrupt data.As for test cases, I retrieved the code from the SO question and also added another test which hits a different code path.
This is the other test case https://github.com/mfkl/zlib-validation/blob/main/ConsoleApp1/Program.cs
This is the result:
My understanding is that the
System.IO.Compression.GzipStreamimplementation ignoresZLibNative.ErrorCode.BufErrorerrors in most cases, while other implementions are stricter about it.My proposed fix is to add an overload with a simple boolean which, when set to true, would take into account the BufError returned by the native lib, and throw accordingly.
I've run the compression test suites and made sure I did not break anything. If there is another approach to go at this, feel free to let me know.
Feedback welcome!