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

Release proposal: v4.1.1 #2995

Merged
merged 1 commit into from
Sep 23, 2015
Merged

Release proposal: v4.1.1 #2995

merged 1 commit into from
Sep 23, 2015

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Sep 22, 2015

2015-09-22, Version 4.1.1 (Stable), @rvagg

Notable changes

  • buffer: Fixed a bug introduced in v4.1.0 where allocating a new zero-length buffer can result in the next allocation of a TypedArray in JavaScript not being zero-filled. In certain circumstances this could result in data leakage via reuse of memory space in TypedArrays, breaking the normally safe assumption that TypedArrays should be always zero-filled. (Trevor Norris) #2931.
  • http: Guard against response-splitting of HTTP trailing headers added via response.addTrailers() by removing new-line ([\r\n]) characters from values. Note that standard header values are already stripped of new-line characters. The expected security impact is low because trailing headers are rarely used. (Ben Noordhuis) #2945.
  • npm: Upgrade to npm 2.14.4 from 2.14.3, see release notes for full details (Kat Marchán) #2958
    • Upgrades graceful-fs on multiple dependencies to no longer rely on monkey-patching fs
    • Fix npm link for pre-release / RC builds of Node
  • v8: Update post-mortem metadata to allow post-mortem debugging tools to find and inspect:
    • JavaScript objects that use dictionary properties (Julien Gilli) #2959
    • ScopeInfo and thus closures (Julien Gilli) #2974

Known issues

See confirmed-bug Issues with confirmed bugs. for complete and current list of known issues.

  • Some problems with unreferenced timers running during beforeExit are still to be resolved. See #1264.
  • Surrogate pair in REPL can freeze terminal. #690
  • Calling dns.setServers() while a DNS query is in progress can cause the process to crash on a failed assertion. #894
  • url.resolve may transfer the auth portion of the url when resolving between two full hosts, see #1435.

Commits

  • [d63e02e08d] - buffer: don't set zero fill for zero-length buffer (Trevor Norris) #2931
  • [5905b14bff] - build: fix icutrim when building small-icu on BE (Stewart Addison) #2602
  • [f010cb5d96] - configure: detect mipsel host (Jérémy Lal) #2971
  • [b93ad5abbd] - deps: backport 357e6b9 from V8's upstream (Julien Gilli) #2974
  • [8da3da4d41] - deps: backport ff7d70b from V8's upstream (Julien Gilli) #2959
  • [2600fb8ae6] - deps: upgraded to node-gyp@3.0.3 in npm (Kat Marchán) #2958
  • [793aad2d7a] - deps: upgrade to npm 2.14.4 (Kat Marchán) #2958
  • [43e2b7f836] - doc: remove usage of events.EventEmitter (Sakthipriyan Vairamani) #2921
  • [9c59d2f16a] - doc: remove extra using v8::HandleScope statement (Christopher J. Brody) #2983
  • [f7edbab367] - doc: clarify description of assert.ifError() (Rich Trott) #2941
  • [b2ddf0f9a2] - doc: refine process.kill() and exit explanations (Rich Trott) #2918
  • [f68fed2e6f] - http: remove redundant code in _deferToConnect (Malcolm Ahoy) #2769
  • [f542e74c93] - http: guard against response splitting in trailers (Ben Noordhuis) #2945
  • [bc9f629387] - http_parser: do not dealloc during kOnExecute (Fedor Indutny) #2956
  • [1860e0cebd] - lib,src: remove usage of events.EventEmitter (Sakthipriyan Vairamani) #2921
  • [d4cd5ac407] - readline: fix tab completion bug (Matt Harrison) #2816
  • [9760e04839] - repl: don't use tty control codes when $TERM is set to "dumb" (Salman Aljammaz) #2712
  • [cb971cc97d] - repl: backslash bug fix (Sakthipriyan Vairamani) #2968
  • [2034f68668] - src: honor --abort_on_uncaught_exception flag (Evan Lucas) #2776
  • [0b1ca4a9ef] - src: Add ABORT macro (Evan Lucas) #2776
  • [4519dd00f9] - test: test sync version of mkdir & rmdir (Sakthipriyan Vairamani) #2588
  • [816f609c8b] - test: use tmpDir instead of fixtures in readdir (Sakthipriyan Vairamani) #2587
  • [2084f52585] - test: test more http response splitting scenarios (Ben Noordhuis) #2945
  • [fa08d1d8a1] - test: add test-spawn-cmd-named-pipe (Alexis Campailla) #2770
  • [71b5d80682] - test: make cluster tests more time tolerant (Michael Dawson) #2891
  • [3e09dcfc32] - test: update cwd-enoent tests for AIX (Imran Iqbal) #2909
  • [6ea8ec1c59] - tools: single, cross-platform tick processor (Matt Loring) #2868

Commits on master not included in v4.x:

  • [5f6579d366] - (SEMVER-MAJOR) buffer: remove raw & raws encoding (Sakthipriyan Vairamani) #2859
  • [b9813641dc] - 2015-09-15 io.js v3.3.1 Release (Rod Vagg) #2698
  • [380a3d89c3] - 2015-09-08, Version 4.0.0 (Stable) Release (Rod Vagg) #2742
  • [42a8a0a53e] - Working on v5.0.0 (Rod Vagg)

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

going to need some reviews in the notable changes, ping @trevnorris, @bnoordhuis, @ChALkeR, @zkat, @misterdjules

@rvagg rvagg mentioned this pull request Sep 22, 2015
@ChALkeR
Copy link
Member

ChALkeR commented Sep 22, 2015

http: Guard against response-splitting of HTTP headers by removing new-line characters from header names. Where user-supplied data is involved in the generation of header names, a malicious user may be able to craft responses that inject arbitrary data at the top of the body. Note that header values are already sanitized. (Ben Noordhuis) #2945.

Is wrong. What you describe is #2526, and #2945 removes \n and \r from the trailing HTTP headers values.

@rvagg rvagg force-pushed the v4.1.1-proposal branch 2 times, most recently from eb99301 to 35bfe82 Compare September 22, 2015 07:25
@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

Whoops, thanks @ChALkeR for pointing that out. It's now:

  • http: Guard against response-splitting of HTTP trailing headers by removing new-line ([\r\n]) characters from values. Note that standard header values are already stripped of new-line characters. (Ben Noordhuis) #2945.

@ChALkeR ChALkeR added the meta Issues and PRs related to the general management of the project. label Sep 22, 2015
@ChALkeR
Copy link
Member

ChALkeR commented Sep 22, 2015

@rvagg I believe that the actual commit wasn't changed. Check the README.md file.

@bnoordhuis
Copy link
Member

README so far LGTM.

@ChALkeR
Copy link
Member

ChALkeR commented Sep 22, 2015

Notable changes LGTM.

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

I think this is good to go, would still appreciate review of Notable items as it's kind of sensitive, we neither want to frighten people nor do we want them to take the two security-ish issues lightly.

Also, failure on ARMv6, which I believe is known flaky but I've been seeing it mentioned a lot lately and there may be something to investigate here.

not ok 98 test-stringbytes-external.js
  ---
  duration_ms: 208.602

... and 208 seconds?? we're supposed to have a 60s timeout.

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

reopened relevant issue on the failing test #2370

@ChALkeR
Copy link
Member

ChALkeR commented Sep 22, 2015

  • http: Guard against response-splitting of HTTP trailing headers by removing new-line ([\r\n]) characters from values. Note that standard header values are already stripped of new-line characters. (Ben Noordhuis) #2945.

You could mention this (possibly re-phrased) from #2945:

The expected security impact is low because approximately no one uses trailing headers. Some HTTP clients can't even parse them.

Or you could mention something like «added through addTrailers()», because that's the only entry point for those, and probably when someone will see it they'll think «addWhat? I don't use that anyway». The current Notable changles make it look like the problem was with something header-related, and it's not clear that it's in a thing that almost noone uses.

Most usages of .addTrailers() are wrappers for that API and tests. I could do a full list today, but it could be a bit late and it's probably not needed. Overall: the usage in real code is close to zero.

@ChALkeR
Copy link
Member

ChALkeR commented Sep 22, 2015

  • buffer: Fixed a bug introduced in v4.1.0 where allocating a new zero-length buffer can result in the next allocation of a TypedArray in JavaScript not being zero-filled. In certain circumstances this could result in data leakage via reuse of memory space in TypedArrays, breaking the normally safe assumption that TypedArrays should be always zero-filled. (Trevor Norris) #2931.

«Next allocation» it is not actually a valid argument here (you could still keep it, though, because the statement is still valid, it just doesn't make this issue considerably less important), but the fact that people usually don't leak TypedArrays or don't rely on it being zero-filled is.

@silverwind
Copy link
Contributor

... and 208 seconds?? we're supposed to have a 60s timeout.

The timeout is upscaled to 180s on ARMv6 in the release setting. Were those test ran as test.py --mode=release?

On another note, I landed fcfd87d which is a pretty important fix imho, if you're still cherry-picking.

@misterdjules
Copy link

@rvagg I'm not sure the V8 changes are "notable", in the sense that it's worth knowing only if you're building a debugging tool that reads V8's internal data structures. If you'd still like to include it, I would suggest to change it slightly:

"Update post-mortem metadata to allow post-mortem debugging tools to find and inspect:

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

updated commits from master they were all fairly trivial so I'm comfortable with the last minute additions

tweaked the http entry to add more clarity about trailers, tweaked the v8 entry to put more detail, it's more technical than we'd normally include but I did think it worthwhile showing that this is being worked on because there are a small set of users for which this is very important.

@ChALkeR I'm not sure what your suggestion is re the TypedArray comment so I haven't changed anything

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

moving straight to release after this so get your feedback in re CHANGELOG

@misterdjules
Copy link

@rvagg I thought that #2959 and #2974 were mentioned previously as part of this release, did you remove them?

@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

sorry @misterdjules! I switched branches just before copypasta and ended up dumping in the 4.1.0 notes in the OP, updated now with 4.1.1 including those 2 PRs using your words

@misterdjules
Copy link

@rvagg OK, I was confused for a second, thank you 👍 :)

Notable changes

* buffer: Fixed a bug introduced in v4.1.0 where allocating a new
  zero-length buffer can result in the next allocation of a TypedArray
  in JavaScript not being zero-filled. In certain circumstances this
  could result in data leakage via reuse of memory space in
  TypedArrays, breaking the normally safe assumption that TypedArrays
  should be always zero-filled. (Trevor Norris) #2931.
* http: Guard against response-splitting of HTTP trailing headers
  added via response.addTrailers() by removing new-line ([\r\n])
  characters from values. Note that standard header values are already
  stripped of new-line characters. The expected security impact is low
  because trailing headers are rarely used. (Ben Noordhuis) #2945.
* npm: Upgrade to npm 2.14.4 from 2.14.3, see release notes for full
  details (Kat Marchán) #2958
  - Upgrades graceful-fs on multiple dependencies to no longer rely on
    monkey-patching fs
  - Fix npm link for pre-release / RC builds of Node
* v8: Update post-mortem metadata to allow post-mortem debugging tools
  to find and inspect:
  - JavaScript objects that use dictionary properties
    (Julien Gilli) #2959
  - ScopeInfo and thus closures (Julien Gilli) #2974
@rvagg
Copy link
Member Author

rvagg commented Sep 22, 2015

Going with ab55b45 as the HEAD for this release, although I've started builds @ https://ci.nodejs.org/job/iojs+release/183/ it looks like we might be in for at least a minor repeat of last week's build dramas with 4.1.0, armv8 has already borked (thanks Jenkins and Java).

@rvagg
Copy link
Member Author

rvagg commented Sep 23, 2015

https://ci.nodejs.org/job/iojs+release/191/ this one's looking promising, so much fail with ICU and Jenkins

@rvagg rvagg merged commit ab55b45 into v4.x Sep 23, 2015
@rvagg rvagg deleted the v4.1.1-proposal branch September 23, 2015 01:33
rvagg added a commit that referenced this pull request Sep 23, 2015
Notable changes

* buffer: Fixed a bug introduced in v4.1.0 where allocating a new
  zero-length buffer can result in the next allocation of a TypedArray
  in JavaScript not being zero-filled. In certain circumstances this
  could result in data leakage via reuse of memory space in
  TypedArrays, breaking the normally safe assumption that TypedArrays
  should be always zero-filled. (Trevor Norris) #2931.
* http: Guard against response-splitting of HTTP trailing headers
  added via response.addTrailers() by removing new-line ([\r\n])
  characters from values. Note that standard header values are already
  stripped of new-line characters. The expected security impact is low
  because trailing headers are rarely used. (Ben Noordhuis) #2945.
* npm: Upgrade to npm 2.14.4 from 2.14.3, see release notes for full
  details (Kat Marchán) #2958
  - Upgrades graceful-fs on multiple dependencies to no longer rely on
    monkey-patching fs
  - Fix npm link for pre-release / RC builds of Node
* v8: Update post-mortem metadata to allow post-mortem debugging tools
  to find and inspect:
  - JavaScript objects that use dictionary properties
    (Julien Gilli) #2959
  - ScopeInfo and thus closures (Julien Gilli) #2974

PR-URL: #2995
@rvagg
Copy link
Member Author

rvagg commented Sep 23, 2015

thanks all, v4.1.1 is live

@silverwind
Copy link
Contributor

Looks like the ARMv6 binaries need to be promoted.

@rvagg
Copy link
Member Author

rvagg commented Sep 24, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants