buffer: couple indexOf fixes by trevnorris · Pull Request #4803 · nodejs/node

trevnorris · 2016-01-21T21:08:17Z

Fix arguments to allow indexOf(val[, byteOffset][, encoding]) instead of forcing
indexOf(val[, byteOffset[, encoding]]) as it does today. This works in conjunction with doc: Document buffer.indexOf changes in v4.2.0 #3373 which adds a documentation update.
Fix binary encoding. Currently the needle is always decoded as utf8, which means extended latin-1 characters will increase the size by too many bytes and fail the search.

Tests have been added for both.

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

jasnell · 2016-01-21T22:35:26Z

LGTM

Currently the signature is indexOf(val[, byteOffset[, encoding]]) Instead allow indexOf(val[, byteOffset][, encoding]) so that byteOffset does not need to be passed. PR-URL: nodejs#4803 Reviewed-By: James M Snell <jasnell@gmail.com>

If the needle contains an extended latin-1 character then using String::Utf8Length() will be too large and the search will return early. Instead use String::Length() when encoding is BINARY. PR-URL: nodejs#4803 Reviewed-By: James M Snell <jasnell@gmail.com>

trevnorris · 2016-01-22T18:35:44Z

Landed in 7240ad4 and 54cd2e1. Thanks much!

rvagg · 2016-01-25T10:42:13Z

indexOf(val[, byteOffset][, encoding]) instead of forcing indexOf(val[, byteOffset[, encoding]])

That would be semver-minor wouldn't it? labelling it as such now but @trevnorris please remove it if you disagree strongly about this.

I went to the docs to check to see if this was a bugfix but we currently list it as ### buf.indexOf(value[, byteOffset])see here, but we do mention the encoding in Buffer#includes(). Perhaps this is asking for a doc update too?

trevnorris · 2016-01-26T10:03:50Z

@rvagg I saw this as only additive. While passing a non-number for byteOffset wouldn't throw, it's also documented as expecting a Number. The doc change recently landed (see #3373 ). Though I guess only bug-fix changes go into patch releases, so semver-minor makes sense.

Currently the signature is indexOf(val[, byteOffset[, encoding]]) Instead allow indexOf(val[, byteOffset][, encoding]) so that byteOffset does not need to be passed. PR-URL: #4803 Reviewed-By: James M Snell <jasnell@gmail.com>

If the needle contains an extended latin-1 character then using String::Utf8Length() will be too large and the search will return early. Instead use String::Length() when encoding is BINARY. PR-URL: #4803 Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins · 2016-02-17T19:54:37Z

@nodejs/lts is this something we want for v4.4.0?

/cc @trevnorris

trevnorris · 2016-02-18T10:34:54Z

@thealphanerd Yes. This does contain bug fixes, and nothing breaking.

* buffer: - You can now supply an encoding argument when filling a Buffer Buffer#fill(string[, start[, end]][, encoding]), supplying an existing Buffer will also work with Buffer#fill(buffer[, start[, end]]). See the API documentation for details on how this works. (Trevor Norris) #4935 - Buffer#indexOf() no longer requires a byteOffset argument if you also wish to specify an encoding: Buffer#indexOf(val[, byteOffset][, encoding]). (Trevor Norris) #4803 * child_process: spawn() and spawnSync() now support a 'shell' option to allow for optional execution of the given command inside a shell. If set to true, cmd.exe will be used on Windows and /bin/sh elsewhere. A path to a custom shell can also be passed to override these defaults. On Windows, this option allows .bat. and .cmd files to be executed with spawn() and spawnSync(). (Colin Ihrig) #4598 * http_parser: Update to http-parser 2.6.2 to fix an unintentionally strict limitation of allowable header characters. (James M Snell) #5237 * dgram: socket.send() now supports accepts an array of Buffers or Strings as the first argument. See the API docs for details on how this works. (Matteo Collina) #4374 * http: Fix a bug where handling headers will mistakenly trigger an 'upgrade' event where the server is just advertising its protocols. This bug can prevent HTTP clients from communicating with HTTP/2 enabled servers. (Fedor Indutny) #4337 * net: Added a listening Boolean property to net and http servers to indicate whether the server is listening for connections. (José Moreira) #4743 * node: The C++ node::MakeCallback() API is now reentrant and calling it from inside another MakeCallback() call no longer causes the nextTick queue or Promises microtask queue to be processed out of order. (Trevor Norris) #4507 * tls: Add a new tlsSocket.getProtocol() method to get the negotiated TLS protocol version of the current connection. (Brian White) #4995 * vm: Introduce new 'produceCachedData' and 'cachedData' options to new vm.Script() to interact with V8's code cache. When a new vm.Script object is created with the 'produceCachedData' set to true a Buffer with V8's code cache data will be produced and stored in cachedData property of the returned object. This data in turn may be supplied back to another vm.Script() object with a 'cachedData' option if the supplied source is the same. Successfully executing a script from cached data can speed up instantiation time. See the API docs for details. (Fedor Indutny) #4777 * performance: Improvements in: - process.nextTick() (Ruben Bridgewater) #5092 - path module (Brian White) #5123 - querystring module (Brian White) #5012 - streams module when processing small chunks (Matteo Collina) #4354

MylesBorins · 2016-02-22T20:42:55Z

@rvagg / @trevnorris I'll add this to the v4.4.0 rc next week after it has had some time living in v5.7.0

Please let me know if it should be hanging out longer before bringing it back

* buffer: - You can now supply an encoding argument when filling a Buffer Buffer#fill(string[, start[, end]][, encoding]), supplying an existing Buffer will also work with Buffer#fill(buffer[, start[, end]]). See the API documentation for details on how this works. (Trevor Norris) #4935 - Buffer#indexOf() no longer requires a byteOffset argument if you also wish to specify an encoding: Buffer#indexOf(val[, byteOffset][, encoding]). (Trevor Norris) #4803 * child_process: spawn() and spawnSync() now support a 'shell' option to allow for optional execution of the given command inside a shell. If set to true, cmd.exe will be used on Windows and /bin/sh elsewhere. A path to a custom shell can also be passed to override these defaults. On Windows, this option allows .bat. and .cmd files to be executed with spawn() and spawnSync(). (Colin Ihrig) #4598 * http_parser: Update to http-parser 2.6.2 to fix an unintentionally strict limitation of allowable header characters. (James M Snell) #5237 * dgram: socket.send() now supports accepts an array of Buffers or Strings as the first argument. See the API docs for details on how this works. (Matteo Collina) #4374 * http: Fix a bug where handling headers will mistakenly trigger an 'upgrade' event where the server is just advertising its protocols. This bug can prevent HTTP clients from communicating with HTTP/2 enabled servers. (Fedor Indutny) #4337 * net: Added a listening Boolean property to net and http servers to indicate whether the server is listening for connections. (José Moreira) #4743 * node: The C++ node::MakeCallback() API is now reentrant and calling it from inside another MakeCallback() call no longer causes the nextTick queue or Promises microtask queue to be processed out of order. (Trevor Norris) #4507 * tls: Add a new tlsSocket.getProtocol() method to get the negotiated TLS protocol version of the current connection. (Brian White) #4995 * vm: Introduce new 'produceCachedData' and 'cachedData' options to new vm.Script() to interact with V8's code cache. When a new vm.Script object is created with the 'produceCachedData' set to true a Buffer with V8's code cache data will be produced and stored in cachedData property of the returned object. This data in turn may be supplied back to another vm.Script() object with a 'cachedData' option if the supplied source is the same. Successfully executing a script from cached data can speed up instantiation time. See the API docs for details. (Fedor Indutny) #4777 * performance: Improvements in: - process.nextTick() (Ruben Bridgewater) #5092 - path module (Brian White) #5123 - querystring module (Brian White) #5012 - streams module when processing small chunks (Matteo Collina) #4354 PR-URL: #5295