Skip to content

Commit d74a694

Browse files
authored
fix(ext/node): verbose zlib error messages (#28831)
1 parent 9da231d commit d74a694

File tree

6 files changed

+103
-2
lines changed

6 files changed

+103
-2
lines changed

ext/node/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ deno_core::extension!(deno_node,
397397
ops::zlib::op_zlib_init,
398398
ops::zlib::op_zlib_reset,
399399
ops::zlib::op_zlib_crc32,
400+
ops::zlib::op_zlib_err_msg,
400401
ops::zlib::brotli::op_brotli_compress,
401402
ops::zlib::brotli::op_brotli_compress_async,
402403
ops::zlib::brotli::op_create_brotli_compress,

ext/node/ops/zlib/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,30 @@ pub fn op_zlib_close(#[cppgc] resource: &Zlib) -> Result<(), ZlibError> {
288288
Ok(())
289289
}
290290

291+
#[op2]
292+
#[string]
293+
pub fn op_zlib_err_msg(
294+
#[cppgc] resource: &Zlib,
295+
) -> Result<Option<String>, ZlibError> {
296+
let mut zlib = resource.inner.borrow_mut();
297+
let zlib = zlib.as_mut().ok_or(ZlibError::NotInitialized)?;
298+
299+
let msg = zlib.strm.msg;
300+
if msg.is_null() {
301+
return Ok(None);
302+
}
303+
304+
// SAFETY: `msg` is a valid pointer to a null-terminated string.
305+
let msg = unsafe {
306+
std::ffi::CStr::from_ptr(msg)
307+
.to_str()
308+
.map_err(|_| JsErrorBox::type_error("invalid error message"))?
309+
.to_string()
310+
};
311+
312+
Ok(Some(msg))
313+
}
314+
291315
#[allow(clippy::too_many_arguments)]
292316
#[op2(fast)]
293317
#[smi]

ext/node/polyfills/_zlib_binding.mjs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export const UNZIP = 7;
4646
import {
4747
op_zlib_close,
4848
op_zlib_close_if_pending,
49+
op_zlib_err_msg,
4950
op_zlib_init,
5051
op_zlib_new,
5152
op_zlib_reset,
@@ -57,6 +58,7 @@ const writeResult = new Uint32Array(2);
5758

5859
class Zlib {
5960
#handle;
61+
#dictionary;
6062

6163
constructor(mode) {
6264
this.#handle = op_zlib_new(mode);
@@ -104,7 +106,11 @@ class Zlib {
104106
// normal statuses, not fatal
105107
break;
106108
case Z_NEED_DICT:
107-
this.#error("Bad dictionary", err);
109+
if (this.#dictionary && this.#dictionary.length > 0) {
110+
this.#error("Bad dictionary", err);
111+
} else {
112+
this.#error("Missing dictionary", err);
113+
}
108114
return false;
109115
default:
110116
// something else.
@@ -160,6 +166,8 @@ class Zlib {
160166
dictionary ?? new Uint8Array(0),
161167
);
162168

169+
this.#dictionary = dictionary;
170+
163171
if (err != Z_OK) {
164172
this.#error("Failed to initialize zlib", err);
165173
}
@@ -177,6 +185,7 @@ class Zlib {
177185
}
178186

179187
#error(message, err) {
188+
message = op_zlib_err_msg(this.#handle) ?? message;
180189
this.onerror(message, err);
181190
op_zlib_close_if_pending(this.#handle);
182191
}

tests/node_compat/config.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,6 +1322,7 @@
13221322
"test-zlib-create-raw.js",
13231323
"test-zlib-deflate-raw-inherits.js",
13241324
"test-zlib-destroy-pipe.js",
1325+
"test-zlib-dictionary-fail.js",
13251326
"test-zlib-empty-buffer.js",
13261327
"test-zlib-flush-write-sync-interleaved.js",
13271328
"test-zlib-from-string.js",

tests/node_compat/runner/TODO.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2664,7 +2664,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
26642664
- [parallel/test-zlib-crc32.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-crc32.js)
26652665
- [parallel/test-zlib-deflate-constructors.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-deflate-constructors.js)
26662666
- [parallel/test-zlib-destroy.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-destroy.js)
2667-
- [parallel/test-zlib-dictionary-fail.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-dictionary-fail.js)
26682667
- [parallel/test-zlib-dictionary.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-dictionary.js)
26692668
- [parallel/test-zlib-failed-init.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-failed-init.js)
26702669
- [parallel/test-zlib-flush-drain-longblock.js](https://github.com/nodejs/node/tree/v23.9.0/test/parallel/test-zlib-flush-drain-longblock.js)
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// deno-fmt-ignore-file
2+
// deno-lint-ignore-file
3+
4+
// Copyright Joyent and Node contributors. All rights reserved. MIT license.
5+
// Taken from Node 23.9.0
6+
// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
7+
8+
// Copyright Joyent, Inc. and other Node contributors.
9+
//
10+
// Permission is hereby granted, free of charge, to any person obtaining a
11+
// copy of this software and associated documentation files (the
12+
// "Software"), to deal in the Software without restriction, including
13+
// without limitation the rights to use, copy, modify, merge, publish,
14+
// distribute, sublicense, and/or sell copies of the Software, and to permit
15+
// persons to whom the Software is furnished to do so, subject to the
16+
// following conditions:
17+
//
18+
// The above copyright notice and this permission notice shall be included
19+
// in all copies or substantial portions of the Software.
20+
//
21+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
22+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
23+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
24+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
25+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
26+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
27+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
28+
29+
'use strict';
30+
const common = require('../common');
31+
const assert = require('assert');
32+
const zlib = require('zlib');
33+
34+
// String "test" encoded with dictionary "dict".
35+
const input = Buffer.from([0x78, 0xBB, 0x04, 0x09, 0x01, 0xA5]);
36+
37+
{
38+
const stream = zlib.createInflate();
39+
40+
stream.on('error', common.mustCall(function(err) {
41+
assert.match(err.message, /Missing dictionary/);
42+
}));
43+
44+
stream.write(input);
45+
}
46+
47+
{
48+
const stream = zlib.createInflate({ dictionary: Buffer.from('fail') });
49+
50+
stream.on('error', common.mustCall(function(err) {
51+
assert.match(err.message, /Bad dictionary/);
52+
}));
53+
54+
stream.write(input);
55+
}
56+
57+
{
58+
const stream = zlib.createInflateRaw({ dictionary: Buffer.from('fail') });
59+
60+
stream.on('error', common.mustCall(function(err) {
61+
// It's not possible to separate invalid dict and invalid data when using
62+
// the raw format
63+
assert.match(err.message, /(invalid|Operation-Ending-Supplemental Code is 0x12)/);
64+
}));
65+
66+
stream.write(input);
67+
}

0 commit comments

Comments
 (0)