Bug 256330

Summary: Unexpected "Extra bytes past the end" error when decompressing data with DecompressionStream
Product: WebKit Reporter: gildas <gildas.lormeau>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: 32jojo32, achristensen, ap, ashley, bfulgham, brandonstewart, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Safari 16   
Hardware: Unspecified   
OS: macOS 13   

Description gildas 2023-05-04 12:41:28 PDT
I'm the author of zip.js, see https://github.com/gildas-lormeau/zip.js/. A user has reported a bug that is specific to Safari. This bug is a regression introduced in the version 16.4. An unexpected error "TypeError: Extra bytes past the end" is thrown when unzipping a file. When disabling the usage of the `DecompressionStream` API in zip.js (via the option `useCompressionStream`), the bug disappears.

Unfortunately, I cannot provide this zip file. You can find more information here: https://github.com/gildas-lormeau/zip.js/issues/412
Comment 1 Alexey Proskuryakov 2023-05-04 16:55:50 PDT
Thank you for the report! Unfortunately, this seems unlikely to be actionable without an example archive.

> This bug is a regression introduced in the version 16.4.

Could you please elaborate on this? Safari 16.4 is the first version to support DecompressionStream, so it could never work differently before. Are you saying that something used to work as expected using a polyfill, and doesn't work with native implementation?
Comment 2 gildas 2023-05-04 17:45:27 PDT
Indeed, I was meaning it's a regression in zip.js that has been introduced by Safari 16.4. In Safari 16.3 (and Firefox), zip.js uses its own implementation. Note that zip.js does not contain user-agent sniffing code. Therefore, I am very reluctant to circumvent it.
Comment 3 gildas 2023-05-04 17:48:18 PDT
By the way, what is the "TypeError: Extra bytes past the end" error? It does not seem to be specified here: https://wicg.github.io/compression/#compression-stream.
Comment 4 gildas 2023-05-04 18:15:39 PDT
Actually, the error seems to be thrown when reading 128 bytes at the offset 1664 of a compressed stream weighting 247,454 bytes. It looks like the error would be thrown probably when creating a `Uint8Array()` of 128 bytes containing the decompressed data.
Comment 5 gildas 2023-05-04 18:18:07 PDT
Sorry, I was meaning "128 bytes containing the **compressed** data." in the last sentence of my last comment.
Comment 6 Alexey Proskuryakov 2023-05-05 08:55:40 PDT
It's raised here: https://github.com/WebKit/WebKit/blob/main/Source/WebCore/Modules/compression/DecompressionStreamDecoder.cpp#L138.

I am not familiar with this API or code, but from quickly glancing at the spec, it does specify raising a TypeError for decoding errors.
Comment 7 Radar WebKit Bug Importer 2023-05-05 08:55:55 PDT
<rdar://problem/108950984>
Comment 8 gildas 2023-05-05 11:01:56 PDT
Thank you for the additional info. It looks like this issue is very similar to these ones in zip.js: https://github.com/gildas-lormeau/zip.js/issues/44, https://github.com/gildas-lormeau/zip.js/issues/101.

I'm trying to find the zip file that was attached. It also looks like it was possible to reproduce the issue by using .NET's DeflateStream in 2013 (see first link).
Comment 9 gildas 2023-05-05 13:11:43 PDT
I was not able to find the zip file unfortunately.
Comment 10 gildas 2023-05-05 13:50:36 PDT
I was able to find an example of code to reproduce the issue inspired from this thread on SO: https://stackoverflow.com/questions/67526864/how-to-inflate-a-gzip-with-a-really-old-zlib.

Steps to reproduce the issue:
- Open a new tab in Safari
- Open the developer tools
- Run the code below
`
(async () => {
	const result = new TransformStream();
	(new Blob([new Uint8Array([0xec, 0x9a, 0x7f, 0x54, 0x1c, 0x55, 0x96, 0xc7, 0x6f, 0x75, 0x37, 0xd0, 0xfc, 0x70])])).stream().pipeThrough(new DecompressionStream("deflate-raw")).pipeTo(result.writable);
	console.log(await new Response(result.readable).blob());
})();
`

Expected result:
An error similar to "TypeError: The input is ended without reaching the stream end" (Firefox) or "TypeError: Compressed input was truncated." (Chrome) should be thrown.

Result in Safari 16.4:
The error "TypeError: Extra bytes past the end." is thrown instead.

The code run as expected in:
- Firefox nightly: yes
- Chrome: yes
Comment 11 gildas 2023-05-05 17:46:50 PDT
Sorry, forget my last comment. I think my test is wrong.
Comment 12 Jozef 2023-05-16 03:28:53 PDT
I believe I might have a better example. The example I share tries to compress text and then decompress. Chrome handles it correctly. Unfortunately text is pretty long as I was not able to replicate it on smaller substring.


To replicate the bug open the file and you should see one error log and one success log. Chrome shows two success logs.

https://gist.github.com/Joozty/3bd03a81d4d6540eaea683d9c6389703
Comment 13 Brandon 2023-05-16 09:35:48 PDT
@Jozef Thanks for the test case; I just tried your example and in Safari and STP I was seeing the issue. 

When I tried a build off of mainline though I am seeing 2 success messages in the console (using MiniBrower).


I integrated a fix a few days ago that handled an edge case during the flush step.

https://github.com/WebKit/WebKit/pull/13741
Comment 14 Ashley Gullen 2023-05-17 05:04:43 PDT
So did Safari 16.4 ship broken CompressionStreams when using large amounts of content? This in turn would break the zip.js library in many cases, and would explain why we've been getting a bunch of customer support queries about errors opening zip-based content in our web app...
Comment 15 Brent Fulgham 2023-05-17 13:02:06 PDT
(In reply to Ashley Gullen from comment #14)
> So did Safari 16.4 ship broken CompressionStreams when using large amounts
> of content? This in turn would break the zip.js library in many cases, and
> would explain why we've been getting a bunch of customer support queries
> about errors opening zip-based content in our web app...

Compression Streams first shipped in Safari 16.4, so yes -- these bugs would be new with that release.

We think this issue is a duplicate of Bug 254021, which has a test case that we were able to use to confirm and fix.
Comment 16 Brent Fulgham 2023-05-17 13:35:17 PDT

*** This bug has been marked as a duplicate of bug 254021 ***