Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v6.x backport] deps: ICU 60.2, 60.1, and 59.1 backports #18240

Closed
wants to merge 31 commits into from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Jan 18, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Backports of: (in order of appearance)

evanlucas and others added 30 commits January 17, 2018 01:59
crypto.randomFill and crypto.randomFillSync are similar to
crypto.randomBytes, but allow passing in a buffer as the first
argument. This allows us to reuse buffers to prevent having to
create a new one on every call.

PR-URL: nodejs#10209
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This patch replaces the word `buf` with `buffer` and fixes the broken
link to `randomfill`.

PR-URL: nodejs#12541
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#13481
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: nodejs#13553
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently REPL supports autocompletion for core modules and those found
in node_modules.  This commit adds tab completion for modules relative
to the current directory.

PR-URL: nodejs#14409
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
Both are simple utility functions defined by the WHATWG
console spec (https://console.spec.whatwg.org/).

PR-URL: nodejs#12678
Ref: nodejs#12675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Add wrapper for uv's uv_udp_set_multicast_interface which provides the
sender side mechanism to explicitly select an interface. The
equivalent receiver side mechanism is the optional 2nd argument of
addMembership().

PR-URL: nodejs#7855
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The test parallel/test-dgram-multicast-set-interface.js was
calling common.skip() on hosts that do not support IPv6. However,
by this point, there were several outstanding common.mustCall()
invocations. The process.exit() in common.skip() triggered
those common.mustCall()s as errors.

Fixes: nodejs#15419
PR-URL: nodejs#15421
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
The crypto classes are also exposed as createClass for each class. This
tests that each of them returns an instance of the class in question.

Backport-PR-URL: nodejs#16245
PR-URL: nodejs#8188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
For consistency with the rest of the crypto classes, exposes the ECDH
class. Originally, only the createECDH function was exposed, and there
was no real reason to hide the class.

Backport-PR-URL: nodejs#16245
PR-URL: nodejs#8188
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
On non-FIPS, we can instantiate DiffieHellman with 256 instead of 1024.
This should be quite a bit faster, and therefore prevent the timeouts.

Backport-PR-URL: nodejs#16245
PR-URL: nodejs#15662
Fixes: nodejs#15655
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: nodejs#15179
Fixes: nodejs#15012
Refs: nodejs#5656
Refs: nodejs#2571
Refs: nodejs#7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#16835
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes: nodejs#14957
PR-URL: nodejs#16839
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The --use-bundled-ca and --use-openssl-ca command line arguments are
mutually exclusive but can both be used on the same command line.

This commit adds a check if both options are used.

Fixes: nodejs#12083
Backport-PR-URL: nodejs#17783
PR-URL: nodejs#12087
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Currently, the following warning will be reported when configuring
without-ssl:

../src/node.cc:3653:8: warning: unused variable 'use_bundled_ca'
[-Wunused-variable]
  bool use_bundled_ca = false;
       ^
../src/node.cc:3654:8: warning: unused variable 'use_openssl_ca'
[-Wunused-variable]
  bool use_openssl_ca = false;
       ^

I missed this when working on
commit 8a7db9d ("src: add
--use-bundled-ca --use-openssl-ca check").

Refs: nodejs#12087
Backport-PR-URL: nodejs#17783
PR-URL: nodejs#12302
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Commit de168b4 ("src: guard
bundled_ca/openssl_ca with HAVE_OPENSSL") included an incorrect end
macro comment which this commit fixes.

PR-URL: nodejs#12688

Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
I've not been able to find any reason for calling
BIO_set_shutdown(bio, 1). This is done by default for the following
versions of OpenSSL:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/
crypto/bio/bio_lib.c#L26

https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/
crypto/bio/bio_lib.c#L90

https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/
crypto/bio/bio_lib.c#L88

https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/
crypto/bio/bio_lib.c#L90

This commit removes the call and the comment.

Backport-PR-URL: nodejs#17784
PR-URL: nodejs#17542
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- Add IsInvalidated() method
- Add capacity() method for finding out the actual capacity, not the
  current size, of the buffer
- Make IsAllocated() work for invalidated buffers
- Allow multiple calls to AllocateSufficientStorage() and Invalidate()
- Assert buffer is malloc'd in Release()
- Assert buffer has not been invalidated in AllocateSufficientStorage()
- Add more descriptive comments describing the purpose of the methods
- Add cctest for MaybeStackBuffer

Backport-PR-URL: nodejs#17365
PR-URL: nodejs#11464
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#17365
PR-URL: nodejs#9246
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Remove custom tests for invalid IDNA domains in url-idna.js in favor of
the more comprehensive official set.

Backport-PR-URL: nodejs#17365
PR-URL: nodejs#13362
Refs: whatwg/url#309
Refs: web-platform-tests/wpt#5976
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Implements WHATWG URL support. Example:

```
var u = new url.URL('http://example.org');
```

Many, many other commits improving the implementation have been squashed
into this backport PR. They are not listed separately here for brevity.

Backport-PR-URL: nodejs#17365
PR-URL: nodejs#7448
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Backport-PR-URL: nodejs#17833
PR-URL: nodejs#13784
Fixes: nodejs#13771
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on nodejs#12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: nodejs#12087
Backport-PR-URL: nodejs#18113
PR-URL: nodejs#12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Introduce two overridable `Agent` methods:

* `keepSocketAlive(socket)`
* `reuseSocket(socket, req)`

These methods can be overridden by particular `Agent` class child to
make keep-alive behavior customizable.

Motivation: destroy persisted sockets after some configurable timeout.
It is very non-trivial to do it with available primitives. Such program
will most likely need to poke with undocumented events and methods of
`Agent`. With introduced API such behavior is easy to implement.

Backport-PR-URL: nodejs#18168
PR-URL: nodejs#13005
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
The motivation for this commit is that we need to specify system CA
certificates when building node. While we are aware of the environment
variable NODE_EXTRA_CA_CERTS this is not a great solution as we build
an RPM and we also don't want users to be able to unset them.

The suggestion is to add a configure time property like this:

--openssl-system-ca-path=OPENSSL_SYSTEM_CA_PATH
             Use the specified path to system CA (PEM format) in
             addition to the OpenSSL supplied CA store or compiled-
             in Mozilla CA copy.

Usage example:
$ ./configure --openssl-system-ca-path=/etc/pki/tls/certs/ca-bundle.crt

This would add the specified CA certificates in addition to the ones
already being used.

Backport-PR-URL: nodejs#18173
PR-URL: nodejs#16790
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Provides list of all builtin modules in Node.

Includes modules of all types:
- prefixed (ex: _tls_common)
- deprecated (ex: sys)
- regular (ex: vm)

Backport-PR-URL: nodejs#18221
PR-URL: nodejs#16386
Refs: nodejs#3307
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* No feature changes.
* Bug fixes.
* Details: http://site.icu-project.org/download/59

Fixes: nodejs#12077
PR-URL: nodejs#12486
Refs: nodejs#7844
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
- Update to released ICU 60.1, including:
  - CLDR 32 (many new languages and data improvements)
  - Unicode 10 (8,518 new characters, including four new scripts,
  7,494 new Han characters, and 56 new emoji characters)
  - UTF-8 malformed bytes now handled according to W3C/WHATWG spec

Fixes: nodejs#15540
PR-URL: nodejs#16876
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
- Update to Maintainance ICU 60.2
- CLDR 32.0.1
- http://site.icu-project.org/download/60#TOC-ICU-60.2-bug-fixes
- Subsumes nodejs#16931

PR-URL: nodejs#17687
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jan 18, 2018
@srl295 srl295 changed the base branch from master to v6.x January 18, 2018 20:45
@srl295 srl295 changed the base branch from v6.x to v6.x-staging January 18, 2018 20:45
@srl295 srl295 self-assigned this Jan 18, 2018
@srl295
Copy link
Member Author

srl295 commented Jan 18, 2018

@mhdawson @MylesBorins by request. please feel free to take over the PR if needed.

@srl295
Copy link
Member Author

srl295 commented Jan 18, 2018

Landed cleanly for me, perhaps a v8 backport already happened to the 6.x staging?

@MylesBorins
Copy link
Contributor

@MylesBorins
Copy link
Contributor

Looks like compilation is failing on OSX

@srl295
Copy link
Member Author

srl295 commented Jan 27, 2018

@MylesBorins osx1010#15684 failure… huh. I don't have osx1010 access… I wonder if I was getting some compiler options 'for free' on latest master and need to update the .gyp

  c++ '-D_DARWIN_USE_64_BIT_INODE=1' '-DU_I18N_IMPLEMENTATION=1' '-DU_ATTRIBUTE_DEPRECATED=' '-D_CRT_SECURE_NO_DEPRECATE=' '-DU_STATIC_IMPLEMENTATION=1' '-DUCONFIG_NO_TRANSLITERATION=1' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' '-DUCONFIG_NO_LEGACY_CONVERSION=1' '-DUCONFIG_NO_CONVERSION=1' -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -Os -gdwarf-2 -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/out/Release/.deps//Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/alphaindex.o.d.raw   -c -o /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/alphaindex.o ../deps/icu-small/source/i18n/alphaindex.cpp
In file included from ../deps/icu-small/source/i18n/affixpatternparser.cpp:14:
In file included from ../deps/icu-small/source/i18n/unicode/dcfmtsym.h:37:
In file included from ../deps/icu-small/source/i18n/unicode/numsys.h:39:
In file included from ../deps/icu-small/source/i18n/unicode/format.h:36:
In file included from ../deps/icu-small/source/common/unicode/unistr.h:33:
../deps/icu-small/source/common/unicode/char16ptr.h:69:27: error: no type named 'nullptr_t' in namespace 'std'
    inline Char16Ptr(std::nullptr_t p);
                     ~~~~~^

Websearch turns up SO#24458099 suggesting

#include <cstddef>

… which fails on Linux. @MylesBorins any hope of getting specific compiler details on the osx1010 machine?

@MylesBorins
Copy link
Contributor

MylesBorins commented Feb 2, 2018

@srl295 I've pushed a potential hack to fix this. It checks to see if the C++ version is below a certain threshold and defines the null_ptr typedef in std if so

CI: https://ci.nodejs.org/job/node-test-pull-request/12911/

edit: This is failing locally on the latest OSX fwiw

@MylesBorins
Copy link
Contributor

I've dug in a bit more, chasing the C++ 11 features and slowly keep hitting edge cases. I pushed the latest commit I had, which is currently failing at std::round not being defined

@srl295
Copy link
Member Author

srl295 commented Feb 2, 2018

@MylesBorins boy when you say it that way, it sounds kinda familiar [commit by some srl). And sorry, I realized I had done the merge on linux, but I was sitting at a mac so I thought I couldn't repro this. I can repro it locally.

But, the problem seems to be the lack of -stdlib=libc++

@srl295
Copy link
Member Author

srl295 commented Feb 2, 2018

@MylesBorins ouch… that's a lot of painful chasing. I think the compiler options are wrong here.

@srl295
Copy link
Member Author

srl295 commented Feb 2, 2018

@srl295
Copy link
Member Author

srl295 commented Feb 2, 2018

basically need to backport 7292a1e and maybe 4a16a5d

kind of think this backport may not really work… ICU v59 was a big jump in terms of C++ requirements.

@MylesBorins
Copy link
Contributor

yeah, those changes may be semver major
/cc @nodejs/lts @nodejs/build @nodejs/tsc

We would have to switch to libc++ on osx and enable more features for gnu++

I'm not sure I feel comfortable doing that just before maintenance to get an upgraded dep.

@MylesBorins
Copy link
Contributor

@srl295 likely that this is not going to land at this point. Thanks for putting in the effort

@srl295
Copy link
Member Author

srl295 commented Feb 11, 2018 via email

@MylesBorins MylesBorins force-pushed the v6.x-staging branch 3 times, most recently from 5bdb18e to 691cd5a Compare February 13, 2018 00:33
@srl295 srl295 deleted the icu-60.2-6.x branch July 6, 2018 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.