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 spec for Compress::Gzip::Writer with extra #14788

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jul 6, 2024

This adds a spec to ensure that writing an extra header field works.

Follow-up to #14550

Note the generated gzip data is not binary identical to the test.gz fixture, so we cannot directly compare the written gzip data.
Instead, the spec reads the data and verifies it has the same content as test.gz.
I believe the difference is because Compress::Gzip::Writer does not include a CRC for the header data (ref C implementation).

Compress::Gzip::Writer:
00000000  1f 8b 08 1c 4d 7b 46 50  00 03 05 00 01 02 03 04  ....M{FP........
00000010  05 74 65 73 74 2e 74 78  74 00 68 61 70 70 79 20  .test.txt.happy
00000020  62 69 72 74 68 64 61 79  00 f3 cf 4b e5 0a 29 cf  birthday...K..).
00000030  07 00 09 b9 78 f5 07 00  00 00                    ....x.....

test.gz:
00000000  1f 8b 08 1f 4d 7b 46 50  02 03 05 00 01 02 03 04  ....M{FP........
00000010  05 74 65 73 74 2e 74 78  74 00 68 61 70 70 79 20  .test.txt.happy
00000020  62 69 72 74 68 64 61 79  00 a1 2d f3 cf 4b e5 0a  birthday..-..K..
00000030  29 cf 07 00 09 b9 78 f5  07 00 00 00              ).....x.....

@straight-shoota straight-shoota added this to the 1.13.0 milestone Jul 8, 2024
@straight-shoota straight-shoota merged commit 0571f19 into crystal-lang:master Jul 8, 2024
61 checks passed
@straight-shoota straight-shoota deleted the spec/compress-gzip-write-extra branch July 8, 2024 19:19
kojix2 pushed a commit to kojix2/crystal that referenced this pull request Jul 9, 2024
kojix2 pushed a commit to kojix2/crystal that referenced this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants