Fix Zip64ExtraField handling #111802
Fix Zip64ExtraField handling #111802
Conversation
ghost
added
the
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-io-compression |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
/azp list |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs
Show resolved
Hide resolved
Just double checking: The outerloop tests get skipped by default AFAIK even when you run them locally, unless you pass the necessary msbuild global property in the command (if from CLI). Did you do this by any chance? I think such suppression does not happen if you run the unit test from VS or VS Code instead. You can choose the specific test and it gets executed. |
|
The VS test runner skips outerloop tests, even if I specifically select them. Those tests remain in the "not executed" state, but when that state rolls up to the parent, it looks like the entire suite passed... It's an odd problem, but I've brute-forced it for the moment - I just locally commented out the |
|
Thanks for checking! So far the results don't show compression-related failures. I'm monitoring the remaining ones too. |
Merging now! |
* main: (22 commits) Clean up Stopwatch a bit (dotnet#111834) JIT: Fix embedded broadcast simd size (dotnet#111638) Revert potential UB due to aliasing + more WB removals (dotnet#111733) re-enable acceleration of Vector512<long>.op_Multiply (dotnet#111832) Handle OSSL 3.4 change to SAN:othername formatting JIT: Fix stack allocated arrays for NativeAOT (dotnet#111827) JIT: enhance RBO inference for similar compares to constants (dotnet#111766) JIT: Don't run optSetBlockWeights when we have PGO data (dotnet#111764) [Android] Make sure RuntimeFlavor=CoreCLR when clr subset is specified (dotnet#111821) Change empty subject test certificate to include a critical SAN. Fix reversed code offsets in GcInfo (dotnet#111792) Swap some libraries areas between leads (dotnet#111816) Add left-handed spherical and cylindrical billboards (dotnet#109605) JIT: revise `optRelopImpliesRelop` to always set `reverseSense` (dotnet#111803) Fix Zip64ExtraField handling (dotnet#111802) Add build support for Android+CoreCLR (dotnet#110471) arm64: Add bic(s) compact encoding (dotnet#111452) JIT: Ensure `BBF_PROF_WEIGHT` flag is set when we have PGO data (dotnet#111780) Add support for AVX10.2, Add AVX10.2 API surface and template tests (dotnet#111209) JIT: Preliminary for enabling inlining late devirted calls (dotnet#111782) ...
2 participants
Bugfix for the outer loop tests which failed after PR #103153.
The original
Zip64ExtraField.WriteBlockmethod contained logic to conditionally write the compressed size, uncompressed size, local header offset and starting disk number to the output stream. When this was modified to use BinaryPrimitives, (link) this condition wasn't ported across properly - it conditionally wrote into the destination buffer, but used constant field offsets. There was also one error in ZipArchiveEntry.WriteDataDescriptor - that's now corrected.I don't see this pattern repeated anywhere else in ZipBlocks or ZipArchiveEntry. ZipArchiveEntry does sometimes write a ZIP64 data descriptor (link) but this logic is reasonable.
Outer loop tests which failed as a result of this bug:
These were the only tests which failed because they're the only ones which generated large enough files to trigger the logic which wrote Zip64ExtraField. They're now passing locally.
Apologies again for missing this.
@carlossanlop - if you're happy with the diff, could you run the outer loop tests against this PR again please?