[9.x] Backport a lot of things #20448
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
Move functions that deal with `BufferList` to `BufferList.prototype`. PR-URL: nodejs#18353 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Docs-only deprecate the getter/setter crypto.fips and replace with crypto.setFips() and crypto.getFips() This is specifically in preparation for ESM module support PR-URL: nodejs#18335 Refs: nodejs#18131 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#18270 Fixes: nodejs#8307 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Implement string decoder in C++. The perks are a decent speed boost (for decoding, whereas creation show some performance degradation), that this can now be used more easily to add native decoding support to C++ streams and (arguably) more readable variable names. PR-URL: nodejs#18537 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#18581 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
moves exposed internalBindings to a single location with short guidelines on how to expose them and a warning for users should they come across it PR-URL: nodejs#18698 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
There are libraries which invoke StringDecoder using .call and .inherits, which directly conflicts with making StringDecoder be a class which can only be invoked with the new keyword. Revert to declaring it as a function. StringDecoder#lastNeed was not defined, redefine it using the new interface and fix StringDecoder#lastTotal. PR-URL: nodejs#18723 Refs: nodejs#18537 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
This commit set the Environment as a thread local when CreateEnvironment is called which is currently not being done. This would lead to a segment fault if later node::AtExit is called without specifying the environment parameter. This specific issue was reported by Electron. If I recall correctly, back when this was implemented the motivation was that if embedders have multiple environments per isolate they should be using the AtExit functions that take an environment. This is not the case with Electron which only create a single environment (as far as I know), and if a native module calls AtExit this would lead to the segment fault. I was able to reproduce Electron issue and the provided test simulates it. I was also able to use this patch and verify that it works for the Electron issue as well. PR-URL: nodejs#18573 Refs: nodejs#9163 Refs: electron/electron#11299 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matheus Marchini <matheus@sthima.com>
Wrap SafeGetenv() in util binding with the purpose of protecting the cases when env vars are accessed with the privileges of another user in jsland. PR-URL: nodejs#18511 Fixes: nodejs#9160 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
- add new constants - replace "magic" numbers in "module" by constants PR-URL: nodejs#18785 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matheus Marchini <matheus@sthima.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This utility is fairly generic and likely useful for more than one test. PR-URL: nodejs#18800 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
TTY tests should almost never be placed in `/parallel/`. Skipping TTY tests there due to missing tty fds just means they will never be run, ever, on any system. This moves the tty-get-color-depth test to `/pseudo-tty/` where the test runner will actually make a pty fd. Refs: nodejs#17615 PR-URL: nodejs#18800 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#18566 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Shutting down the connection is what `_final` is there for. PR-URL: nodejs#18608 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This adds the eslint rules back in that were falsely removed while landing nodejs#18566. PR-URL: nodejs#18933 Refs: nodejs#18566 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- add new constants - replace numbers by constants PR-URL: nodejs#18869 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Matheus Marchini <matheus@sthima.com>
Follow-up from nodejs#18800 Code that tries to exercise tty fds must be placed in `/pseudo-tty/`. PR-URL: nodejs#18886 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Add 80 characters limit to docs. Change docs to fit 80 characters per row. PR-URL: nodejs#18726 Fixes: nodejs#18703 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
PR-URL: nodejs#18717 Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#18415 Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Provide public APIs for native typechecking that is actually useful.
The motivation for this is providing alternatives to userland
modules that would currently rely on `process.binding('util')`.
PR-URL: nodejs#18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Refs: nodejs#18415 PR-URL: nodejs#19149 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Implement asynchronous equivalent for assert.throws() and assert.doesNotThrow(). PR-URL: nodejs#18023 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit adds proper error handling to net.connect() when a custom lookup() function returns an invalid address family. PR-URL: nodejs#19415 Fixes: nodejs#19407 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: James M Snell <jasnell@gmail.com>
The usefulness of `assert.doesNotReject` is very limited and this warns against the usage. PR-URL: nodejs#19462 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The color output was broken at some point and that was not detected because it was not tested for properly so far. This makes sure the colors work again and it adds a regression test as well. PR-URL: nodejs#19464 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#19446 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: Shingo Inoue <leko.noor@gmail.com>
Squashed from: - lib: separate writev responsibilities from writeGeneric - lib: fix calling of cb twice - lib: extract streamId out of stream_base to caller - lib: add symbols instead of methods to hide impl details - lib: remove unneeded lines - lib: use Object.assign instead of apply - lib: rename mixin StreamBase to StreamSharedMethods - lib: use stream shared funcs as top level instead of properties of prototypes - lib: mv lib/internal/stream_shared_methods.js lib/internal/stream_base_commons.js - lib: add comment for readability - lib: refactor _writev in Http2Stream - lib: rephrase comment - lib: revert usage of const,let for perf reasons PR-URL: nodejs#19527 Refs: nodejs#19060 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit renames all JavaScript source files in lib to lower snake_case. PR-URL: nodejs#19556 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
The old variants have been deprecated since b20af80. Refs: nodejs#18415 PR-URL: nodejs#19602 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
changes the base instance for ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED from Error to TypeError as a more accurate representation of the error.. PR-URL: nodejs#19958 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Adds NASM installation to the Boxstarter script. Refs: nodejs#19918 PR-URL: nodejs#19950 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#20028 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#18689 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Also, make signatures easier for copy-paste testing. PR-URL: nodejs#20036 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#20003 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Original commit message:
[api] introduce v8::Value::IsModuleNamespaceObject
This allows an embedder to check if a Value is a module namespace object.
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Idffceff451dd5f5c6a53d4cb3ce02c1c2c5b653c
Reviewed-on: https://chromium-review.googlesource.com/1011762
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#52597}
Refs: v8/v8@39d546a
PR-URL: nodejs#20016
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#20016 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This adds direct promise support to `assert.rejects` and `assert.doesNotReject`. It will now accept both, functions and ES2015 promises as input. Besides this the documentation was updated to reflect the latest changes. It also refactors the tests to a non blocking way to improve the execution performance and improves the coverage. PR-URL: nodejs#19885 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This makes sure `assert.rejects` is always called with a second argument. Besides that it is also changes a eslint error message to suggest objects instead of a regular expression. PR-URL: nodejs#19885 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This adds a example to `assert.throws` to document checking against error instances. It also makes it clearer what some arguments can receive as input. PR-URL: nodejs#19885 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
* Fix patch for CVE-2017-1000381 to not be overly aggressive. * win32: Preserve DNS server order returned by Windows when sorting and exclude DNS servers in legacy subnets. * win32: Support most recent Visual Studio 2017. * gethostbyaddr should fail with ECANCELLED not ENOTFOUND when ares_cancel is called. * win32: Exclude legacy ipv6 subnets. * android: Applications compiled for Oreo can no longer use __system_property_get and must use Java calls to retrieve DNS servers. * win32: Force use of ANSI functions. * CMake minimum version is now 3.1. * ares_gethostbyname.3: fix callback status values. * docs: Document WSAStartup requirement. * Fix a typo in init_by_resolv_conf. * Android JNI code leaks local references in some cases. * Force using the ANSI versions of WinAPI functions. The most important changes have already been included via * 50e580d deps: cherry-pick 0ef4a0c64b6 from c-ares upstream * 9a0631d deps: cherry-pick 18ea996 from c-ares upstream PR-URL: nodejs#19939 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
c-ares switched to using ares_ssize_t for platform-independent ssize_t, our GYP usage to include config/<platform>/ares_config.h causes problems when including gyp as a library in core, i.e. in env.h and cares_wrap.h, where the defines don't get pulled in properly. This, so far, is the easiest approach to just making it work nicely--explicitly defining ares_ssize_t for the different Windows variants and ssize_t for non-Windows where we don't have a configured type from an ares_config.h. In all of our non-Windows platforms it is ssize_t anyway so this is safe. PR-URL: nodejs#15378 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: nodejs#19939 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Was 72c5458: PR-URL: nodejs#5090 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reimplemented for c-ares 1.13.0 PR-URL: nodejs#15378 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: nodejs#19939 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Indicate that `base` is ignored if `input` is absolute. PR-URL: nodejs#19899 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#20046 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#20047 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: nodejs#20050 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#19900 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#19828 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
@addaleax @joyeecheung I ran into an issue while backporting #18415 that you two could most likely fix best. Test I guess it is some kind of circular dependency or so. I tried to fix it but I failed, so it would be great if you could have a look at it. The error is triggered due to the backport of #19177. I could of course just exclude that part but I think it would be nice to also backport that. |
|
@BridgeAR Is there any reason you’re backporting a lot of patches to v9.x? I assume we wouldn’t do a release with those, so I figure the goal is to enable easier backporting to v8.x, but I’m not sure how well this fares with the policy that we generally want to have coverage for commits from a Current (i.e. 10.x) release? Anyway, I can try to take a look later today. |
|
@addaleax well, I think it would be nice to get these changes in 9.x as well. But one of my main goals is definitely to make backporting to 8.x easier. I am going to try to get as much things into 8.x as possible. Without having almost everything properly backported in 9.x this is going to be super tricky. I am almost through with all commits that should be backported besides a couple commits that are related to Right now I am just fixing up some minor issues left. |
|
@BridgeAR I have tried to backport the refactors a while back in https://github.com/joyeecheung/node/commits/backport-refactors-to-v9.x but was stuck because some ESM loader refactor was not backported first and I figured v9.x was reaching EOL soon so didn't pursue this further. |
|
@BridgeAR ... whatever subset of these are (a) non-controversial and (b) semver-minor or patch ... we can generally just pull them into the staging branch directly. I know you can't do that yourself but if you're reasonably certain that these will all land cleanly, we should be able to pull them in without requiring a review. |
|
Going to close this out since 9.x won't get another release. @BridgeAR I assume you can just use your branch as a jumping off point for 8.x if you were still planning to do that back-porting work but definitely feel free to reopen the PR if you prefer. |
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.
This is currently WIP. One test is failing on my machine.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes