Add Compression best practices guide#52968
Add Compression best practices guide#52968alinpahontu2912 wants to merge 8 commits intodotnet:mainfrom
Conversation
rzikm
left a comment
rzikm
left a comment
There was a problem hiding this comment.
It's getting better, few additional comments.
|
|
||
| ## Data integrity | ||
|
|
||
| ZIP entries include a CRC-32 checksum that you can use to verify data hasn't been corrupted or tampered with. |
There was a problem hiding this comment.
I think you should also mention TAR CRC in the first paragraph, now users might assume that this section is Zip-only and skip the rest of it.
There was a problem hiding this comment.
Pull request overview
Adds a new guidance article under File and stream I/O that explains how to work with ZIP and TAR archives in .NET, with a focus on API selection, safe extraction patterns, and operational considerations.
Changes:
- Adds a new best-practices article for ZIP and TAR archives, including security guidance for untrusted input.
- Adds a new C# snippet project and a consolidated
Program.cscontaining the referenced code regions. - Links the new article from
docs/fundamentals/toc.yml.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/standard/io/zip-tar-best-practices.md | New best-practices guide covering API choice, trusted vs. untrusted extraction, memory/perf, platform differences, and encryption notes. |
| docs/standard/io/snippets/zip-tar-best-practices/csharp/Project.csproj | New snippet project targeting net11.0 for compiling the article snippets. |
| docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs | Adds the C# snippet implementations referenced by the article. |
| docs/fundamentals/toc.yml | Adds a TOC entry pointing to the new best-practices article. |
|
cc also @GrabYourPitchforks and @blowdart for wording |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MihaZupan
left a comment
MihaZupan
left a comment
There was a problem hiding this comment.
It's great we're documenting this, thank you!
It'd be good if we were also able to provide better ways of getting these things right in the first place though.
| string fullDestDir = Path.GetFullPath(destinationDir); | ||
| if (!fullDestDir.EndsWith(Path.DirectorySeparatorChar)) | ||
| fullDestDir += Path.DirectorySeparatorChar; | ||
|
|
||
| foreach (ZipArchiveEntry entry in archive.Entries) | ||
| { | ||
| string destPath = Path.GetFullPath(Path.Join(fullDestDir, entry.FullName)); | ||
|
|
||
| if (!destPath.StartsWith(fullDestDir, StringComparison.Ordinal)) |
There was a problem hiding this comment.
Do we have any plans to make this easier to do correctly in the future?
E.g. the missing trailing separator on the destination dir is trivial to miss.
There was a problem hiding this comment.
We do have the trailing separator check for both zips and tars, but for the higher-level, convenience ExtractToDirectory methods. However, these are less secure and we recommend iterating through the archives and extracting entries one by one. The disadvantage is that some extra manual checks are necessary this way.
There was a problem hiding this comment.
@MihaZupan There is this proposal for Path.IsSubdirectory: dotnet/runtime#87581. But it doesn't seem to be very active. File system case sensitivity makes that more difficult as well.
| // </VulnerablePattern> | ||
|
|
||
| // <SafeExtractZip> | ||
| void SafeExtractZip(string archivePath, string destinationDir, |
There was a problem hiding this comment.
If the safe way to use our API is to copy-paste such a helper into your code, we should consider exposing better APIs
E.g. bool TryResolvePath(string destinationDirectory, out string path) on TarEntry/ZipArchiveEntry, and/or adding maxTotalSize/maxEntrySize/maxEntryCount as options to ExtractToDirectory.
There was a problem hiding this comment.
You've got a point, maybe we should consider creating per-entry safe extraction methods or add some options to let user decide how to interact with these apis
Summary
Add a guide explaining how to best work with Zip and Tar archives in .NET.
Internal previews