Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only include files used by the crate #190

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Conversation

kornelski
Copy link
Contributor

@kornelski kornelski commented Apr 1, 2024

The libz-sys crate is 4MB large, but it only uses 1/10th of the data it bundles.

The zlib repo has all kinds of irrelevant files: a PDF documentation, support files for dead platforms, multiple build C systems, interfaces for various other languages, tests and support utilities. None of that is used by the sys crate.

Also the CVE-2024-3094 has demonstrated that seemingly standard and innocent files like configure and test fixtures can be used to hide malware. While I don't expect libz to do such thing, it is better to have fewer obscure files in the crate, so that it's easier to review.


I haven't done the same for the zlib-ng crate, because it uses CMake and it's harder to see which files it uses. It might be better to switch it to the cc crate first.

@kornelski
Copy link
Contributor Author

The CI failures seem to be only caused by network flakiness:

fatal: unable to access 'https://github.com/madler/zlib/': Recv failure: Connection was reset

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, that's a great catch!

I will merge this once CI is green, I restarted it, and also would expect it to work now.

@Byron Byron merged commit 0c3216c into rust-lang:main Apr 2, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants