Move the throw out of c++ and into js using internal/errors
PR-URL: https://github.com/nodejs/node/pull/16546
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
When `throw undefined` or `throw null` is executed, the REPL crashes.
This change does a check for `null|undefined` before accessing an
error's properties to prevent crashing.
Fixes: https://github.com/nodejs/node/issues/16545
Fixes: https://github.com/nodejs/node/issues/16607
PR-URL: https://github.com/nodejs/node/pull/16574
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
The original commit was landed without running CITGM. Unfortunately
this change breaks the module `d` which has over 500k downloads a day.
It is worth mentioning that the compatibility hack can be removed
without breaking anything.
We should definitely revisit for the next Semver-Major but shipping
this today will cause non trivial ecosystem breakages.
Refs: https://github.com/nodejs/node/pull/3384
PR-URL: https://github.com/nodejs/node/pull/16634
Refs: https://github.com/nodejs/node/pull/3384
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Migrates most of CipherBase errors to use internal/errors.
There are still a handful remaining that need to be handled
separately
PR-URL: https://github.com/nodejs/node/pull/16527
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
_read should always resume the underlying code that is attempting
to push data to a readable stream. Adjust http2 core code to
resume its reading appropriately.
Some other general cleanup around reading, resuming & draining.
PR-URL: https://github.com/nodejs/node/pull/16580
Fixes: https://github.com/nodejs/node/issues/16578
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@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>
Initializing IOCP on the same fd twice can fail on Windows.
Consequently, if the IPC channel uses fd 1 or 2 and the console is setup
first, writing to the IPC channel will fail.
PR-URL: https://github.com/nodejs/node/pull/16562
Fixes: https://github.com/nodejs/node/issues/16141
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Complete the migration to the new error system of _stream_readable
and _stream_writable. Adds the corresponding documentation.
PR-URL: https://github.com/nodejs/node/pull/16589
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
No longer check whether key is a symbol as Object.keys does not
return symbols. No longer convert key to string as it is always
a string. Validate that only one value is passed for each
pseudo-header.
Extend illegal connection header message to include the name of
the problematic header.
Extend tests to cover this behaviour.
PR-URL: https://github.com/nodejs/node/pull/16575
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* correctly reset write timers: currently reset timers on
both session & stream when write starts and when it ends.
* prevent large writes from timing out: when writing a large
chunk of data in http2, once the data is handed off to C++,
the JS session & stream lose all track of the write and will
timeout if the write doesn't complete within the timeout window
Fix this issue by tracking whether a write request is ongoing and
also tracking how many chunks have been sent since the most recent
write started. (Since each write call resets the timer.)
PR-URL: https://github.com/nodejs/node/pull/16525
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
With this change, we do async stack tracking only when explicitly
requested by the inspector client. This avoids unnecessary overhead
for clients that might not be interested in async stack traces.
PR-URL: https://github.com/nodejs/node/pull/16308
Fixes: https://github.com/nodejs/node/issues/16180
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/16261
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
The reason is that absolute URLs do not go through extension and index
checks. By switching to an absolute path, the resolver still applies
extensions properly to the top-level main.
PR-URL: https://github.com/nodejs/node/pull/16526
Fixes: https://github.com/nodejs/node/issues/16476
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
It can be useful to load dependencies as part of the loader hook
definition file. This fixes a bug where `import x from 'x'` would
always return `x` as `undefined` if the import was made in a loader
hooks definition module.
A parallel change to the CJS loading injection process meant that the
CJS module wasn't being injected into the correct loader instance,
which is corrected here with a test.
PR-URL: https://github.com/nodejs/node/pull/16381
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
With the exception of ThrowCryptoError, use internal/errors
to report fips unavailable or forced
PR-URL: https://github.com/nodejs/node/pull/16428
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Rather than using `http`, use `_http_client`, etc. directly.
Also moving all the exports to the bottom, in line with most of the rest
of the codebase.
PR-URL: https://github.com/nodejs/node/pull/16395
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is for issue 16452. When 'set-cookie' header is set with an array
that has only one string value, it's split into its individual
characters.
Fix by resetting `isArray` to false when the value is converted from an
array to a string.
Fixes: https://github.com/nodejs/node/issues/16452
PR-URL: https://github.com/nodejs/node/pull/16458
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Provide `util.isDeepStrictEqual()` that works like
`assert.deepStrictEqual()` but returns a boolean rather than throwing an
error.
Several userland modules have needed this functionality and implemented
it independently. This functionality already exists in Node.js core, so
this exposes it for use by modules. Modules that have needed this
functionality include `lodash`, `concordance` (used by `ava`), and
`qunit`.
PR-URL: https://github.com/nodejs/node/pull/16084
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Stashing it away in internal/buffer so that it can't be used in
userland, but can still be used in internals.
PR-URL: https://github.com/nodejs/node/pull/16391
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Improve argument type checking and move into js, use internal/errors
PR-URL: https://github.com/nodejs/node/pull/15757
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Move input argument validation to js, using internal/errors.
Also update docs
* `password` and `salt` may be Buffers or any TypedArrays
* `crypto.DEFAULT_ENCODING` changes the returned derivedKey type
PR-URL: https://github.com/nodejs/node/pull/15746
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* move CHECK statements into DEBUG checks
* improve performance by removing branches
* Several if checks were left in while the code was being developed.
Now that the core API has stablized more, the checks are largely
unnecessary and can be removed, yielding a significant boost in
performance.
* refactor flow control for proper backpressure
* use std::queue for inbound headers
* use std::queue for outbound data
* remove now unnecessary FreeHeaders function
* expand comments and miscellaneous edits
* add a couple of misbehaving flow control tests
PR-URL: https://github.com/nodejs/node/pull/16239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Due to how WHATWG-URL parser works, port numbers are omitted if
they are the default port for a scheme. This meant that
http2.connect could not accept connections on port 80 with http
scheme. Fix this bug by detecting http: scheme and setting port
to 80.
PR-URL: https://github.com/nodejs/node/pull/16337
Fixes: https://github.com/nodejs/node/issues/14304
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: https://github.com/nodejs/node/pull/12815
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Support generic `Duplex` streams through more duck typing
on the server and client sides.
Since HTTP is, as a protocol, independent of its underlying transport
layer, Node.js should not enforce any restrictions on what streams
its HTTP parser may use.
Ref: https://github.com/nodejs/node/issues/16256
PR-URL: https://github.com/nodejs/node/pull/16267
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Support generic `Duplex` streams through using `StreamWrap`
on the server and client sides, and adding a `createConnection`
method option similar to what the HTTP/1 API provides.
Since HTTP2 is, as a protocol, independent of its underlying transport
layer, Node.js should not enforce any restrictions on what streams
its internals may use.
Ref: https://github.com/nodejs/node/issues/16256
PR-URL: https://github.com/nodejs/node/pull/16269
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`buffer.transcode` is still using raw TypeError. This change is to
convert it to use internal/errors.
Ref: https://github.com/nodejs/node/issues/11273
PR-URL: https://github.com/nodejs/node/pull/16352
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
In PR [1], a bunch of properties were removed from the error thrown by
execSync and execFileSync. It turns out that some of those were still
supposed to be there, as the documentation states that the error
contains the entire result from the spawnSync call.
[1] https://github.com/nodejs/node/pull/13601
PR-URL: https://github.com/nodejs/node/pull/16060
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was originally changed in 6f75b6672c
but it appears unnecessary and benhcmark results show little difference
without the extra property.
Refs: https://github.com/nodejs/node/pull/6436
PR-URL: https://github.com/nodejs/node/pull/16355
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Previously, various inputs other than non-negative integers would
produce incorrect results.
Added type-checking on input, returning false for anything other than
non-negative integers.
Also clarified in docs.
PR-URL: https://github.com/nodejs/node/pull/15567
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
A few small changes:
- fix debug statement location
- simplify conditional with returns
- consistent use of template strings
PR-URL: https://github.com/nodejs/node/pull/16327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
options.getTrailers & options.selectPadding will always be a
function by the time the code reaches the assert checks. And
even if not (which seems currently impossible), the assert
failure will be almost the same as calling something that is
not a function. The downside of leaving these is that typeof
checks are decently expensive.
PR-URL: https://github.com/nodejs/node/pull/16327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Minimize property access via Symbols by consistently creating
new constants for things that are retrieved multiple times
within a method or a function. Fix cases where existing
constants aren't used where appropriate.
PR-URL: https://github.com/nodejs/node/pull/16327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This simplifies the top-level load when ES modules are enabled
as we can entirely delegate the module resolver, which will hand
over to CommonJS where appropriate.
All not found errors are made consistent to throw during resolve
and have the MODULE_NOT_FOUND code.
Includes the test case from https://github.com/nodejs/node/pull/15736.
Fixes: https://github.com/nodejs/node/issues/15732
PR-URL: https://github.com/nodejs/node/pull/16147
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
