In readline.markdown, don't use strict mode reserved keyword "interface"
as a variable name.
This commit changes the name of one `readline.Interface` instance from
"interface" to "rl", as it is named in other places of the doc.
PR-URL: https://github.com/nodejs/node/pull/4900
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Docs have 108 instances of "writable" and only 1 "writeable" so
fix this one.
PR-URL: https://github.com/nodejs/node/pull/4954
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Constant declaration for `net` is omitted in `idle_server.js`
Add a constant declaration for constant `net`
PR-URL: https://github.com/nodejs/node/pull/3950
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Many variables in the buffer tests are redeclared. Change them so that
they are scoped appropriately.
PR-URL: https://github.com/nodejs/node/pull/4893
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
In openssl s_client on Windows, RAND_screen() is invoked to initialize
random state but it takes several seconds in each connection.
This added -no_rand_screen to openssl s_client on Windows to skip
RAND_screen() and gets a better performance in the unit test of
test-tls-server-verify.
Do not enable this except to use in the unit test.
Fixes: https://github.com/nodejs/node/issues/1461
PR-URL: https://github.com/nodejs/node/pull/1836
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
A handful of variable declarations in `lib/buffer.js` redeclare the same
variable in the same scope. This change removes each redeclaration by
switching to `const`, switching to `let`, or explicitly hoisting the
`var` declaration.
PR-URL: https://github.com/nodejs/node/pull/4886
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
It can happen that the server-side socket is destroyed before the
client-side socket has gracefully closed, thus causing a 'ECONNRESET'
error in this socket. To solve this, also close gracefully in the server
side too.
PR-URL: https://github.com/nodejs/node/pull/4888
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
node_crypto.cc attempts to handle the case where OCSP stapling APIs
aren't provided by using NODE__HAVE_TLSEXT_STATUS_CB. But the build
would actually fail in this case because of a couple of places that were
missing #ifdefs.
With this change the build works although, as expected,
test-tls-ocsp-callback.js will fail.
PR-URL: https://github.com/nodejs/node/pull/4914
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Also changes some `var`s to `const` as they never change.
PR-URL: https://github.com/nodejs/node/pull/4867
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
SSL_CIPHER objects are conceptually const in any case and this allows
STACK_OF(SSL_CIPHER) to return a const pointer, as is done in BoringSSL
and, perhaps, OpenSSL in the future.
PR-URL: https://github.com/nodejs/node/pull/4913
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Do not swallow error details when reporting UV_EPROTO asynchronously,
and when creating artificial errors.
Fix: #3692
PR-URL: https://github.com/nodejs/node/pull/4885
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Testing this wasn't really useful, besides Object.observe is going to be
deprecated.
Also this test fails with Chakra (#4765) for obvious reason.
PR-URL: https://github.com/nodejs/node/pull/4769
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
This commit replaces multiple usages of `function(){}` with ES2015
arrow functions in places it was forgotten earlier. The goal is to
make the docs more consistent since other functions were already
replaced with ES2015 arrows.
In addition, it fixes invalid syntax in modules.markdown to valid
syntax as well as remove `var self = this` pattern usages in the code
where they are now possible to avoid through arrow functions.
PR-URL: https://github.com/nodejs/node/pull/4832
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4244
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
`test-child-process-fork-net2.js` has a switch statement with 6 cases.
Each case uses `child.send()`, passing an object for the callback.
`child.send()` ignores the callback because it is not a function.
Removing the unused argument.
PR-URL: https://github.com/nodejs/node/pull/4870
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
Timer race results in some flakiness on slower devices in CI. Remove
unneeded setTimeout() and replace booleans with common.mustCall().
PR-URL: https://github.com/nodejs/node/pull/4793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`test-assert.js` redeclares a variable with `var`. This change converts
it to a `const` declaration and wraps it in a standalone block to scope
it to just the test that uses it.
PR-URL: https://github.com/nodejs/node/pull/4854
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
`lib/_tls_common.js` had instances of `for` loops that defined variables
with `var` such that they were re-declared in the same scope. This
change scopes those variables with `let` so that they are not
re-declared.
PR-URL: https://github.com/nodejs/node/pull/4853
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
1. The release team's names were not in the sorted order and
2. the old list of releasers' names were not in bold
This patch fixes both of these issues
PR-URL: https://github.com/nodejs/node/pull/4876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: https://github.com/nodejs/node/pull/4863
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
When running the tests if `NODE_TEST_DIR` is set to a device different
than the location of the test files (where this repo is checked out),
then the parallel/test-fs-link.js test will fail with
`EXDEV: cross-device link not permitted`. The code works fine (and is in
fact throwing an error as desired) but the test fails.
This commit first creates the "source" file in the same directory as the
"destination" (where the hardlink will be created).
PR-URL: https://github.com/nodejs/node/pull/4861
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
test-net-settimeout is unnecessarily complex. This change simplifies it.
PR-URL: https://github.com/nodejs/node/pull/4799
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Previously, port was assumed to be a number and would cause an abort in
cares_wrap. This change throws a TypeError if port is not a number
before we actually hit C++.
Fixes: https://github.com/nodejs/node/issues/4837
PR-URL: https://github.com/nodejs/node/pull/4839
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
1. correct code type in addons.markdown
2. add missed code type in crypto.markdown
PR-URL: https://github.com/nodejs/node/pull/4858
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
`lib/_stream_readable.js` contained three instances of `var`
declarations occurring twice in the same scope. Refactored to `const` or
`let` as appropriate.
PR-URL: https://github.com/nodejs/node/pull/4816
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the docs we typically check for errors and surface them. This
is IMO a good idea and good practice. This PR adds a check for
errors in three places in the `net` docs where it was missing.
PR-URL: https://github.com/nodejs/node/pull/4834
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>>
The description for `statusMessage` was accidentally moved under the
heading for `http.IncomingMessage.socket`. This commit puts it back
in the correct place.
PR-URL: https://github.com/nodejs/node/pull/4822
Fixes: https://github.com/nodejs/node/issues/4558
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Klauke <romaaan.git@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
- Hide the scrollbar on the TOC on all browsers. It was never the
intention for it to be visible with the scroll indication in place.
A wrapper element with 20px padding was added to accommodate for
hopefully all scrollbar widths as well as to avoid overflowing
content.
- Fixed the scroll indication gradient on Safari, which was caused by
the wrong from-color, which now matches the to-color.
- Fixed a issue in old IE where the TOC didn't render on the correct
position through setting `left: 0` and `top: 0` on it.
PR-URL: https://github.com/nodejs/node/pull/4748
Reviewed-By: James M Snell <jasnell@gmail.com>
In a few places the code was refactored to use `maybeCallback` which
always returns a function. Checking for `if (callback)` always returns
true anyway.
PR-URL: https://github.com/nodejs/node/pull/4795
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
This commit fixes an issue with the Node.js command prompt
on Windows where the PATH environment variable would not be
set correctly if the existing PATH contained an '&'.
Fixes: https://github.com/nodejs/node/issues/4802
PR-URL: https://github.com/nodejs/node/pull/4804
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4805
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/4806
Reviewed-By: Stephan Belanger <admin@stephenbelanger.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Call `obj.postSend` in error case of `process.send()`. The
`net.Socket`'s handle should not be leaked.
Note that there are two callbacks invoked on handles
when they are sent to the child process over IPC pipes.
These callbacks are specified by `handleConversion` object, and
during send two of them are invoked:
* `send`
* `postSend`
Now for `net.Socket` in particular, `postSend` performs clean up by
closing the actual uv handle. However this clean up will not happen in
one of the branches. This pull request aims to fix this.
PR-URL: https://github.com/nodejs/node/pull/4752
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-http-exit-delay has a flaky history. Examination of the bug it was
written to find suggests that the test should be removed.
* The test is trying to find a delay of up to 1 second, but the delay
may also be much smaller than that. So the test will not catch the bug
all the time. It is therefore flaky when the bug exists.
* Experience has shown that the test is flaky as well when the bug is
absent in the code because it can sometimes take slower devices (such as
the Raspberry Pi devices that we have in continuous integration) more
than the allowed one second to run. Increasing the timeout for those
devices will make the test pass but will also mean that the test isn't
really testing anything because the delay it is trying to catch was a
delay of up to one second.
I don't think this is an appropriate test to run once on CI. If there is
to be a test for the issue in question, it should be a benchmark test
that is run a large number of times. We don't really have such tests in
CI yet
I would argue that this test is actively a problem. It does not reliably
catch the issue it is supposed to catch, nor can it likely be made to do
so. (To do so would likely require converting it to a benchmarking test
as previously described. We don't run those in CI, at least not at this
time.)
Because this test will have both false positives and false negatives,
especially on the slower devices, it contributes to a culture of
dismissing failed tests. It does not reliably identify an issue nor does
it reliably pass on a working code base. This test should be removed.
Ref: https://github.com/nodejs/node/pull/4277
PR-URL: https://github.com/nodejs/node/pull/4786
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Parsers hold a reference to the socket associated with the request
through onParserExecute. This must be removed when the parser is
freed so that the socket can be garbage collected when destroyed.
Regression introduced in commit 59b91f1 ("http_parser: consume
StreamBase instance").
PR-URL: https://github.com/nodejs/node/pull/4773
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
* Exchange 20 millisecond timers for setImmediate().
* Do not attempt to unlink path that will have been guaranteed to be
removed by `common.refreshTmpDir()`
* Do not swallow errors thrown by failed creation of needed test
subdirectory. If that happens, we want to know about it.
* Use `common.isSunOS` in one place where it is applicable
PR-URL: https://github.com/nodejs/node/pull/4776
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Event emitters support symbols as event names. The process object
assumes that the event name is a string, and examines the first
three characters to check for signals. This causes an exception
if the event name is a symbol. This commit ensures that the
event name is a string before trying to slice() it.
PR-URL: https://github.com/nodejs/node/pull/4798
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
ChALkeR
Peter Lyons
Minwoo Jung
Rich Trott
Shigeki Ohtsu
Myles Borins
Santiago Gimeno
Kári Tristan Helgason
Adam Langley
Fedor Indutny
Vladimir Kurchatkin
Benjamin Gruenbaum
Sakthipriyan Vairamani
Bryan English
Dmitriy Lazarev
Jimb Esser
Myles Borins
Roman Reiss
Drew Folta
Evan Lucas
Alexander Makarenko
Jackson Tian
Jeff Harris
Brian White
Robert Jefe Lindstaedt
Glen Arrowsmith
Kat Marchán
Karl Skomski
Tom Atkinson
cjihrig