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

buffer: backport new buffer constructor APIs by jasnell · Pull Request #5763 · nodejs/node · GitHub

/ node Public

buffer: backport new buffer constructor APIs - #5763

Closed
jasnell wants to merge 2 commits into
nodejs:v5.xfrom
jasnell:v5.x-new-buffer-api
ConversationCommits (2)ChecksFiles changed
Closed

buffer: backport new buffer constructor APIs#5763
jasnell wants to merge 2 commits into
nodejs:v5.xfrom
jasnell:v5.x-new-buffer-api

Conversation

jasnell commented Mar 17, 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 new Buffer.alloc(), Buffer.allocUnsafe(), and Buffer.from() APIs for v5.

Also included in this backport is the change that allows fill('') to zero-fill (as opposed to doing nothing) and the additional byteOffset and length arguments for Buffer(arrayBuffer) and
Buffer.from(arrayBuffer).

This backport includes the new test cases.

This backport does not update all of the internal uses of the existing Buffer() constructor.

This backport also does not include the soft deprecation of the existing Buffer() constructor.

Refs: #5744
/cc: @Fishrock123 @trevnorris @nodejs/ctc @nodejs/lts

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 17, 2016
jasnell reviewed Mar 17, 2016
View reviewed changes
Comment thread lib/buffer.js Outdated

jasnell Mar 17, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Note: this particular bit is a semver-major change that was landed in master. I included it in the backport to ensure that the new api behavior in master would match what is backported. I can pull this new throw back out, however.

Fishrock123 Mar 17, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yeah let's keep this purely additive

jasnell Mar 17, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, pulled that throw back out and another. It's still not 100% "purely additive" in that it includes the change to fill('') to ensure it zero-fills as opposed to doing nothing.

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member Author

@nodejs/collaborators ... ping

trevnorris commented Mar 21, 2016

Copy link
Copy Markdown
Contributor

Don't remember the discussion about back porting to v5.x, but assuming we did. LGTM.

jasnell commented Mar 22, 2016

Copy link
Copy Markdown
Member Author

Adding this to CTC Agenda for 2016-03-23 just to make sure there are no @nodejs/ctc objections to backporting the API changes to v5 and v4 and the --zero-fill-buffers flag back to v5, v4, v0.12 and v0.10. The plan discussed by the LTS WG is that the the --zero-fill-buffers and API backports could be landed in the lts staging branches but that the releases wouldn't happen until around the time v6 is cut.

rvagg mentioned this pull request Mar 23, 2016
Closed

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

Discussion on the CTC was to go ahead with in v5 but to hold off on backporting further back.

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

@nodejs/ctc @nodejs/collaborators ... have one LGTM on this and no objections from CTC. However, it would be good to get some additional sign off. I'll give it 24 more hours for review then land tomorrow.

jasnell added 2 commits March 23, 2016 14:49
buffer: backport new buffer constructor APIs
ad5e145 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
and `Buffer.from()` APIs for v5.

Also included in this backport is the change that allows fill('')
to zero-fill (as opposed to doing nothing) and the additional
`byteOffset` and `length` arguments for `Buffer(arrayBuffer)` and
`Buffer.from(arrayBuffer)`.

This backport includes the new test cases.

This backport *does not* update all of the internal uses of the
existing `Buffer()` constructor.

This backport also *does not* include the soft deprecation of the
existing `Buffer()` constructor.
Pull out new throws
c04c7bd
jasnell force-pushed the v5.x-new-buffer-api branch from 6c5bc34 to c04c7bd Compare March 23, 2016 21:49

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

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

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 new buffer constructor APIs
c1534e7 
This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`,
and `Buffer.from()` APIs for v5.

Also included in this backport is the change that allows fill('')
to zero-fill (as opposed to doing nothing) and the additional
`byteOffset` and `length` arguments for `Buffer(arrayBuffer)` and
`Buffer.from(arrayBuffer)`.

This backport includes the new test cases.

This backport *does not* update all of the internal uses of the
existing `Buffer()` constructor.

This backport also *does not* include the soft deprecation of the
existing `Buffer()` constructor.

PR-URL: #5763
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

jasnell commented Mar 24, 2016

Copy link
Copy Markdown
Member Author

Landed in v5.x in c1534e7

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
c0b reviewed Apr 24, 2016
View reviewed changes
Comment thread doc/api/buffer.markdown

```js
const buf = new Buffer([1, 2, 3]);
const buf = Buffer(.from[1, 2, 3]);

c0b Apr 24, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this seems to be a typo; should be Buffer.from([1, 2, 3]); ??

ChALkeR commented May 5, 2016

Copy link
Copy Markdown
Member

@jasnell This missed the #5833 (well, because this PR was created earilier), this creates a minor inconsistency between 5.x and 6.x in the new API.

ChALkeR mentioned this pull request May 5, 2016
Closed
4 tasks

jasnell commented May 5, 2016

Copy link
Copy Markdown
Member Author

Yeah, as far as I know #5833 hasn't been backported yet to v5. I'll open a PR for that soon

feross commented May 30, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

@jasnell Any chance we can also backport to Node.js v4? Once v0.10 and and v0.12 are unsupported at the end of the year, it would be nice to start advising people to use only the new Buffer APIs (Buffer.from, etc.) in new code.

If we have support in v4, that would make shims like safe-buffer unnecessary and make it a no-brainer best practice.

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.

6 participants

Back | FazBrowse Home | New Git URL