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. [email protected] 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 <[email protected]> Reviewed-By: jasnell - James M Snell <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
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 <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
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 <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]>
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
This makes sure that we dump a backtrace and use raise(SIGABRT) on Windows. PR-URL: nodejs#9613 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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 <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Original Commit Message: constants.ENGINE_METHOD_RSA was documented, but not implemented. PR-URL: nodejs#5463 Reviewed-By: Fedor Indutny <[email protected]> Backport-Of: nodejs#5463 PR-URL: nodejs#11002 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Backport-Of: nodejs#5463 PR-URL: nodejs#11002 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: not-an-aardvark <[email protected]>
PR-URL: nodejs#9649 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-by: Michael Dawson <[email protected]>
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]>
Provide details for fields of rinfo object of UDP message event. PR-URL: nodejs#10050 Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Michal Zasso <[email protected]>
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 <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
PR-URL: nodejs#10616 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Prince John Wesley <[email protected]>
Add links to the engine classes for the zlib single-call convenience methods. PR-URL: nodejs#10829 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]>
PR-URL: nodejs#10883 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
PR-URL: nodejs#10954 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Changed authors listing from `Noah Rose` to `Noah Rose Ledesma`. PR-URL: nodejs#10945 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
* 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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
new year new alias PR-URL: nodejs#10586 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#10446 Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs/CTC#41 PR-URL: nodejs#10604 Fixes: https://github.com/nodejs/CTC#41 Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Michal Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
* Change === to == in one place * Add explanation about another non-strict if-statement PR-URL: nodejs#11513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Test that an error is thrown when bind() is called on an already bound socket. PR-URL: nodejs#10894 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Claudio Rodriguez <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
* 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 <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes coverity scan issue 55489. PR-URL: nodejs#10891 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Brian White <[email protected]>
PR-URL: nodejs#10911 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
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 <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
* 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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Refs: nodejs#9493 PR-URL: nodejs#10925 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
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 <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
The last release Chris did was v1.8.1 from April 2015. PR-URL: nodejs#11011 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]>
PR-URL: nodejs#11024 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#11277 Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
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 <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
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 [email protected]
Reviewed-By: Sam Roberts [email protected]
Reviewed-By: Evan Lucas [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Joyee Cheung [email protected]