The test was sometimes timing out due to a race condition. In OS X,
events for `fs.watch()` might only start showing up after a delay. This
is a limitation of the operating system. To work around that, there was
a timer in the test that delayed the writing of the file by 100ms.
However, sometimes that was not enough, and so the event never fired,
and the test timed out.
Change the timer to an interval so that it fires repeatedly until it is
picked up. This change only affects OS X.
Fixes: https://github.com/nodejs/node/issues/8511
PR-URL: https://github.com/nodejs/node/pull/9303
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='
PR-URL: https://github.com/nodejs/node/pull/9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
59714cb7b3 introduced the
`util.inspect.custom` symbol, but it was exported as
`customInspectSymbol` by `internal/util.js` and referenced as
`inspectSymbol` by `buffer.js`.
PR-URL: https://github.com/nodejs/node/pull/9289
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, make lint is failing with the following error:
3:7 error 'common' is assigned a value but never used no-unused-vars
✖ 1 problem (1 error, 0 warnings)
PR-URL: https://github.com/nodejs/node/pull/9334
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Add a test for _writableState.needDrain.
PR-URL: https://github.com/nodejs/node/pull/8799
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Related: https://github.com/nodejs/node/issues/8686
* Adds process.versions.cldr, .tz, and .unicode
* Changes how process.versions.icu is loaded
* Lazy loads the process.versions.* values for these
* add an exception to util.js
to cause 'node -p process.versions' to still work
* update process.version docs
Fixes: https://github.com/nodejs/node/issues/9237
Allows Symbol to be converted to String so it can be included in the
error.
Fixes: https://github.com/nodejs/node/issues/9003
PR-URL: https://github.com/nodejs/node/pull/9021
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Instead of writing to the REPL’s input stream for the alignment
spaces in `.editor` mode, let `readline` handle the spaces
properly (echoing them using `_ttyWrite` and adding them to the
current line buffer).
Fixes: https://github.com/nodejs/node/issues/9189
PR-URL: https://github.com/nodejs/node/pull/9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In `.editor` mode, `repl.write()` would have crashed when the
`key` argument was not present, because the overwritten
`_ttyWrite` of REPLs doesn’t check for the absence of a second
argument like `readline.write()` does.
Since the docs indicate that the argument is optional, add
a check paralleling the one in `readline.write()`.
PR-URL: https://github.com/nodejs/node/pull/9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replaces function expressions with ES6 arrow functions as well as
improve the comparison operator to check if operands are of same types.
PR-URL: https://github.com/nodejs/node/pull/9239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Removed the errorTimer from test-http-set-timeout.js, as this timer is
not necessary to test the setTimeout functionality.
Also edited the console.log message on line 8 to log the correct
timeout duration. Changed var to const, and added common.mustCall() to
on timeout and on error callbacks.
Fixes: https://github.com/nodejs/node/issues/9256
PR-URL: https://github.com/nodejs/node/pull/9264
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
- Changed `assert.ok()` to `assert.strictEqual()`.
- Changed `var` to `const` where possible.
PR-URL: https://github.com/nodejs/node/pull/9231
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Add buffer.transcode(source, from, to) method. Primarily uses ICU
to transcode a buffer's content from one of Node.js' supported
encodings to another.
Originally part of a proposal to add a new unicode module. Decided
to refactor the approach towrds individual PRs without a new module.
Refs: https://github.com/nodejs/node/pull/8075
PR-URL: https://github.com/nodejs/node/pull/9038
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit adds coverage for the timeout option used by
child_process exec() and execFile().
PR-URL: https://github.com/nodejs/node/pull/9208
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Rather than the pseudo-wcwidth impl used currently, use the ICU
character properties database to calculate string width and
determine if a character is full width or not. This allows the
algorithm to correctly identify emoji's as full width, ensures
the algorithm will continue to fucntion properly as new unicode
codepoints are added, and it's faster.
This was originally part of a proposal to add a new unicode module,
but has been split out.
Refs: https://github.com/nodejs/node/pull/8075
PR-URL: https://github.com/nodejs/node/pull/9040
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
the WHATWG url parser relies on ICU's punycode implementation.
A handful of the standard tests fail when ICU is not present
because of the additional checks that are not implemented. For
now, skip the parse and setter tests if ICU is not present.
PR-URL: https://github.com/nodejs/node/pull/9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: https://github.com/nodejs/node/pull/9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.
PR-URL: https://github.com/nodejs/node/pull/9199
Fixes: https://github.com/nodejs/node/issues/7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Environment variables should be treated case-insensitive on Windows
platforms and case-sensitive on UNIX platforms.
This commit ensures this behavior persists.
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/9166
Fixes: https://github.com/nodejs/node/issues/9157
Commit 93a44d5 ("src: fix deferred events not working with -e") defers
evaluation of the script to the next tick.
A side effect of that change is that 'beforeExit' listeners run before
the actual script. 'beforeExit' is emitted when the event loop is
empty but process.nextTick() does not ref the event loop.
Fix that by using setImmediate(). Because it is implemented in terms
of a uv_check_t handle, it interacts with the event loop properly.
Fixes: https://github.com/nodejs/node/issues/8534
PR-URL: https://github.com/nodejs/node/pull/8821
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.
PR-URL: https://github.com/nodejs/node/pull/9184
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add a way through environment variables to set the --preserve-symlinks
flag. Any non-null value of NODE_PRESERVE_SYMLINKS will enable symlinks.
PR-URL: https://github.com/nodejs/node/pull/8749
Fixes: https://github.com/nodejs/node/issues/8509
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This test was failing on FreeBSD from time to time in the project CI.
The bug the test was written for would guarantee that the timer would
fire at least 100ms late, but the assertion was firing if it was more
than 50ms late.
This changes the assertion to fire when the timer is more than 100ms
late.
I ran a modified version of this test using 0.10.38 (which has the bug)
and 0.10.39 (which has the fix) to confirm that it still fails in the
buggy one and passes in the fixed one.
PR-URL: https://github.com/nodejs/node/pull/9198
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
`test-dgram-send-callback-buffer-length` was timing out (via the
200ms timeout in the code) on FreeBSD in CI. The 200ms timeout is
arbitrary and not necessary. Remove it.
PR-URL: https://github.com/nodejs/node/pull/9197
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
`common` is required twice in test-setproctitle.js. Remove one of the
instances.
Other refactoring:
* var -> const and let
* assert.equal -> assert.strictEqual
* assert.notEqual -> assert.notStrickEqual
* string concatenation -> template string
* use of assert.ifError() instead of asserting error is null
PR-URL: https://github.com/nodejs/node/pull/9169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Using the black magic of Symbol.toPrimitive the numeric value of
start/end can be changed when Uint32Value() is called once
Buffer::Fill() is entered. Allowing the CHECK() to be bypassed.
The bug report was only for "start", but the same can be done with
"end". Perform checks for both in node::Buffer::Fill() to make sure the
issue can't be triggered, even if process.binding is used directly.
Include tests for each case. Along with a check to make sure the last
time the value is accessed returns -1. This should be enough to make
sure Buffer::Fill() is receiving the correct value. Along with two tests
against process.binding directly.
Fixes: https://github.com/nodejs/node/issues/9149
PR-URL: https://github.com/nodejs/node/pull/9174
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <ranziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In most cases, named functions match the variable or property to which
they are being assigned. That also seems to be the practice in a series
of PRs currently being evaluated that name currently-anonymous
functions.
This change applies that rule to instances in the code base that don't
comply with that practice.
This will be enforceable with a lint rule once we upgrade to ESLint
3.8.0.
PR-URL: https://github.com/nodejs/node/pull/9113
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Since 2e568d9 there is a bug where unpiping a stream
from a readable stream that has `_readableState.pipesCount > 1`
will cause it to remove the first stream in the
`_.readableState.pipes` array no matter where in the list the
`dest` stream was.
PR-URL: https://github.com/nodejs/node/pull/9171
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
* Add documentation for `--openssl-conf=file`.
* Fix openssl.cnf loading and OpenSSL init ordering
* Fix FIPS tests so `OPENSSL_CONF` is not longer usable but
`--openssl-conf` is
PR-URL: https://github.com/nodejs/node-private/pull/82
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
As shown in https://github.com/nodejs/node/issues/9096, the offset and
end value of the `slice` call are coerced to numbers and then passed to
`FastBuffer`, which internally truncates the mantissa part if the number
is actually a floating point number. This actually affects the new
length of the slice calculation. For example,
> const original = Buffer.from('abcd');
undefined
> original.slice(original.length / 3).toString()
'bc'
This happens because, starting value of the slice is 4 / 3, which is
1.33 (approximately). Now, the length of the slice is calculated as
the difference between the actual length of the buffer and the starting
offset. So, it becomes 2.67 (4 - 1.33). Now, a new `FastBuffer` is
constructed, with the following values as parameters,
1. actual buffer object,
2. starting value, which is 1.33 and
3. the length 2.67.
The underlying C++ code truncates the numbers and they become 1 and 2.
That is why the result is just `bc`.
This patch makes sure that all the offsets are coerced to integers
before any calculations are done.
Fixes: https://github.com/nodejs/node/issues/9096
PR-URL: https://github.com/nodejs/node/pull/9101
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: https://github.com/nodejs/node/pull/9119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
The no-useless-escape rule in ESLint did not previously flag certain
unnecessary escaping in template strings. These will be flagged in
ESLint 3.8.0.
PR-URL: https://github.com/nodejs/node/pull/9112
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Fix a regression introduced in commit 2996b5c ("crypto: Allow GCM
ciphers to have a longer IV length") from April 2016 where a misplaced
parenthesis in a 'is ECB cipher?' check made it possible to use empty
IVs with non-ECB ciphers.
Also fix some exit bugs in test/parallel/test-crypto-authenticated.js
that were introduced in commit 4a40832 ("test: cleanup IIFE tests")
where removing the IFFEs made the test exit prematurely instead of just
skipping subtests.
PR-URL: https://github.com/nodejs/node/pull/9032
Refs: https://github.com/nodejs/node/pull/6376
Refs: https://github.com/nodejs/node/issues/9024
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
npm should run in a sandbox to avoid unwanted interactions. Without
this change, npm would read the userconfig file $HOME/.npmrc which may
contain configs that break this test.
Fixes: https://github.com/nodejs/node/issues/9074
PR-URL: https://github.com/nodejs/node/pull/9079
Reviewed-By: Jeremiah Senkpiel <Fishrock123@rocketmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2a4b068aca introduced a regression in where checking
`instanceof` would fail for `Writable` subclasses inside the
subclass constructor, i.e. before `Writable()` was called.
Also, calling `null instanceof Writable` or
`undefined instanceof Writable` would fail due to accessing the
`_writableState` property of the target object.
This fixes these problems.
PR-URL: https://github.com/nodejs/node/pull/9088
Ref: https://github.com/nodejs/node/pull/8834#issuecomment-253640692
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
This commit fixes a regression introduced in 0ed8839a27 that caused
additional queued immediate callbacks to be ignored if
`clearImmediate(immediate)` was called within the callback for
`immediate`.
PR-URL: https://github.com/nodejs/node/pull/9086
Fixes: https://github.com/nodejs/node/issues/9084
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
`end` MUST always be emitted **before** `close`. However, if a handle
will invoke `uv_close_cb` immediately, or in the same JS tick - `close`
may be emitted first.
PR-URL: https://github.com/nodejs/node/pull/9066
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
tick-processor-base.js is a module used by three other tests. It is not
a test fixture so move it out of the fixture directory. (One downside to
having it in the fixture directory is that fixture code is not currently
linted.)
It is possible that the code in tick-processor-base.js should be
integrated into common.js. This can potentially happen subsequently (and
might make a reasonable good first contribution for a new contributor).
PR-URL: https://github.com/nodejs/node/pull/9022
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* var to const
* add check that expected error is ENOENT
* indexOf() to includes()
PR-URL: https://github.com/nodejs/node/pull/8999
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Unsanitized paths containing line feed characters can be used for
header injection and request splitting so reject them with an exception.
There seems to be no reasonable use case for allowing control characters
(characters <= 31) while there are several scenarios where they can be
used to exploit software bugs so reject control characters altogether.
PR-URL: https://github.com/nodejs/node/pull/8923
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: not-an-aardvark <not-an-aardvark@users.noreply.github.com>
Implements WHATWG URL support. Example:
```
var u = new url.URL('http://example.org');
```
Currently passing all WHATWG url parsing tests and all but two of the
setter tests. The two setter tests are intentionally skipped for now
but will be revisited.
PR-URL: https://github.com/nodejs/node/pull/7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
This commit adds a test for the killSignal option to spawnSync(),
and the other sync child process functions by extension.
PR-URL: https://github.com/nodejs/node/pull/8960
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Make the internal `SyncWriteStream` a proper `stream.Writable`
subclass. This allows for quite a bit of simplification, since
`SyncWriteStream` predates the streams2/streams3 implementations.
Fixes: https://github.com/nodejs/node/issues/8828
PR-URL: https://github.com/nodejs/node/pull/8830
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Use `[Symbol.hasInstance]()` to return `true` when asking for
`new Duplex() instanceof Writable`.
PR-URL: https://github.com/nodejs/node/pull/8834
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Rich Trott
Gerges Beshay
Charmander
Daniel Bevenius
Italo A. Casas
Steven R. Loomis
jseagull
Anna Henningsen
Deverick
BethGriggs
Jeena Lee
James M Snell
cjihrig
Evan Lucas
Oliver Salzburg
Ben Noordhuis
Marc Udoff
Trevor Norris
Franziska Hinkelmann
Niels Nielsen
Rod Vagg
Sakthipriyan Vairamani (thefourtheye)
larissayvette
João Reis
Brian White
Fedor Indutny
Jenna Vuong