FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer: backport --zero-fill-buffers command line option by jasnell · Pull Request #5744 · nodejs/node · GitHub

/ node Public

buffer: backport --zero-fill-buffers command line option - #5744

Closed
jasnell wants to merge 1 commit into
nodejs:v5.xfrom
jasnell:v5.x-zero-fill-buffers
ConversationCommits (1)ChecksFiles changed
Closed

buffer: backport --zero-fill-buffers command line option#5744
jasnell wants to merge 1 commit into
nodejs:v5.xfrom
jasnell:v5.x-zero-fill-buffers

Conversation

jasnell commented Mar 16, 2016

Copy link
Copy Markdown
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

buffer

Description of change

This backports the --zero-fill-buffers command line flag introduced in master. When used, all Buffer and SlowBuffer instances will zero fill by default.

This does not backport any of the other Buffer API or behavior changes.

Note: My intent is to open similar backports for --zero-fill-buffer in v4, v0.12 and v0.10

/cc @trevnorris

jasnell added buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version. v5.x labels Mar 16, 2016

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

@nodejs/ctc

Fishrock123 commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

Uh, why not land the actual patches with the new APIs?

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

Because it's technically a semver-major even tho it's a soft deprecation and there's been no decision about pulling all of the APIs back. I'm certainly open to the discussion but we'd need to decide how far back to backport

ChALkeR commented Mar 17, 2016

Copy link
Copy Markdown
Member

We can land those without the soft deprecation, but it should be discussed in a separate issue, probably.

+1 to these PRs that implement the --zero-fill-buffers flag (for all versions). I did not yet review the code, will do that later today.

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

I'm +1 with backporting the new APIs to v5 without the soft deprecation.
The @nodejs/lts WG and @nodejs/ctc would need to discuss and decide if it makes sense to backport the new APIs to v4/v0.12/v0.10.

At the very least, however, let's get the --zero-fill-buffers flag landed in each.

Fishrock123 commented Mar 17, 2016

Copy link
Copy Markdown
Contributor

Everything except the deprecation should (i.e. needs to) land in v5.x -- the deprecation should have been in a separate commit.

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

That would be easy enough to do. I'll get the v5 PR updated this morning.
Any opinions about backporting the APIs to v4?
On Mar 17, 2016 7:10 AM, "Jeremiah Senkpiel" notifications@github.com
wrote:

Everything except the deprecation should (i.e. needs to) land in v5.x --
the deprecation should have been in a separate commit.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#5744 (comment)

ChALkeR commented Mar 17, 2016

Copy link
Copy Markdown
Member

@jasnell it would be good to include the new API in the next v4 minor, imo.

jasnell commented Mar 17, 2016

Copy link
Copy Markdown
Member Author

Adding lts-agenda so the @nodejs/lts WG can discuss backporting the APIs to v4

jasnell mentioned this pull request Mar 17, 2016
Closed
4 tasks

ChALkeR commented Mar 19, 2016

Copy link
Copy Markdown
Member

LGTM

1 similar comment

trevnorris commented Mar 19, 2016

Copy link
Copy Markdown
Contributor

LGTM

jasnell mentioned this pull request Mar 21, 2016
Closed

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member Author

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

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

Discussion by the CTC today was to get this landed in v5 but hold off backporting further.

buffer: backport --zero-fill-buffers command line option
c64d0b0 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.
jasnell force-pushed the v5.x-zero-fill-buffers branch from b30fefd to c64d0b0 Compare March 23, 2016 21:46

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

@Fishrock123 ... updated this to fix up doc/node.1 and doc/api/cli.markdown. PTAL

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

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

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

CI is green except for one unrelated failure.

jasnell added a commit that referenced this pull request Mar 24, 2016
buffer: backport --zero-fill-buffers command line option
3c02727 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.

PR-URL: #5744
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

jasnell commented Mar 24, 2016

Copy link
Copy Markdown
Member Author

Landed in v5.x in 3c02727

jasnell closed this Mar 24, 2016
evanlucas mentioned this pull request Mar 31, 2016
Closed
evanlucas added a commit that referenced this pull request Mar 31, 2016
2016-03-31 Version 5.10.0 (Stable) Release
9cd6e98 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. (Forrest L Norvell)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Mar 31, 2016
2016-03-31 Version 5.10.0 (Stable) Release
1bb2c4b 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Apr 1, 2016
2016-03-31 Version 5.10.0 (Stable) Release
79ea8c3 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)
evanlucas added a commit that referenced this pull request Apr 1, 2016
2016-03-31 Version 5.10.0 (Stable) Release
64c87e2 
Notable changes:

* buffer:
  * make byteLength work with ArrayBuffer & DataView (Jackson Tian)
[#5255](#5255)
  * backport --zero-fill-buffers command line option (James M Snell)
[#5744](#5744)
  * backport new buffer constructor APIs (James M Snell)
[#5763](#5763)
  * add swap16() and swap32() methods (James M Snell)
[#5724](#5724)
* fs: add the fs.mkdtemp() function. (Florian MARGAINE)
[#5333](#5333)
* net: emit host in lookup event (HUANG Wei)
[#5598](#5598)
* node: --no-browser-globals configure flag (Fedor Indutny)
[#5853](#5853)
* npm: Upgrade to v3.8.3. Fixes a security flaw in the use of
authentication tokens in HTTP requests that would allow an attacker to
set up a server that could collect tokens from users of the command-line
interface. Authentication tokens have previously been sent with every
request made by the CLI for logged-in users, regardless of the destination
of the request. This update fixes this by only including those tokens
for requests made against the registry or registries used for the
current install. (Forrest L Norvell)
[npm#6](npm#6)
* repl: support standalone blocks (Prince J Wesley)
[#5581](#5581)
* src: override v8 thread defaults using cli options (Tom Gallacher)
[#4344](#4344)

PR-URL: #5970
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL