Skip to content

Conversation

@MylesBorins
Copy link
Contributor

@MylesBorinsMylesBorins commented Apr 19, 2017

Node.js ChangeLog

2017-05-02, Version 4.8.3 'Argon' (Maintenance), @MylesBorins

Notable Changes

  • module:
  • src:
    • fix base64 decoding in rare edgecase (Nikolai Vavilov) #11995
  • tls:
    • fix rare segmentation faults when using TLS

Commits

  • [44260806a6] - Partial revert "tls: keep track of stream that is closed" (Trevor Norris) #11947
  • [ab3fdf531f] - deps: cherry-pick ca0f9573 from V8 upstream (Ali Ijaz Sheikh) #11940
  • [07b92a3c0b] - doc: add supported platforms list for v4.x (Michael Dawson) #12091
  • [ba91c41478] - module: fix loading from global folders on Windows (Richard Lau) #9283
  • [b5b78b12b8] - src: add fcntl.h include to node.cc (Bartosz Sosnowski) #12540
  • [eb393f9ae1] - src: fix base64 decoding (Nikolai Vavilov) #11995
  • [8ed18a1429] - src: ensure that fd 0-2 are valid on windows (Bartosz Sosnowski) #11863
  • [ff1d61c11b] - stream_base,tls_wrap: notify on destruct (Trevor Norris) #11947
  • [6040efd7dc] - test: fix flaky test-tls-wrap-timeout (Rich Trott) #7857
  • [7a1920dc84] - test: add hasCrypto check to tls-socket-close (Daniel Bevenius) #11911
  • [1dc6b38dcf] - test: add test for loading from global folders (Richard Lau) #9283
  • [54f5258582] - tls: fix segfault on destroy after partial read (Ben Noordhuis) #11898
  • [99749dccfe] - tls: keep track of stream that is closed (jBarz) #11776
  • [6d3aaa72a8] - tls: TLSSocket emits 'error' on handshake failure (Mariusz 'koder' Chwalba) #8805

lekoderand others added 12 commits April 18, 2017 20:02
Removes branch that would make TLSSocket emit '_tlsError' event if error occured on handshake and control was not released, as it was never happening. Added test for tls.Server to ensure it still emits 'tlsClientError' as expected. Note that 'tlsClientError' does not exist in the v4.x branch so this back-port emits 'clientError' instead. See also pull request #4557. Fixes: #8803 PR-URL: #8805 Refs: #4557 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: #12091 Reviewed-By: James M Snell <[email protected]> Reviewed-By: JoãReis <[email protected]> Reviewed-By: Johan Bergströ[email protected]>
Test executes with a copy of the node executable since $PREFIX/lib/node is relative to the executable location. PR-URL: #9283 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: João Reis <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Code was calculating $PREFIX/lib/node relative to process.execPath, but on Windows process.execPath is $PREFIX\node.exe whereas everywhere else process.execPath is $PREFIX/bin/node (where $PREFIX is the root of the installed Node.js). PR-URL: #9283 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: João Reis <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
TLSWrap object keeps a pointer reference to the underlying TCPWrap object. This TCPWrap object could be closed and deleted by the event-loop which leaves us with a dangling pointer. So the TLSWrap object needs to track the "close" event on the TCPWrap object. PR-URL: #11776 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Brian White <[email protected]>
The TLSWrap constructor is passed a StreamBase* which it stores as TLSWrap::stream_, and is used to receive/send data along the pipeline (e.g. tls -> tcp). Problem is the lifetime of the instance that stream_ points to is independent of the lifetime of the TLSWrap instance. So it's possible for stream_ to be delete'd while the TLSWrap instance is still alive, allowing potential access to a then invalid pointer. Fix by having the StreamBase destructor null out TLSWrap::stream_; allowing all TLSWrap methods that rely on stream_ to do a check to see if it's available. While the test provided is fixed by this commit, it was also previously fixed by 478fabf. Regardless, leave the test in for better testing. PR-URL: #11947 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
This partually reverts commit 4cdb0e8. A nullptr check in TSLWrap::IsAlive() and the added test were left. PR-URL: #11947 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
OnRead() calls into JS land which can result in the SSL context object being destroyed on return. Check that `ssl_ != nullptr` afterwards. Fixes: #11885 PR-URL: #11898 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Trigger OOM crash if no memory returned in v8::ArrayBuffer::New and v… …8::SharedArrayBuffer::New. This API does not allow reporting failure, but we should crash rather than have the caller get an ArrayBuffer that isn't properly set up. BUG=chromium:681843 Review-Url: https://codereview.chromium.org/2641953002 Cr-Commit-Position: refs/heads/master@{#42511} PR-URL: #11940 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Check that stdin, stdout and stderr are valid file descriptors on Windows. If not, reopen them with 'nul' file. Refs: #875Fixes: #11656 PR-URL: #11863 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Currently test-tls-socket-close will fail if node was built using --without-ssl. This commit adds a check to verify is crypto support exists and if not skip this test. PR-URL: #11911 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Make sure trailing garbage is not treated as a valid base64 character. Fixes: #11987 PR-URL: #11995 Reviewed-By: Anna Henningsen <[email protected]>
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. lib / src Issues and PRs related to general changes in the lib or src directory. meta Issues and PRs related to the general management of the project. v4.x v8 engine Issues and PRs related to the V8 dependency. labels Apr 19, 2017
@MylesBorins
Copy link
ContributorAuthor

MylesBorins commented Apr 19, 2017

@MylesBorins
Copy link
ContributorAuthor

/cc @nodejs/platform-windows looks like something wrong here too

@mscdexmscdex removed build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. lib / src Issues and PRs related to general changes in the lib or src directory. v8 engine Issues and PRs related to the V8 dependency. labels Apr 19, 2017
@refack
Copy link
Contributor

Same thing #11863

@sam-github
Copy link
Contributor

@MylesBorins There are about 90 missing PRs from v6.x, where missing means the PR isn't labelled as dont-land, see https://gist.github.com/sam-github/eee701f571b54fca3acc46a3cb53e150 (this excludes all majors and minors, meta, etc.).

Is this WIP, or is dont-land not being applied consistently?

@MylesBorins
Copy link
ContributorAuthor

MylesBorins commented Apr 20, 2017

@sam-github this is a maintenance release... no longer doing active backporting. I only backported breaking bugs

edit: I am no longer applying do-not-land as there is not an active backporting / audit for v4 anymore. Landing things as requested or when I see breaking changes that need it

@sam-github
Copy link
Contributor

@addaleax searching around for the docs you are adding for LTS labels right now, that info in that last comment should be added (that the labels stop being applied when a release goes into maintenance - as opposed to when a release goes out of support).

@addaleax
Copy link
Member

@sam-github done! (#12431)

#11863 adds _O_RDWR to node.cc which is defined in fcntl.h. This adds this include directly to node.cc. PR-URL: #12540 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
@MylesBorins
Copy link
ContributorAuthor

@MylesBorins
Copy link
ContributorAuthor

@MylesBorins
Copy link
ContributorAuthor

@MylesBorins
Copy link
ContributorAuthor

CI is good... citgm is really sad

v4.8.2: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/747/
v4.8.3-rc.1: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/748/

@MylesBorins
Copy link
ContributorAuthor

MylesBorins commented Apr 29, 2017

Auditing citgm failures and comparing platforms.

fs-extra failures are omitted as they were not related to release and are now fixed in citgm

aix61-ppc64

  • All failures are install based or unchanged since last release EDIT(gib): not supported for Node 4

ubuntu1604-64

  • readable stream is infra failure on our end
  • uglify-js passes on a fresh ubuntu-1604 box

win-vs2015

  • All failures are timeouts or the same as prior release

debian8-64

  • watchify ~ passes on fresh debian 8 install
  • mkdirp ~ passes on fresh box
  • radium ~ phantom failing on fresh box

fedora23 / fedora 22

  • ember-cli ~ failures are timeout related

Based on the above results I'm willing to sign off on this release. @nodejs/build I would really like to dig into why our infra is causing these specific failures

Competing timers were causing a race condition and thus the test was flaky. Instead, we check an object property on process exit. Fixes: #7650 Backport-PR-URL: #12567 PR-URL: #7857 Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: jasnell - James M Snell <[email protected]>
@MylesBorinsMylesBorinsforce-pushed the v4.8.3-proposal branch 2 times, most recently from d7126df to 049b4eeCompareMay 2, 2017 15:45
Notable Changes: * module: - The module loading global fallback to the Node executable's directory now works correctly on Windows. (Richard Lau) #9283 * src: - fix base64 decoding in rare edgecase (Nikolai Vavilov) #11995 * tls: - fix rare segmentation faults when using TLS * (Trevor Norris) #11947 * (Ben Noordhuis) #11898 * (jBarz) #11776 PR-URL: #12499
@MylesBorinsMylesBorins merged commit 788f280 into v4.xMay 2, 2017
MylesBorins added a commit that referenced this pull request May 2, 2017
MylesBorins added a commit that referenced this pull request May 2, 2017
Notable Changes: * module: - The module loading global fallback to the Node executable's directory now works correctly on Windows. (Richard Lau) #9283 * src: - fix base64 decoding in rare edgecase (Nikolai Vavilov) #11995 * tls: - fix rare segmentation faults when using TLS * (Trevor Norris) #11947 * (Ben Noordhuis) #11898 * (jBarz) #11776 PR-URL: #12499
imyller added a commit to imyller/meta-nodejs that referenced this pull request May 2, 2017
 Notable Changes: * module: - The module loading global fallback to the Node executable's directory now works correctly on Windows. (Richard Lau) nodejs/node#9283 * src: - fix base64 decoding in rare edgecase (Nikolai Vavilov) nodejs/node#11995 * tls: - fix rare segmentation faults when using TLS * (Trevor Norris) nodejs/node#11947 * (Ben Noordhuis) nodejs/node#11898 * (jBarz) nodejs/node#11776 PR-URL: nodejs/node#12499 Signed-off-by: Ilkka Myller <[email protected]>
imyller added a commit to imyller/meta-nodejs that referenced this pull request May 2, 2017
 Notable Changes: * module: - The module loading global fallback to the Node executable's directory now works correctly on Windows. (Richard Lau) nodejs/node#9283 * src: - fix base64 decoding in rare edgecase (Nikolai Vavilov) nodejs/node#11995 * tls: - fix rare segmentation faults when using TLS * (Trevor Norris) nodejs/node#11947 * (Ben Noordhuis) nodejs/node#11898 * (jBarz) nodejs/node#11776 PR-URL: nodejs/node#12499 Signed-off-by: Ilkka Myller <[email protected]>
@gibfahngibfahn deleted the v4.8.3-proposal branch May 2, 2017 22:39
anchnk pushed a commit to anchnk/node that referenced this pull request May 6, 2017
Notable Changes: * module: - The module loading global fallback to the Node executable's directory now works correctly on Windows. (Richard Lau) nodejs#9283 * src: - fix base64 decoding in rare edgecase (Nikolai Vavilov) nodejs#11995 * tls: - fix rare segmentation faults when using TLS * (Trevor Norris) nodejs#11947 * (Ben Noordhuis) nodejs#11898 * (jBarz) nodejs#11776 PR-URL: nodejs#12499
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metaIssues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

18 participants

@MylesBorins@refack@sam-github@addaleax@mscdex@nodejs-github-bot@lekoder@mhdawson@richardlau@jBarz@trevnorris@bnoordhuis@ofrobots@bzoz@danbev@seishun@Trott