Skip to content

Commit bf7bef4

Browse files
marcopiracciniaduh95
authored andcommitted
zlib: avoid an extra empty zstd frame
Signed-off-by: marcopiraccini <marco.piraccini@gmail.com> PR-URL: #66161 Fixes: #66078 Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0ddc6b7 commit bf7bef4

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

‎src/node_zlib.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ class ZstdCompressContext final : public ZstdContext {
385385

386386
uint64_t pledged_src_size_ = ZSTD_CONTENTSIZE_UNKNOWN;
387387
std::optional<uint64_t> consumed_src_size_;
388+
bool frame_ended_ = false;
388389

389390
// A frame is complete once ZSTD_compressStream2() has been called with
390391
// ZSTD_e_end and has returned 0. Resetting an incomplete frame is only unsafe
@@ -1787,6 +1788,7 @@ CompressionError ZstdCompressContext::Init(uint64_t pledged_src_size,
17871788
std::string_view dictionary,
17881789
bool) {
17891790
pledged_src_size_ = pledged_src_size;
1791+
frame_ended_ = false;
17901792
frame_complete_ = true;
17911793
frame_output_emitted_ = false;
17921794
if (pledged_src_size == ZSTD_CONTENTSIZE_UNKNOWN) {
@@ -1859,6 +1861,7 @@ CompressionError ZstdCompressContext::ResetStream() {
18591861
} else {
18601862
consumed_src_size_ = 0;
18611863
}
1864+
frame_ended_ = false;
18621865
frame_complete_ = true;
18631866
frame_output_emitted_ = false;
18641867
error_ = ZSTD_error_no_error;
@@ -1868,13 +1871,18 @@ CompressionError ZstdCompressContext::ResetStream() {
18681871
}
18691872

18701873
void ZstdCompressContext::DoThreadPoolWork() {
1874+
// Ending an already ended frame would append an empty one.
1875+
if (frame_ended_ && input_.size == 0) {
1876+
return;
1877+
}
18711878
// Zstd overrides a configured pledge when the first call uses ZSTD_e_end.
18721879
size_t const input_pos = input_.pos;
18731880
size_t const remaining =
18741881
ZSTD_compressStream2(cctx_.get(), &output_, &input_, flush_);
18751882
if (consumed_src_size_.has_value()) {
18761883
*consumed_src_size_ += input_.pos - input_pos;
18771884
}
1885+
frame_ended_ = remaining == 0 && flush_ == ZSTD_e_end;
18781886
if (output_.pos > 0) {
18791887
frame_output_emitted_ = true;
18801888
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
3+
// Regression test for https://github.com/nodejs/node/issues/66078: ending a
4+
// ZstdCompress stream with writes still queued appended an empty frame.
5+
6+
const common = require('../common');
7+
const assert = require('assert');
8+
const { finished } = require('stream/promises');
9+
const zlib = require('zlib');
10+
11+
const kMagic = Buffer.from([0x28, 0xb5, 0x2f, 0xfd]);
12+
13+
function countFrames(buffer) {
14+
let count = 0;
15+
for (let i = buffer.indexOf(kMagic); i !== -1; i = buffer.indexOf(kMagic, i + 1)) {
16+
count++;
17+
}
18+
return count;
19+
}
20+
21+
async function compress(use) {
22+
const stream = zlib.createZstdCompress();
23+
const chunks = [];
24+
stream.on('data', (chunk) => chunks.push(chunk));
25+
await use(stream);
26+
stream.end();
27+
await finished(stream);
28+
return Buffer.concat(chunks);
29+
}
30+
31+
(async () => {
32+
const expected = await compress((stream) => stream.write('hello world'));
33+
assert.strictEqual(countFrames(expected), 1);
34+
35+
assert.deepStrictEqual(await compress((stream) => {
36+
stream.write('hello ');
37+
stream.write('world');
38+
}), expected);
39+
40+
assert.deepStrictEqual(await compress((stream) => {
41+
stream.write('hello world');
42+
stream.flush(zlib.constants.ZSTD_e_end);
43+
}), expected);
44+
45+
// The empty flush has to complete before end(), which would otherwise
46+
// promote it to ZSTD_e_end.
47+
assert.deepStrictEqual(await compress((stream) => {
48+
stream.write('hello world');
49+
stream.flush(zlib.constants.ZSTD_e_end);
50+
return new Promise((resolve) => stream.flush(resolve));
51+
}), expected);
52+
53+
const empty = await compress(() => {});
54+
assert.strictEqual(countFrames(empty), 1);
55+
assert.strictEqual(zlib.zstdDecompressSync(empty).length, 0);
56+
57+
const twoFrames = await compress((stream) => {
58+
stream.write('hello ');
59+
stream.flush(zlib.constants.ZSTD_e_end);
60+
stream.write('world');
61+
});
62+
assert.strictEqual(countFrames(twoFrames), 2);
63+
assert.strictEqual(zlib.zstdDecompressSync(twoFrames).toString(), 'hello world');
64+
})().then(common.mustCall());

0 commit comments

Comments
 (0)