refactor: simplify buffer reader usage and enhance zip extraction logic #1791
Conversation
src/source/extract.rs
Outdated
| let mut file = archive | ||
| .by_index(i) | ||
| .map_err(|e| SourceError::ZipExtractionError(e.to_string()))?; | ||
| let outpath = tmp_extraction_dir.path().join(file.mangled_name()); |
There was a problem hiding this comment.
https://docs.rs/zip/latest/zip/read/struct.ZipFile.html#method.mangled_name basically what we are doing is using zip crate's mangled_name to manually extract correcly the paths into files + folders with sanitized file names
There was a problem hiding this comment.
I don't think we should use that function as the docs recommend: enclosed_name.
This will read well-formed ZIP files correctly, and is resistant to path-based exploits. It is recommended over ZipFile::mangled_name.
Why does mangled name fix anything?
There was a problem hiding this comment.
enclosed_name causes windows file lock issues, which seems to give me problematic results on windows. I would love your input on that, retrying is an approach but i am not sure about it for zip extraction
|
It is still unclear to me what this solves? can you maybe explain the exact error this solves? You only link to an issue listing and the recipe you reference fails in a cargo invocation, not in rattler-build. |
this will solve the zed stock problem @baszalmstra is facing the recipe i used with test is the same one https://github.com/baszalmstra/zed-feedstock/blob/be25438c26f8f44a70566f6ea992367c04c55580/recipe/recipe.yaml
the problem seems to be the zip crate's bug itself, which fails to extract certain zip files on windows and sometimes even fails to extract them correctly https://github.com/zip-rs/zip2/issues?q=is%3Aissue%20state%3Aopen%20windows (seems like a recent issue but no comments so far on any of the bugs, which i think pretty critical) so instead with the same logic we are extracting them manually and it works correctly now