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

v12.13.2 proposal #30865

Closed
wants to merge 226 commits into from
Closed

v12.13.2 proposal #30865

wants to merge 226 commits into from

Conversation

BethGriggs
Copy link
Member

@BethGriggs BethGriggs commented Dec 9, 2019

Now #31069

TBD, Version 12.13.2 'Erbium' (LTS), @BethGriggs

Notable changes

  • crypto: fix key requirements in asymmetric cipher (Tobias Nießen) #30249
  • deps:
    • update llhttp to 2.0.1 (Fedor Indutny) #30553
    • upgrade npm to 6.13.1 (claudiahdz) #30533
    • update nghttp2 to 1.40.0 (gengjiawen) #30493
  • v8: mark serdes API as stable (Anna Henningsen) #30234

Commits

cclauss and others added 30 commits December 1, 2019 10:33
PR-URL: #30220
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
This is the certdata.txt[0] from NSS 3.47, released on 2019-10-21.

This is the version of NSS that will ship in Firefox 71 on
2019-12-10.

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

PR-URL: #30195
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Update the list of root certificates in src/node_root_certs.h with
tools/mk-ca-bundle.pl.

Certificates added:

Certificates removed:
- Certplus Class 2 Primary CA
- Deutsche Telekom Root CA 2

PR-URL: #30195
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
This would otherwise sometimes just print relatively useless
information about the value in question, such as `[object Object]`.

PR-URL: #30167
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Do not accept the BOM if it comes from a different encoding, and
only discard the BOM after it has actually been read (including
when it is spread over multiple chunks in streaming mode).

Fixes: #25315

PR-URL: #30132
Reviewed-By: Gus Caplan <[email protected]>
This function was not actually available during any part
of the Node 12 release line because it had been removed
earlier (likely accidentally).

Refs: #27220

PR-URL: #30098
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
This makes the actual behaviour match the documented (and arguably
the correct) behaviour.

PR-URL: #30230
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Since this code runs during process and Worker shutdown, it should not
call user-provided code and thereby e.g. provide a way to break out of
`worker.terminate()`.

PR-URL: #30228
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
`writing-tests.md` states to use arrow functions when appropriate.
This updates the examples to do that.

Further, this syncs the docs with what's found in
[`test/parallel/test-http-agent-null.js`](https://github.com/nodejs/node/blob/master/test/parallel/test-http-agent-null.js)

PR-URL: #30126
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
This has been around for a long time, and the underlying V8 API has
become stable as well a while ago.

PR-URL: #30234
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Allow doctool to fallback to use local files if not building a release
build.

PR-URL: #30214
Fixes: #29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Move a handful of inactive Collaborators to emeriti.

PR-URL: #30243
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
If `asyncReset()` is used to specify an alternative resource object
to mark a re-used socket in the HTTP Agent implementation,
store that object and keep it alive, because domains rely on GC tracking
for resource objects to manage their own lifetimes, and previously that
resource object might have been garbage-collected too early, leading to
crashes.

Fixes: #30122

PR-URL: #30196
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
This makes it possible to tell whether a signal is being tracked in JS.

PR-URL: #30229
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Run `EndStartedProfilers` on Environment teardown.

This is part of a series of changes to make embedding easier, by
requiring fewer internal methods to build a fully functioning
Node.js instance.

PR-URL: #30229
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Run inspector cleanup code on Environment teardown.

This is part of a series of changes to make embedding easier, by
requiring fewer internal methods to build a fully functioning
Node.js instance.

PR-URL: #30229
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
This makes more sense than releasing and re-wrapping the raw pointer.

PR-URL: #30229
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
This is part of a series of changes to make embedding easier, by
requiring fewer internal methods to build a fully functioning
Node.js instance.

This also aligns the worker_threads code with the main thread code.

PR-URL: #30229
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.

PR-URL: #30210
Fixes: #30209
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.

Fixes: #30209

PR-URL: #30210
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This consists of some hopefully-uncontroversial
simplifications/clarifications to the text. The one substantial change
is to update Node.js Board of Directors to be the OpenJS Board of
Directors.

PR-URL: #30259
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
This test occasionally fails on macOS with the following error

```
events.js:187
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:201:27)
Emitted 'error' event on TLSSocket instance at:
    at emitErrorNT (internal/streams/destroy.js:84:8)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: -54,
  code: 'ECONNRESET',
  syscall: 'read'
}
```

Fix it by making the client send the close_notify alert instead of the
server.

PR-URL: #30202
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Store the result of excetuting the function in variable. Instead of
excetuting it for multiple times.

PR-URL: #30303
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes: #30245
PR-URL: #30256
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Jan Krems <[email protected]>
PR-URL: #30299
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #30301
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
PR-URL: #30321
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Changed a variable declaration.

PR-URL: #30320
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
PR-URL: #30318
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Refs: nodejs/code-and-learn#97

PR-URL: #30261
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
There is currently no test that confirms that an invalid TLS protocol
results in ERR_TLS_INVALID_PROTOCOL_VERSION. Add tests to check this for
the `minVersion` and `maxVersion` options in `createSecureContext()`.

Refs: https://codecov.io/gh/nodejs/node/src/c14c476614e3134867ddb997bdfe5a41ba668175/lib/_tls_common.js#L56
Refs: https://coverage.nodejs.org/coverage-c14c476614e31348/lib/_tls_common.js.html#L56

PR-URL: #30741
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@nodejs-github-bot nodejs-github-bot added 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. v12.x labels Dec 9, 2019
Notable changes:

- crypto: fix key requirements in asymmetric cipher (Tobias Nießen)
  #30249
- deps:
    - update llhttp to 2.0.1 (Fedor Indutny)
      #30553
    - upgrade npm to 6.13.1 (claudiahdz)
      #30533
    - update nghttp2 to 1.40.0 (gengjiawen)
      #30493
- v8: mark serdes API as stable (Anna Henningsen)
  #30234

PR-URL: #30865
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Dec 9, 2019

As noted in #30109, there's a failing test on freebsd.
I don't know what's the cause and it's strange because I ran CI last time v12.x-staging was pushed and it was green

@targos
Copy link
Member

targos commented Dec 9, 2019

@richardlau
Copy link
Member

Is someone able to validate that #30346 is correct for 12.x?

richardlau
richardlau previously approved these changes Dec 9, 2019
Comment on lines +219 to +220
* [Node.js 13](https://github.com/nodejs/node/blob/v13.x/BUILDING.md)
* [Node.js 12](https://github.com/nodejs/node/blob/v12.x/BUILDING.md)
Copy link
Member

Choose a reason for hiding this comment

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

Technically these two are not "older versions of Node.js" but I don't particularly object to the links being here.

@targos
Copy link
Member

targos commented Dec 9, 2019

@richardlau what do you think about the test that is consistently failing on FreeBSD only?
v12.x is green and that test is not touched by the changes so it is happening because of something else.

@richardlau richardlau dismissed their stale review December 9, 2019 19:08

FreeBSD failure needs investigating

@richardlau
Copy link
Member

@richardlau what do you think about the test that is consistently failing on FreeBSD only?
v12.x is green and that test is not touched by the changes so it is happening because of something else.

Fair point. I've dismissed my review pending further investigation.

@richardlau
Copy link
Member

richardlau commented Dec 9, 2019

@richardlau what do you think about the test that is consistently failing on FreeBSD only?
v12.x is green and that test is not touched by the changes so it is happening because of something else.

Fair point. I've dismissed my review pending further investigation.

Probably worth noting that FreeBSD was downgraded to an experimental platform in Node.js 12 so in theory test failures there do not block releases (we don't build FreeBSD binaries for distribution on https://nodejs.org/en/download/, for example). But it would be good to understand what's going on here.

(Edit: discussion about potentially restoring FreeBSD tier 2 status: nodejs/build#1791)

cc @nodejs/platform-freebsd

@targos
Copy link
Member

targos commented Dec 11, 2019

Is someone able to validate that #30346 is correct for 12.x?

I confirm. The methods were removed in V8 6.9 (v8/v8@313bc6d). v12.0.0 had V8 7.4.

@Trott
Copy link
Member

Trott commented Dec 12, 2019

I imagine this should add d8fc0ae.

@richardlau
Copy link
Member

I imagine this should add d8fc0ae.

Sounds like the npm update will be going out in a security release: nodejs/nodejs.org#2822

@Trott
Copy link
Member

Trott commented Dec 22, 2019

Since 12.14.0 has been released, should this be closed? Or updated to be a 12.14.1 release?

@BethGriggs
Copy link
Member Author

Closing in favour of #31069 (I don't think you can change the PR branch and I'd like the version numbers to match)

@BethGriggs BethGriggs closed this Dec 23, 2019
@addaleax addaleax deleted the v12.13.2-proposal branch December 24, 2019 23:28
@targos targos added release Issues and PRs related to Node.js releases. and removed 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. labels Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Issues and PRs related to Node.js releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.