This is an approach to address the issue linked below. Previously,
when `.write()` and `.flush()` calls to a zlib stream were interleaved
synchronously (i.e. without waiting for these operations to finish),
multiple flush calls would have been coalesced into a single flushing
operation.
This patch changes behaviour so that each `.flush()` all corresponds
to one flushing operation on the underlying zlib resource, and the
order of operations is as if the `.flush()` call were a `.write()`
call.
One test had to be removed because it specifically tested the previous
behaviour.
As a drive-by fix, this also makes sure that all flush callbacks are
called. Previously, that was not the case.
Fixes: https://github.com/nodejs/node/issues/28478
PR-URL: https://github.com/nodejs/node/pull/28520
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use the "no-restricted-globals" ESLint rule to lint for it.
PR-URL: https://github.com/nodejs/node/pull/27027
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When using `Errors.captureStackFrames` the error's stack property
is set again. This adds a helper function that wraps this functionality
in a simple API that does not only set the stack including the `code`
property but it also improves the performance to create the error.
The helper works for thrown errors and errors returned from wrapped
functions in case they are Node.js core errors.
PR-URL: https://github.com/nodejs/node/pull/26738
Fixes: https://github.com/nodejs/node/issues/26669
Fixes: https://github.com/nodejs/node/issues/20253
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This removes all internal calls to the deprecated `_extends()`
function. It is slower than `Object.assign()` and the object spread
notation since V8 6.8 and using the spread notation often also
results in shorter code.
PR-URL: https://github.com/nodejs/node/pull/25105
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Split the `Zlib` class into `ZlibBase` and `Zlib` classes,
to facilitate introduction of similar streams with minor
implementation differences.
PR-URL: https://github.com/nodejs/node/pull/24939
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Get a proper stack trace when no callback is passed.
PR-URL: https://github.com/nodejs/node/pull/24929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This adds the `capitalized-comments` eslint rule to verify that
actual sentences use capital letters as starting letters. It ignores
special words and all lines below 62 characters.
PR-URL: https://github.com/nodejs/node/pull/24808
Reviewed-By: Sam Ruby <rubys@intertwingly.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
We prefer for users to use `zlib.constants.XXX` instead of `zlib.XXX`.
Having both enumerable means that inspecting the `zlib` module
shows both variants, making the output significantly longer and
redundant.
PR-URL: https://github.com/nodejs/node/pull/24824
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This switches all `util.inherits()` calls to use
`Object.setPrototypeOf()` instead. In fact, `util.inherits()` is
mainly a small wrapper around exactly this function while adding
the `_super` property on the object as well.
Refs: #24395
PR-URL: https://github.com/nodejs/node/pull/24755
Refs: https://github.com/nodejs/node/issues/24395
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/23734
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This paves way for making `bytesRead` consistent with all other
Node.js streams that provide a property with this name.
PR-URL: https://github.com/nodejs/node/pull/23308
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This makes it easier to implement the lookup in a way that targets
error codes from a specific compression library, as a way towards
supporting multiple ones (e.g. brotli).
PR-URL: https://github.com/nodejs/node/pull/23413
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Previously, flushing on zlib streams was implemented through
stream 'drain' handlers. This has a number of downsides; in
particular, it is complex, and could lead to unpredictable
behaviour, since it meant that in a sequence like
```js
compressor.write('abc');
compressor.flush();
waitForMoreDataAsynchronously(() => {
compressor.write('def');
});
```
it was not fully deterministic whether the flush happens after
the second chunk is written or the first one.
This commit replaces this mechanism by one that piggy-backs
along the stream’s write queue, using a “special” `Buffer`
instance that signals that a flush is currently due.
PR-URL: https://github.com/nodejs/node/pull/23186
Reviewed-By: James M Snell <jasnell@gmail.com>
Use the same symbol that other `AsyncWrap` instances also use
for accessing the JS wrapper.
PR-URL: https://github.com/nodejs/node/pull/23189
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Give the callback a more specific name, explain what it does
and why it is necessary, and move it to a location much closer
to its use site.
PR-URL: https://github.com/nodejs/node/pull/23187
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
According to the real logic codes, it seems no matter whether 'nread >=
kMaxLength' or not. We always close the 'self' stream first. So we can
shorten it by merging them into one sample.
PR-URL: https://github.com/nodejs/node/pull/22802
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
When `checkRangesOrGetDefault` calls `checkFiniteNumber`, the latter
function only has two parameters, so `lower` and `upper` don't need to
be passed for validation.
PR-URL: https://github.com/nodejs/node/pull/22115
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/21792
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
From the zlib v1.2.11 manual:
> ZEXTERN int ZEXPORT inflateInit2 OF((z_streamp strm,
> int windowBits));
>
> ...
> windowBits can also be zero to request that inflate use the window
> size in the zlib header of the compressed stream.
The current validation of windowBits in zlib.js doesn't check for this
case.
PR-URL: https://github.com/nodejs/node/pull/19686
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The introduction of `.bytesRead` to zlib streams was unfortunate,
because other streams in Node.js core use the exact opposite naming
of `.bytesRead` and `.bytesWritten`.
While one could see how the original naming makes sense in
a `Transform` stream context, we should try to work towards more
consistent APIs in core for these things.
This introduces `zlib.bytesWritten` and documentation-only deprecates
`zlib.bytesRead`.
PR-URL: https://github.com/nodejs/node/pull/19414
Refs: https://github.com/nodejs/node/issues/8874
Refs: https://github.com/nodejs/node/pull/13088
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The old variants have been deprecated since b20af8088a.
Refs: https://github.com/nodejs/node/pull/18415
PR-URL: https://github.com/nodejs/node/pull/19602
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This refactors a couple tests to have upper case first characters
in comments and to use `input` instead of `i`.
It also adds a few TODOs and rewrites a few lines to use default
arguments and to prevent function recreation when unnecessary.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This makes a effort to make sure all of these errors will actually
also show the received input.
On top of that it refactors a few tests for better maintainability.
It will also change the returned type to always be a simple typeof
instead of special handling null.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This adds a rule that forbids the use of native Error constructors in
the `lib` directory. This is to encourage use of the `internal/errors`
mechanism. The rule is disabled for errors that are not created with
the `internal/errors` module but are still assigned an error code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
This improves error handling for streams in a few ways.
1. It ensures that no user defined methods (_read, _write, ...) are run
after .destroy has been called.
2. It introduces an explicit error to tell the user if they are write to
write, etc to the stream after it has been destroyed.
3. It makes streams always emit close as the last thing after they have
been destroyed
4. Changes the default _destroy to not gracefully end streams.
It also updates net, http2, zlib and fs to the new error handling.
PR-URL: https://github.com/nodejs/node/pull/18438
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This is a first batch of updates that touches non-underscored modules in
lib.
PR-URL: https://github.com/nodejs/node/pull/19034
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
- Use assert to check mode in the Zlib constructor since it should
only be passed by us.
- Introduce checkRangesOrGetDefault() and checkFiniteNumber()
to simplify type and range checking for numeric arguments
- Instead of `ERR_INVALID_OPT_VALUE`, throw `ERR_OUT_OF_RANGE` and
`ERR_INVALID_ARG_TYPE` with descriptions of the expected ranges
or types to make the errors more user-friendly.
- Add message tests for the changed errors
PR-URL: https://github.com/nodejs/node/pull/18675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Number.isNaN is now as fast as `val !== val`. Switch to the more
readable version. Also switch all `isNaN` to `Number.isNaN`.
PR-URL: https://github.com/nodejs/node/pull/18744
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Non-ASCII characters in /lib get compiled into the node binary,
and may bloat the binary size unnecessarily. A linter rule may
help prevent this.
PR-URL: https://github.com/nodejs/node/pull/18043
Fixes: https://github.com/nodejs/node/issues/11209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17057
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
add4b0ab8c introduced a regression from Node 8 to Node 9 by removing the
`ArrayBuffer.isView(chunk)` check in `Zlib.prototype._transform` without
properly forcing `opts.objectMode` to `false` in the constructor because
the change to `opts` occurs after `opts` has been passed to the
`Transform` constructor. This commit fixes the issue by moving the call
to `Transform` after the changes to `opts`.
The regression can be demonstrated by running
node -e 'require("zlib").Gunzip({objectMode: true}).write({})'
On Node 8 this correctly throws a `TypeError`:
events.js:183
throw er; // Unhandled 'error' event
^
TypeError: invalid input
at Gunzip._transform (zlib.js:367:15)
at Gunzip.Transform._read (_stream_transform.js:186:10)
at Gunzip.Transform._write (_stream_transform.js:174:12)
at doWrite (_stream_writable.js:387:12)
at writeOrBuffer (_stream_writable.js:373:5)
at Gunzip.Writable.write (_stream_writable.js:290:11)
at [eval]:1:44
at ContextifyScript.Script.runInThisContext (vm.js:50:33)
at Object.runInThisContext (vm.js:139:38)
at Object.<anonymous> ([eval]-wrapper:6:22)
On Node 9 this causes an assertion failure:
node[21732]: ../src/node_zlib.cc:179:static void node::{anonymous}::ZCtx::Write(const v8::FunctionCallbackInfo<v8::Value>&) [with bool async = true]: Assertion `Buffer::HasInstance(args[1])' failed.
1: node::Abort() [node]
2: node::Assert(char const* const (*) [4]) [node]
3: 0x1250916 [node]
4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]
5: 0xb7547c [node]
6: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
7: 0x20c44b8842fd
Aborted
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
PR-URL: https://github.com/nodejs/node/pull/16960
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
add4b0ab8c made the assumption that compressed data
would never lead to an empty decompressed stream.
Fix that by explicitly checking the number of read bytes.
PR-URL: https://github.com/nodejs/node/pull/17042
Fixes: https://github.com/nodejs/node/issues/17041
Refs: https://github.com/nodejs/node/pull/13322
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
On 4 April 2017, Node.js versions v4.8.2 and v6.10.2 were
released. These versions bumped the vendored zlib library from
v1.2.8 to v1.2.11 in response to what it describes as low-severity
CVEs. In zlib v1.2.9, a change was made that causes an error to be
raised when a raw deflate stream is initialised with windowBits set
to 8.
In zlib v1.2.9, 8 become an invalid value for this parameter, and Node's zlib
module will crash if you call this:
```
zlib.createDeflateRaw({windowBits: 8})
```
On some versions this crashes Node and you cannot recover from it, while on some
versions it throws an exception. The permessage-deflate library up to
version v0.1.5 does make such a call with no try/catch
This commit reverts to the original behavior of zlib by gracefully changed
windowBits: 8 to windowBits: 9 for raw deflate streams.
Original-PR-URL: https://github.com/nodejs-private/node-private/pull/95
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/16511
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
