Most Windows systems do not have an external `echo` program installed,
so any attempts to spawn `echo` as a child process will fail with
`ENOENT`. This commit forces the use of the built-in `echo` provided
by `cmd.exe`.
PR-URL: https://github.com/nodejs/node/pull/7049
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fix issues with disabled test-debug-brk-no-arg and re-enable the test.
PR-URL: https://github.com/nodejs/node/pull/7143
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Updating tests to use `common.fixturesDir` whenever possible/reasonable.
Left out things like tests for `path` and `require.resolve`.
PR-URL: https://github.com/nodejs/node/pull/6997
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
When `free`ing the socket to be reused in keep-alive Agent wait for
both `prefinish` and `end` events. Otherwise the next request may be
written before the previous one has finished sending the body, leading
to a parser errors.
PR-URL: https://github.com/nodejs/node/pull/7149
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Illegal tokens are only recoverable in string literals, RegExp literals,
and block comments. If not in one of these constructs, immediately
return an error rather than giving the user false hope by giving them a
chance to try to recover.
PR-URL: https://github.com/nodejs/node/pull/7104
Fixes: https://github.com/nodejs/node/issues/3611
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
All the callback functions in `fs` module are supposed to be executed
with no context (`this` value should not be a valid object). But
`mkdtemp`'s callback will have the `FSReqWrap` object as the context.
Sample code to reproduce the problem
'use strict';
const fs = require('fs');
fs.mkdtemp('/tmp/abcd', null, function() {
console.log(this);
});
This would print
FSReqWrap { oncomplete: [Function] }
But that should have printed `null` and this patch fixes that.
PR-URL: https://github.com/nodejs/node/pull/7068
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The command line flag `--debug-brk` was ignored when the `-e` flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.
PR-URL: https://github.com/nodejs/node/pull/7089
Fixes: https://github.com/nodejs/node/issues/3589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove a disabled test in favor of one that expects an error.
This validates (somewhat) that the underlying code is calling the
correct system call for setting UID and GID. Unlike the formerly
disabled test, it does not try to validate that the system UID/GID
setting works.
PR-URL: https://github.com/nodejs/node/pull/7084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Test failed or zero-sized Buffer allocations not affecting subsequent
creations of typed arrays.
PR-URL: https://github.com/nodejs/node/pull/7093
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Said function requires that a v8::Context has been entered first,
introducing a chicken-and-egg problem when creating the first context.
PR-URL: https://github.com/nodejs/node/pull/7082
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
test-child-process-fork-net will sometimes fail in CI with EADDRINUSE
because an earlier test failed to free common.PORT. Have the operating
system provide an available port instead.
PR-URL: https://github.com/nodejs/node/pull/7033
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
test-https-strict sometimes fails with EADDRINUSE in CI. Remove use of
common.PORT to make the test resistant from side effects from other
tests that may have not freed up the port.
PR-URL: https://github.com/nodejs/node/pull/7024
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.
Fixes: https://github.com/nodejs/node/issues/7047
PR-URL: https://github.com/nodejs/node/pull/7051
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
There are several places in the cluster module where a version
of process.send() is called, but the result is swallowed. Most
of these cases are internal, but Worker.prototype.send(), which
is publicly documented, also suffers from this problem. This
commit exposes the return value to facilitate better error
handling, and bring Worker.prototype.send() into compliance
with the documentation.
PR-URL: https://github.com/nodejs/node/pull/6998
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: https://github.com/nodejs/node/pull/7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
According to kill(2), kill returns `EPERM` error if when signalling a
process group any of the members could not be signalled.
PR-URL: https://github.com/nodejs/node/pull/7046
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
A possibly-buggy fixture server uses `common.PORT+1000` for its port
rather than `common.PORT`. That could result in it clashing with other
ports if tests are run in parallel. The test runner increments
`common.PORT` by 100 for each running instance for tests. Change to use
common.PORT and have the tests that use the fixture start with
common.PORT+1 for anything they need.
PR-URL: https://github.com/nodejs/node/pull/6990
Refs: https://github.com/nodejs/node/issues/6989
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit provides a rewrite of StringDecoder that both improves
performance (for non-single-byte encodings) and understandability.
Additionally, StringDecoder instantiation performance has increased
considerably due to inlinability and more efficient encoding name
checking.
PR-URL: https://github.com/nodejs/node/pull/6777
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Update parallel/test-http-agent-getname to use assert.strictEqual()
consistently and const-ify variables while we're here.
PR-URL: https://github.com/nodejs/node/pull/6654
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
On UNIX platforms, the debugger doesn't reliably kill the inferior when
killed by a signal. Work around that by spawning the debugger in its
own process group and killing the process group instead of just the
debugger process.
This is a hack to get the continuous integration back to green, it
doesn't address the underlying issue, which is that the debugger
shouldn't leave stray processes behind.
Fixes: https://github.com/nodejs/node/issues/7034
PR-URL: https://github.com/nodejs/node/pull/7037
Refs: https://github.com/nodejs/node/pull/3470
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It allows reopening a server after it has been closed.
Fixes: https://github.com/nodejs/node/issues/6693
PR-URL: https://github.com/nodejs/node/pull/6981
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-cluster-disconnect-handles already includes logic that
tries to cleanup any child processes when the test fails. This
commit adds additional checks to verify that the child exited
normally, and fails the test if that is not the case.
Refs: https://github.com/nodejs/node/issues/6988
PR-URL: https://github.com/nodejs/node/pull/6993
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This commit fix a possible crash situation in dgram send().
A crash is possible if an array is passed, and then altered after the
send call, as the call to libuv is wrapped in process.nextTick().
It also avoid sending an empty array to libuv by allocating an empty
buffer. It also does some cleanup inside send() to increase readability.
It removes test flakyness by use common.mustCall and
common.platformTimeout. Fixes situations were some events were not
asserted to be emitted.
Fixes: https://github.com/nodejs/node/issues/6616
PR-URL: https://github.com/nodejs/node/pull/6804
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
A Proxy context should not hide built-in global objects.
Ref: https://github.com/nodejs/node/issues/6158
PR-URL: https://github.com/nodejs/node/pull/6967
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Before this commit `node --debug-port=1234 debug t.js` ignored the
--debug-port= argument, binding to the default port 5858 instead,
making it impossible to debug more than one process on the same
machine that way.
This commit also reduces the number of places where the default port
is hard-coded by one.
Fixes: https://github.com/nodejs/node/issues/3345
PR-URL: https://github.com/nodejs/node/pull/3470
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
cluster's internal message handling includes a cache of callback
functions. Once the message for that callback is received, it is
removed from the cache. If, for any reason, the same message ID
is processed twice, the callback will be missing from the cache
and cluster will try to call undefined as a function. This commit
guards against this scenario.
Refs: https://github.com/nodejs/node/issues/6561
PR-URL: https://github.com/nodejs/node/pull/6902
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
v8::Object::GetAlignedPointerFromInternalField() returns a random value
if Wrap() hasn't been run on the object handle. Causing v8 to abort if
certain getters are accessed. It's possible to access these getters and
functions during class construction through the AsyncWrap init()
callback, and also possible in a subset of those scenarios while running
the persistent handle visitor.
Mitigate this issue by manually setting the internal aligned pointer
field to nullptr in the BaseObject constructor and add necessary logic
to return appropriate values when nullptr is encountered.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Though the TLSWrap constructor is only called via TLSWrap::Wrap() (i.e.
tls_wrap.wrap()) internally, it is still exposed to JS. Don't allow the
application to abort by inspecting the instance before it has been
wrap'd by another handle.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This test checks that ownerless cluster worker handles are closed
correctly on disconnection.
Fixes: https://github.com/nodejs/node/issues/6561
PR-URL: https://github.com/nodejs/node/pull/6909
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The test in this commit runs correctly if IPC messages are
properly consumed and emitted. Otherwise, the test times out.
Fixes: https://github.com/nodejs/node/issues/6561
PR-URL: https://github.com/nodejs/node/pull/6909
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit refactors test-child-process-spawnsync.js to use
the reusable common.spawnSyncPwd().
PR-URL: https://github.com/nodejs/node/pull/6939
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
When the 'buffer' encoding is passed to spawnSync(), an exception
is thrown in Buffer's toString() method because 'buffer' is not
a valid encoding there. This commit special cases the 'buffer'
encoding.
Fixes: https://github.com/nodejs/node/issues/6930
PR-URL: https://github.com/nodejs/node/pull/6939
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
As it is, `fs.mkdtemp` crashes with a C++ assertion if the callback
function is not passed. This patch uses `maybeCallback` to create one,
if no callback function is passed.
PR-URL: https://github.com/nodejs/node/pull/6828
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
In preparation for a lint rule to flag `__defineGetter__`, refactor the
one remaining instance in the code base.
PR-URL: https://github.com/nodejs/node/pull/6774
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The only tests for `addMembership()` and `dropMembership()` (from the
`dgram` module) were in `test/internet` which means they almost never
get run. This adds checks in `test/parallel`.
PR-URL: https://github.com/nodejs/node/pull/6753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The test is currently flaky and CI provides no real information because
the test times out rather than failing on an assertion. Add logging to
gather more information about the failure.
Refs: https://github.com/nodejs/node/issues/6754
PR-URL: https://github.com/nodejs/node/pull/6769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Make sure that `catch-stdout-error` has written data before the
destination process exits.
Fixes: https://github.com/nodejs/node/issues/6791
PR-URL: https://github.com/nodejs/node/pull/6808
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
The TypeError checks are already done later on in the test file
for all path functions.
PR-URL: https://github.com/nodejs/node/pull/6590
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Currently we are overwriting the entire env object of the child-process
spawned in `npm-test-install`. This commit alternatively clones the
`process.env` object and modifies it with the neccessary changes before
passing it the the spawned process.
Fixes: https://github.com/nodejs/node/issues/6736
PR-URL: https://github.com/nodejs/node/pull/6797
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously a `checks.workers` boolean was conditionally set, but never
checked. Additionally, it was never actually set because
`cluster.onlineWorkers` is always undefined.
PR-URL: https://github.com/nodejs/node/pull/6535
Reviewed-By: Anna Henningsen <anna@addaleax.net>