254021 2023-03-16 07:17:08 -0700 Compression Streams not handling large outputs during the flush stage 2023-05-17 13:35:17 -0700 1 1 1 Unclassified WebKit WebCore Misc. Safari Technology Preview Mac (Apple Silicon) macOS 12 RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=252474 InRadar P2 Normal --- 1 bzugmeyer brandonstewart brandonstewart cdumez gildas.lormeau kurt webkit-bug-importer oldest_to_newest 1941838 0 bzugmeyer 2023-03-16 07:17:08 -0700 I am testing the new `CompressionStream` API in Safari Technology Preview. When trying to compress a moderately large buffer, the data it produces seems invalid. To reproduce: Compress a large enough buffer of non-repeating data. In the following example, I use a buffer of approximately 48kB. Functions are coming from the WebKit CompressionStream test suite[1]: const input = new TextEncoder().encode( JSON.stringify(Array.from({ length: 10_000 }, (_, i) => i)) ); const output = await compressArrayBuffer(input, "deflate"); assert_array_equals(input, pako.inflate(output)); Expected: `pako.inflate` returns a buffer equal to the input buffer. Actual: The decompression fails with `pako.inflate` returning `undefined`. Notes: With this particular example, I noticed that it works correctly when compressing up to 35_578 bytes. The issue only occurs when I try to compress more bytes (>= 35_579). Please see this page[2] for a practical way to reproduce the issue. In Chrome, every test succeed, but in Safari tests with larger inputs are failing. [1]: https://github.com/WebKit/WebKit/blob/20329b62061b40d5a423a1d75b67779945b84729/LayoutTests/imported/w3c/web-platform-tests/compression/compression-stream.tentative.any.js [2]: https://safari-compressionstream-issue.benoitzugmeyer.repl.co 1941862 1 ap 2023-03-16 08:59:08 -0700 Hopefully this is a dupe of bug 252474. 1943362 2 webkit-bug-importer 2023-03-23 07:18:14 -0700 <rdar://problem/107133345> 1943446 3 brandonstewart 2023-03-23 11:41:36 -0700 Thanks for the bug report I can reproduce in Safari, Safari Technology Preview, and MiniBrowser. This is not a dupe of the other one from what I can tell at first glance. <!DOCTYPE html> <html> <script type="module"> import * as pako from "https://cdnjs.cloudflare.com/ajax/libs/pako/2.1.0/pako.esm.mjs"; import * as fflate from "https://unpkg.com/fflate@0.7.4/esm/browser.js"; // JSON-encoded array of 10 thousands numbers ("[0,1,2,...]"). This produces 48_891 bytes of data. const fullData = new TextEncoder().encode( JSON.stringify(Array.from({ length: 10_000 }, (_, i) => i)) ); await test(10) await test(1_000) await test(10_000) await test(30_000) await test(40_000) // fails await test(35_578) // succeeds await test(35_579) // fails async function test(bytesLength) { const data = fullData.subarray(0, bytesLength) const compressedData = await compressArrayBuffer(data, "deflate"); // Decompress with pako, and check that we got the same result as our original string // Similar to https://github.com/WebKit/WebKit/blob/20329b62061b40d5a423a1d75b67779945b84729/LayoutTests/imported/w3c/web-platform-tests/compression/compression-stream.tentative.any.js#L54-L55 try { assert_array_equals(data, pako.inflate(compressedData)); console.log(`[pako] Succeeded with ${bytesLength} bytes`) } catch (error) { console.error(`[pako] Failed with ${bytesLength} bytes:`, error) } // Double check with another library try { assert_array_equals(data, fflate.unzlibSync(compressedData)); console.log(`[fflate] Succeeded with ${bytesLength} bytes`) } catch (error) { console.error(`[fflate] Failed with ${bytesLength} bytes:`, error) } try { assert_array_equals(data, await decompress(compressedData)); console.log(`deflate succeed with ${bytesLength} bytes`); } catch (error) { console.error(`deflate Failed with ${bytesLength} bytes`, error); } } async function compressArrayBuffer(input, format) { const cs = new CompressionStream(format); const writer = cs.writable.getWriter(); writer.write(input); const closePromise = writer.close(); const out = []; const reader = cs.readable.getReader(); let totalSize = 0; while (true) { const { value, done } = await reader.read(); if (done) break; out.push(value); totalSize += value.byteLength; } await closePromise; const concatenated = new Uint8Array(totalSize); let offset = 0; for (const array of out) { concatenated.set(array, offset); offset += array.byteLength; } return concatenated; } function assert_array_equals(a, b) { if (!a) { throw new Error(`Arrays not equal: a is falsy (${a})`) } if (!b) { throw new Error(`Arrays not equal: b is falsy (${b})`) } if (!a.length === b.length) { throw new Error(`Arrays not equal: a.length !== b.length (${a.length} !== ${b.length})`) } a.forEach((v, i) => { if (a[i] !== b[i]) { throw new Error(`Arrays not equal: a[${i}] !== b[${i}] (${a[i]} !== ${b[i]})`) } }) } async function concatenateStream(readableStream) { const reader = readableStream.getReader(); let totalSize = 0; const buffers = []; while (true) { const { value, done } = await reader.read(); if (done) { break; } buffers.push(value); totalSize += value.byteLength; } reader.releaseLock(); const concatenated = new Uint8Array(totalSize); let offset = 0; for (const buffer of buffers) { concatenated.set(buffer, offset); offset += buffer.byteLength; } return concatenated; } async function decompress(view) { const ds = new DecompressionStream('deflate'); const writer = ds.writable.getWriter(); writer.write(view); writer.close(); return await concatenateStream(ds.readable); } </script> </html> 1954615 4 brandonstewart 2023-05-10 23:14:49 -0700 So what was happening here is that we missed an edge case where during the flush step we may have data longer than the allocated output. Since the avail_in was set to 0 we would just exit. We need to verify that the stream has ended before exiting. The reason the 35_579 size failed was the allocated output during the flush step was 16384, while the output data was just slightly larger. The first compression outputted only 2 bytes, while the remaining 13686 were only spit out during the flush step. This fixes it. @@ -132,8 +131,8 @@ ExceptionOr<RefPtr<JSC::ArrayBuffer>> CompressionStreamEncoder::compress(const u result = deflate(&m_zstream, (m_finish) ? Z_FINISH : Z_NO_FLUSH); if (result != Z_OK && result != Z_STREAM_END && result != Z_BUF_ERROR) return Exception { TypeError, "Failed to compress data."_s }; - - if (!m_zstream.avail_in) { + + if (!m_zstream.avail_in && (!m_finish || (m_finish && result == Z_STREAM_END))) { shouldCompress = false; output.resize(allocateSize - m_zstream.avail_out); } Working on the PR now. 1954617 5 brandonstewart 2023-05-10 23:18:29 -0700 Pull request: https://github.com/WebKit/WebKit/pull/13741 1954886 6 ews-feeder 2023-05-11 20:07:13 -0700 Committed 263997@main (a30d9284f5b9): <https://commits.webkit.org/263997@main> Reviewed commits have been landed. Closing PR #13741 and removing active labels. 1956006 7 bfulgham 2023-05-17 13:35:17 -0700 *** Bug 256330 has been marked as a duplicate of this bug. ***