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

v4.8.5 proposal #15507

Closed
wants to merge 32 commits into from
Closed

v4.8.5 proposal #15507

wants to merge 32 commits into from

Conversation

MylesBorins
Copy link
Contributor

2017-10-03, Version 4.8.5 'Argon' (Maintenance), @MylesBorins

Notable Changes

Coming Soon

Commits

ArchangeGabriel and others added 30 commits July 11, 2017 17:44
* The following function from <unicode/normlzr.h> is used:
     normalize()

* Until ICU 59, <unicode/normlzr.h> is indirectly included, but this changed with the 59 release. Adding this header has been the right thing to do for many years, so it is backwards compatible and fix compilation with recent ICU.

Refs: #13022
PR-URL: #13040
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
This fixes wrong hash results on Windows with some CPUs that support
Intel SHA Extension and resolves the issue of TLS connection errors.

After upgrading forthcoming openssl-1.0.2l, this is no nolonger needed.

  Original commit message:

    perlasm/x86_64-xlate.pl: work around problem with hex constants in masm.

    Perl, multiple versions, for some reason occasionally takes issue with
    letter b[?] in ox([0-9a-f]+) regex. As result some constants, such as
    0xb1 came out wrong when generating code for MASM. Fixes GH#3241.

    Reviewed-by: Rich Salz <[email protected]>
    (Merged from openssl/openssl#3385)

    (cherry picked from commit c47aea8af1e28e46e1ad5e2e7468b49fec3f4f29)

Refs: openssl/openssl#3241
Refs: openssl/openssl#3385
Fixes: #12691
PR-URL: #12913
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Regenerate asm files according to the fix of
openssl/crypto/perlasm/x86_64-xlate.pl.

Fixes: #12691
PR-URL: #12913
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock()
used to self-recurse before this commit, causing stack overflows on
systems with small stack sizes.  Make it non-recursive by storing
intermediate results in a heap-allocated list.

Fixes: #11991
Backport-PR-URL: #13080
PR-URL: #12460
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yang Guo <[email protected]>
Functions that call `ECDH::BufferToPoint` were not clearing the
error stack on failure, so an invalid key could leave leftover
error state and cause subsequent (unrelated) signing operations
to fail.

PR-URL: #13275
Backport-PR-URL: #13399
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fixes: #12833
PR-URL: #12957
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gpg 2.1 no longer includes the key-id by default which breaks
the release script. This makes sure we are explicit about it.

PR-URL: #13309
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Backport-PR-URL: #14635
PR-URL: #14578
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Backport-PR-URL: #14635
PR-URL: #14578
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Allow passing the prefix in via the PKGDIR env var. This will allow us
to use this same script to codesign the binary tarball.

PR-URL: #14179
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Previously, we were signing the binary that was released in the .pkg,
but not the binary released in the tarball.

PR-URL: #14179
Fixes: #11936
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    ares_parse_naptr_reply: make buffer length check more accurate

    9478908a490a6bf009ba58d81de8c1d06d50a117 introduced a length check
    for records parsed by `ares_parse_naptr_reply()`. However, that
    function is designed to parse replies which also contain non-NAPTR
    records; for A records, the `rr_len > 7` check will fail as there
    are only 4 bytes of payload.
    In particular, parsing ANY replies for NAPTR records was broken
    by that patch.

    Fix that by moving the check into the case in which it is already
    known that the record is a NAPTR record.

Ref: c-ares/c-ares@18ea996
PR-URL: #13883
Reviewed-By: James M Snell <[email protected]>
This is the certdata.txt[0] that ships in NSS 3.28.1, released on
2017-01-04.

[0] https://hg.mozilla.org/projects/nss/raw-file/NSS_3_28_1_RTM/lib/ckfw/builtins/certdata.txt

PR-URL: #12402
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
Update the list of root certificates in src/node_root_certs.h with
tools/mk-ca-bundle.pl.

Certificates added:
- AC RAIZ FNMT-RCM
- Amazon Root CA 1
- Amazon Root CA 2
- Amazon Root CA 3
- Amazon Root CA 4
- Certplus Root CA G1
- Certplus Root CA G2
- Hellenic Academic and Research Institutions ECC RootCA 2015
- Hellenic Academic and Research Institutions RootCA 2015
- ISRG Root X1
- LuxTrust Global Root 2
- OpenTrust Root CA G1
- OpenTrust Root CA G2
- OpenTrust Root CA G3

Certificates removed:
- Buypass Class 2 CA 1
- EBG Elektronik Sertifika Hizmet Sağlayıcısı
- IGC/A
- Juur-SK
- RSA Security 2048 v3
- Root CA Generalitat Valenciana

PR-URL: #12402
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
This is the certdata.txt[0] that ships in NSS 3.30.2, released on
2017-04-20.

[0] https://hg.mozilla.org/projects/nss/raw-file/NSS_3_30_2_RTM/lib/ckfw/builtins/certdata.txt

PR-URL: #13279
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Update the list of root certificates in src/node_root_certs.h with
tools/mk-ca-bundle.pl.

Certificates added:
- TUBITAK Kamu SM SSL Kok Sertifikasi - Surum 1

Certificates removed:
- ApplicationCA - Japanese Government
- Microsec e-Szigno Root CA
- TÜRKTRUST Elektronik Sertifika Hizmet Sağlayıcısı H6
- WellsSecure Public Root Certificate Authority

PR-URL: #13279
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This reverts commit 2d07fd7, with the
exception of the `V8_PATCH_LEVEL` which will continue to increase.

This commit was intended to be a backport of v8's e093a04 and 09db540,
but it failed to consider the reversion of e093a04 in 5f5a328.

PR-URL: #14829
Reviewed-By: Ben Noordhuis <[email protected]>
Original commit messages:
v8/v8@09db540
  Reland of Rehash and clear deleted entries in weak collections during GC

  BUG=v8:4909
  [email protected],[email protected]
  LOG=n

  Review URL: https://codereview.chromium.org/1890123002

  Cr-Commit-Position: refs/heads/master@{#35538}

v8/v8@686558d
  Fix comment about when we rehash ObjectHashTables before growing them

  [email protected]
  BUG=

  Review-Url: https://codereview.chromium.org/1918403003
  Cr-Commit-Position: refs/heads/master@{#35853}

Refs: https://crbug.com/v8/4909
Refs: #6180
Refs: #7689
Refs: #6398
Fixes: #14228

PR-URL: #14829
Reviewed-By: Ben Noordhuis <[email protected]>
When upgrading OpenSSL, Step 6 in upgrading guide explains the steps
that need to be taken if asm files need updating. This might not
always be the case and something that needs to be checked from
release to release.

This commit adds an example of using github to manually compare two tags
to see if any changes were made to asm files.

Backport-PR-URL: #13696
PR-URL: #13234
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This replaces all sources of openssl-1.0.2l.tar.gz into
deps/openssl/openssl

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
All symlink files in deps/openssl/openssl/include/openssl/ are removed
and replaced with real header files to avoid issues on Windows. Two
files of opensslconf.h in crypto and include dir are replaced to refer
config/opensslconf.h.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
`x86masm.pl` was mistakenly using .486 instruction set, why `cpuid` (and
perhaps others) are requiring .686 .

Fixes: #589
Backport-PR-URL: #13696
PR-URL: #1389
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Shigeki Ohtsu <[email protected]>
See
https://mta.openssl.org/pipermail/openssl-dev/2015-February/000651.html

iojs needs to stop using masm and move to nasm or yasm on Win32.

Fixes: #589
Backport-PR-URL: #13696
PR-URL: #1389
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reapply b910613 .

Fixes: #589
Backport-PR-URL: #13696
PR-URL: #1389
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
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: #1461
Backport-PR-URL: #13696
PR-URL: #1836
Reviewed-By: Ben Noordhuis <[email protected]>
Regenerate config files for supported platforms with Makefile.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Regenerate asm files with Makefile and CC=gcc and ASM=nasm where gcc
version was 5.4.0 and nasm version was 2.11.08.

Also asm files in asm_obsolete dir to support old compiler and
assembler are regenerated without CC and ASM envs.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Added the missing make command in steps 6.3 when building
asm_obsolete.

Also updated the commit message to include the version nasm in
addition to the gcc version.

Fixes: #13161
Backport-PR-URL: #13696
PR-URL: #13233
Reviewed-By: Shigeki Ohtsu <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
This is a partial backport of semver-patch bits of
9e4660b.

This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.

* Throw an Error when the zlib library rejects the value of windowBits,
  instead of crashing with an assertion.

* Treat windowBits and memLevel options consistently with other ones and
  don't crash when non-numeric values are given.

Backport-PR-URL: #14860
PR-URL: #13098
Fixes: #13082
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Unset `mode_` when initializing the zlib stream failed, so that
we don’t try to call the zlib end functions (`deflateEnd()` etc.)
when cleaning up in `ZCtx::Close()`.

Fixes: #14178
Ref: #13098
Backport-PR-URL: #14860
PR-URL: #14666
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. meta Issues and PRs related to the general management of the project. openssl Issues and PRs related to the OpenSSL dependency. v4.x labels Sep 20, 2017
Notable Changes:

Coming Soon

PR-URL: #15507
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Sep 20, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/10170/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/996/
rc.1: https://nodejs.org/download/rc/v4.8.5-rc.1/

@nodejs/tsc @nodejs/release @nodejs/lts:

This is potentially the last maintenance release of v4.x. If you know of any patches being floated by individual companies that might be worth including please chime in. Similarly if you think any of the above patches should not land please chime in.

Specifically we should discuss the zlib changes, openssl updates, and updated certs

@Qantas94Heavy
Copy link
Contributor

I noticed in #13022 that there is a bug in V8 present in 4.x that fails to parse certain Intl timezones correctly.

Is this worth fixing? If so, upstream V8 commits bff3074d and 4e18190e would need to be backported.

@bnoordhuis
Copy link
Member

@Qantas94Heavy v8/v8@bff3074d looks pretty harmless but v8/v8@4e18190e might need closer scrutiny. Have you tested if they apply (and build) cleanly?

@Qantas94Heavy
Copy link
Contributor

@bnoordhuis For the first commit, the only reason it doesn't apply cleanly is due to src/i18n.js being moved to src/js/i18n.js.

For the second commit, as above but also needs %_Call to be replaced with %_CallFunction.

After applying, build and tests run successfully.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MylesBorins MylesBorins mentioned this pull request Sep 22, 2017
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Sep 22, 2017

I've opened a PR to discuss the two V8 patches. I also kicked off a CI job against v4.8.4 to compare as I think we shipped with some known timeouts

https://ci.nodejs.org/job/node-test-commit/12533/

@MylesBorins
Copy link
Contributor Author

As the V8 patches in #15562 have not been able to be tested I think we need to make a decision if we are going to move forward with this release or delay it.

personally I would like to see us get the V8 test job working again so we can verify this release... there are other patches against V8 as well.

Thoughts?

@MylesBorins
Copy link
Contributor Author

/cc @nodejs/release

@MylesBorins
Copy link
Contributor Author

As we've discovered a potentially deeper ICU bug in v4.x I'm going to delay the release at least a week

@Qantas94Heavy
Copy link
Contributor

Do we need #12392 or is that not necessary?

@MylesBorins
Copy link
Contributor Author

@Qantas94Heavy I don't think so. #15562 managed to get V8 CI green

@MylesBorins
Copy link
Contributor Author

Closing to make new proposal for v4.8.6

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. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. meta Issues and PRs related to the general management of the project. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.