doc: handle backpressure when write() return false #11824
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original commit message: [debug] load correct stack slot for frame details. R=bmeurer@chromium.org BUG=v8:5071 Review URL: https://codereview.chromium.org/2045863002 . Cr-Commit-Position: refs/heads/master@{nodejs#36769} PR-URL: nodejs#10873 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: jasnell - James M Snell <jasnell@gmail.com> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
SecureContext::AddRootCerts only parses the root certificates once and keeps the result in root_cert_store, a global X509_STORE. This change addresses the following issues: 1. SecureContext::AddCACert would add certificates to whatever X509_STORE was being used, even if that happened to be root_cert_store. Thus adding a CA certificate to a SecureContext would also cause it to be included in unrelated SecureContexts. 2. AddCRL would crash if neither AddRootCerts nor AddCACert had been called first. 3. Calling AddCACert without calling AddRootCerts first, and with an input that didn't contain any certificates, would leak an X509_STORE. 4. AddCRL would add the CRL to whatever X509_STORE was being used. Thus, like AddCACert, unrelated SecureContext objects could be affected. The following, non-obvious behaviour remains: calling AddRootCerts doesn't /add/ them, rather it sets the CA certs to be the root set and overrides any previous CA certificates. Points 1–3 are probably unimportant because the SecureContext is typically configured by `createSecureContext` in `lib/_tls_common.js`. This function either calls AddCACert or AddRootCerts and only calls AddCRL after setting up CA certificates. Point four could still apply in the unlikely case that someone configures a CRL without explicitly configuring the CAs. PR-URL: nodejs#9409 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Fix leaking the BIO in the error path. Introduced in commit 34febfb ("crypto: fix handling of root_cert_store"). PR-URL: nodejs#9604 Refs: nodejs#9409 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rod Vagg <rod@vagg.org>
This makes sure that we dump a backtrace and use raise(SIGABRT) on Windows. PR-URL: nodejs#9613 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Use of abort() was added in 34febfb, and changed to ABORT() in 21826ef, but conditional+ABORT() is better expressesed using a CHECK_xxx() macro. See: nodejs#9409 (comment) PR-URL: nodejs#10413 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This test is newly added to v4.x stream and is consistently failing. We have a couple of issues with pseudo-tty tests in AIX, and while the investigation is going on, need to skip this test to make CI green. PR-URL: nodejs#11602 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Original Commit Message: constants.ENGINE_METHOD_RSA was documented, but not implemented. PR-URL: nodejs#5463 Reviewed-By: Fedor Indutny <fedor@indutny.com> Backport-Of: nodejs#5463 PR-URL: nodejs#11002 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Original Commit Message: ENGINE_METHOD_PKEY_METH and ENGINE_METHOD_PKEY_ASN1_METH are misspelled in the documentation, both should be ..._METHS. PR-URL: nodejs#5463 Reviewed-By: Fedor Indutny <fedor@indutny.com> Backport-Of: nodejs#5463 PR-URL: nodejs#11002 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@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: nodejs#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>
PR-URL: nodejs#9649 Reviewed-By: Trevor Norris <trev.norris@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> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
The node -> out/*/node symlink is getting recreated in parallel with other targets in the makefile which require it (e.g. test-ci) and this seems to be causing a race condition which is showing up on AIX Fixes: nodejs#9825 PR-URL: nodejs#9827 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Michael Dawson <michael_dawson@ca.ibm.com>
Currently when running make node_g the following error is displayed: if [ ! -r node -o ! -L ]; then ln -fs out/Debug/node node_g; fi /bin/sh: line 0: [: argument expected It looks like there was a typo for the NODE_EXE where node became lowercase instead of uppercase. Ref: nodejs#9827 PR-URL: nodejs#10153 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>
Provide details for fields of rinfo object of UDP message event. PR-URL: nodejs#10050 Reviewed-By: James M Snell <jasnell@gmail.com>
The COLLABORATOR_GUIDE was still listing v0.10 and v0.12 as LTS when they are EOL now. PR-URL: nodejs#10720 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
git apply does not preserve the original commit message. These updated instructions offer a simpler flow for backporting. PR-URL: nodejs#10665 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michal Zasso <targos@protonmail.com>
The guide for writing tests is missing information on how tests are named. This adds that information. There is also some copy-editing done on the first paragraph of the guide. PR-URL: nodejs#10584 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: nodejs#10616 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Add links to the engine classes for the zlib single-call convenience methods. PR-URL: nodejs#10829 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit clarifies variables in the Filesystem docs. Prior, the documentation for fs.write() had an ambiguous remark on the parameters of offset and length. Therefore, this commit makes explicit that the length parameter in fs.write() is used to denote the number of bytes, which is a clearer reference for its usage. PR-URL: nodejs#9792 Ref: nodejs#7868 Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
PR-URL: nodejs#10883 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: nodejs#10954 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Changed authors listing from `Noah Rose` to `Noah Rose Ledesma`. PR-URL: nodejs#10945 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* use common.mustCall() where appropriate * Buffer.allocUnsafe() -> Buffer.alloc() * do crypto check before loading any additional modules * specify 1ms duration for `setTimeout()` PR-URL: nodejs#10225 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
We were transporting the heap statistics as uint32 values to JS land but those wrap around for values > 4 GB. Use 64 bits floats instead, those should last us a while. Fixes: nodejs#10185 PR-URL: nodejs#10186 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
new year new alias PR-URL: nodejs#10586 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Remove the numbers from the comments to make it clear that assert does not follow the [CJS spec](http://wiki.commonjs.org/wiki/Unit_Testing/1.0). Additionally, clean up the existing comments for consistent formatting/language and ease of reading. PR-URL: nodejs#10579 Fixes: nodejs#9063 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs#10446 Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes: nodejs/CTC#41 PR-URL: nodejs#10604 Fixes: https://github.com/nodejs/CTC#41 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Michal Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* Change === to == in one place * Add explanation about another non-strict if-statement PR-URL: nodejs#11513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Test that an error is thrown when bind() is called on an already bound socket. PR-URL: nodejs#10894 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
* use common.mustCall to validate functions executions * use common.fail to check test fail * remove console.log * use arrow functions PR-URL: nodejs#10845 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes coverity scan issue 55489. PR-URL: nodejs#10891 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: nodejs#10911 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Adds Google Analytics tracking script to all doc pages when `DOCS_ANALYTICS` is set when running `make`: ```bash $ DOCS_ANALYTICS=<GOOGLE ANALYTICS ID> make ``` By default (when `DOCS_ANALYTICS` is not set), no tracking scripts are included. It respects "Do Not Track" settings end users might have in their browser. Also changes make target `doc-upload` from depending on the `$(TARBALL)` target, to only depend on `doc` directly. PR-URL: nodejs#6601 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
In test-timers, confirm that all input values that should be coerced to 1 ms are not being coerced to a significantly larger value. This eliminates the need for the separate test-regress-nodejsGH-897. PR-URL: nodejs#10960 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Adds a benchmark to compare the speed of property setting/getting in four cases: - Dot notation: `obj.prop = value` - Bracket notation with string: `obj['prop'] = value` - Bracket notation with string variable: `obj[prop] = value` - Bracket notation with Symbol variable: `obj[sym] = value` PR-URL: nodejs#10949 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* test reading a file passing a not valid enconding Refs: https://coverage.nodejs.org/coverage-067be658f966dafe/root/internal/fs.js.html PR-URL: nodejs#10947 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Refs: nodejs#9493 PR-URL: nodejs#10925 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
test-child-process-spawnsync-timeout failed from time to time on Raspberry Pi devices. Use common.platformTimeout() to allow a little more time to run on resource-constrained hosts. PR-URL: nodejs#10998 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Make use of Arrow function. Add a small test and this file's coverage is 100%. https://github.com/nodejs/node/blob/a647d82f83ad5ddad5db7be2cc24c3d686121792/lib/_stream_duplex.js#L25 Coverage: https://coverage.nodejs.org/coverage-067be658f966dafe/root/_stream_duplex.js.html PR-URL: nodejs#10963 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The last release Chris did was v1.8.1 from April 2015. PR-URL: nodejs#11011 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
PR-URL: nodejs#11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#11277 Reviewed-By: James M Snell <jasnell@gmail.com>
Because any call to util.inspect() with an object results in inspectPromise() being called, Debug was being initialized even when it's not needed. Instead, the initialization is placed after the isPromise check. PR-URL: nodejs#8452 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
This commit adds a guard against an out of bounds access of arguments, and replaces another use of arguments with a named function parameter. Refs: nodejs#10323
`request.abort()` did not destroy the socket if it was called before a socket was assigned to the request and the request did not use an `Agent` or a Unix Domain Socket was used. Fixes: nodejs#10812 PR-URL: nodejs#10818 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The doc specified that writable.write() was advisory only. However, ignoring that value might lead to memory leaks. This PR specifies that behavior. Moreover, it adds an example on how to listen for the 'drain' event correctly. See: nodejs@f347dad PR-URL: nodejs#10631 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs-github-bot
added
doc
2 tasks
MylesBorins
force-pushed
the
v4.x-staging
branch
2 times, most recently
from
004f6b0
to
dcbc1b4
Compare
|
@mcollina ... can you rebase this on the updated v4.x-staging? @nodejs/lts ... given that v4 is heading into maintenance mode, does it make sense to land this? |
|
I'm actually fine in not backporting it, as v4 is heading in maintenance mode. |
|
Ok, let's close the PR then. We can revisit it again later if necessary. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport e9044c8 to v4.
cc @jasnell @MylesBorins
Original commit message
The doc specified that writable.write() was advisory only. However,
ignoring that value might lead to memory leaks. This PR specifies that
behavior. Moreover, it adds an example on how to listen for the 'drain'
event correctly.
See: f347dad
PR-URL: #10631
Reviewed-By: Colin Ihrig cjihrig@gmail.com
Reviewed-By: Sam Roberts vieuxtech@gmail.com
Reviewed-By: Evan Lucas evanlucas@me.com
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Joyee Cheung joyeec9h3@gmail.com