-
Notifications
You must be signed in to change notification settings - Fork 21
Explicitly call out what to do on the end of the compressed input #77
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
base: main
Are you sure you want to change the base?
Conversation
1. If |buffer| is not empty: | ||
1. Let |arrays| be the result of splitting |buffer| into one or more non-empty pieces and converting them into {{Uint8Array}}s. | ||
1. [=list/For each=] {{Uint8Array}} |array| of |arrays|, [=TransformStream/enqueue=] |array| in |ds|'s [=GenericTransformStream/transform=]. | ||
1. If the end of the compressed input has not been reached, then throw a {{TypeError}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the order of what Blink does, but does this matter right now? I couldn't craft a test input where this actually matters - flushing would actually enqueue any extra chunk, even though this theoretically can happen..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of inflate() used by Blink has this comment:
In this implementation, the flush parameter of inflate() only affects the
return code (per zlib.h). inflate() always writes as much as possible to
strm->next_out, given the space available and the provided input--the effect
documented in zlib.h of Z_SYNC_FLUSH.
I think this implies that in some other implementation it could make a difference, but it doesn't in Blink.
I think it anyway it can only apply to the "deflate-raw" format, as "deflate" and "gzip" both end with checksums.
RFC1951 doesn't seem to shed any light on the behaviour of decompressors.
My feeling is that it is good for the standard to be explicit about how to handle any remaining output, even if real implementations don't need this step.
…maug This corresponds to the not-merged-yet spec PR: whatwg/compression#77 Differential Revision: https://phabricator.services.mozilla.com/D266433
This corresponds to the not-merged-yet spec PR: whatwg/compression#77 Differential Revision: https://phabricator.services.mozilla.com/D266433 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1990921 gecko-commit: 86cb8e42ea52ae9f19fd1edb128d9abf2bb009d3 gecko-reviewers: smaug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
1. If |buffer| is not empty: | ||
1. Let |arrays| be the result of splitting |buffer| into one or more non-empty pieces and converting them into {{Uint8Array}}s. | ||
1. [=list/For each=] {{Uint8Array}} |array| of |arrays|, [=TransformStream/enqueue=] |array| in |ds|'s [=GenericTransformStream/transform=]. | ||
1. If the end of the compressed input has not been reached, then throw a {{TypeError}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of inflate() used by Blink has this comment:
In this implementation, the flush parameter of inflate() only affects the
return code (per zlib.h). inflate() always writes as much as possible to
strm->next_out, given the space available and the provided input--the effect
documented in zlib.h of Z_SYNC_FLUSH.
I think this implies that in some other implementation it could make a difference, but it doesn't in Blink.
I think it anyway it can only apply to the "deflate-raw" format, as "deflate" and "gzip" both end with checksums.
RFC1951 doesn't seem to shed any light on the behaviour of decompressors.
My feeling is that it is good for the standard to be explicit about how to handle any remaining output, even if real implementations don't need this step.
Test is pending automerge in web-platform-tests/wpt#55118. Feel free to merge this PR as I don't have the write permission. |
Fixes #76
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff