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

Add crc32 algorithm to DigestStream #3358

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

danlapid
Copy link
Collaborator

Since openssl does not support crc32 as a digest algorithm I extended our support through a new self implemented class and took the initial implementation out to it's own class.

Solves internal ticket #8999

@danlapid danlapid requested review from a team as code owners January 18, 2025 19:06
src/workerd/api/crypto/crypto.c++ Outdated Show resolved Hide resolved
src/workerd/api/crypto/crypto.c++ Outdated Show resolved Hide resolved
@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch from b29e9cc to 6b25145 Compare January 20, 2025 11:57
@danlapid danlapid requested a review from anonrig January 20, 2025 11:58
@danlapid
Copy link
Collaborator Author

@anonrig @jasnell fixed comments.
Any opposition to merging?

@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch from 6b25145 to 8ecaf8e Compare January 20, 2025 13:20
@danlapid
Copy link
Collaborator Author

Windows is still failing to do the endian conversion for some reason.
And we want two more algorithms implemented, I'll move back to draft for a bit.

@danlapid danlapid marked this pull request as draft January 20, 2025 14:47
@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch from 8ecaf8e to 4d89ed4 Compare January 20, 2025 14:49
@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch 2 times, most recently from eddc015 to d78071d Compare January 20, 2025 17:35
Copy link

github-actions bot commented Jan 20, 2025

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch 2 times, most recently from 927633b to 312fc25 Compare January 20, 2025 18:42
@danlapid danlapid marked this pull request as ready for review January 20, 2025 19:02
@danlapid danlapid requested review from anonrig and jasnell January 20, 2025 19:02
@danlapid
Copy link
Collaborator Author

@jasnell @anonrig quite a bit has changed so I reset your approvals.

Windows is passing now though sadly it required a whole endianness file for it to actually work reliably.

src/workerd/api/crypto/crc-impl.c++ Outdated Show resolved Hide resolved
src/workerd/api/crypto/crc-impl.c++ Outdated Show resolved Hide resolved
src/workerd/api/crypto/crypto.c++ Outdated Show resolved Hide resolved
@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch from 312fc25 to fe6cfac Compare January 21, 2025 00:13
Since openssl does not support crc as a digest algorithm I extended
our support through a new self implemented class and took the initial
implementation out to it's own class.

crc32 support is implemented using zlib's implementation. crc32c and
crc64nvme are supported using our own implementation to generate
the crc table at compiletime.
@danlapid danlapid force-pushed the dlapid/crc32_digeststream branch from fe6cfac to a10436c Compare January 21, 2025 00:28
@danlapid danlapid enabled auto-merge January 21, 2025 00:29
@danlapid danlapid merged commit c15de7e into main Jan 21, 2025
17 checks passed
@danlapid danlapid deleted the dlapid/crc32_digeststream branch January 21, 2025 00:51
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.

4 participants