Home Original page

stream,net: slightly more consistent typechecking by addaleax · Pull Request #17644 · nodejs/node

added 2 commits

December 13, 2017 07:34
`validChunk` allowed `undefined` as a chunk in object mode; however,
this was redundant, since:

- `validChunk()` is only used by `.write()`
- If the `validChunk()` check passes for `undefined`, `.write()`
  calls `writeOrBuffer()`
- If `writeOrBuffer()` does not receive a Buffer, it calls
  `decodeChunk()`
  - `decodeChunk()` ignores `undefined` because it checks
    `typeof chunk === 'string'`
  - After that call, `chunk.length` is accessed, which throws an
    error if `chunk` is undefined`.

This “fixes” a bug in the sense that `state.pendingcb` is no longer
incremented for write attempts that fail like this.
This is superfluous now that typechecking in `net` and
`stream` are aligned.

BridgeAR

apapirovski

maclover7

@addaleax addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 15, 2017

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request

Dec 15, 2017 
`validChunk` allowed `undefined` as a chunk in object mode; however,
this was redundant, since:

- `validChunk()` is only used by `.write()`
- If the `validChunk()` check passes for `undefined`, `.write()`
  calls `writeOrBuffer()`
- If `writeOrBuffer()` does not receive a Buffer, it calls
  `decodeChunk()`
  - `decodeChunk()` ignores `undefined` because it checks
    `typeof chunk === 'string'`
  - After that call, `chunk.length` is accessed, which throws an
    error if `chunk` is undefined`.

This “fixes” a bug in the sense that `state.pendingcb` is no longer
incremented for write attempts that fail like this.

PR-URL: nodejs#17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request

Dec 15, 2017 
This is superfluous now that typechecking in `net` and
`stream` are aligned.

PR-URL: nodejs#17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

MylesBorins pushed a commit that referenced this pull request

Jan 8, 2018 
`validChunk` allowed `undefined` as a chunk in object mode; however,
this was redundant, since:

- `validChunk()` is only used by `.write()`
- If the `validChunk()` check passes for `undefined`, `.write()`
  calls `writeOrBuffer()`
- If `writeOrBuffer()` does not receive a Buffer, it calls
  `decodeChunk()`
  - `decodeChunk()` ignores `undefined` because it checks
    `typeof chunk === 'string'`
  - After that call, `chunk.length` is accessed, which throws an
    error if `chunk` is undefined`.

This “fixes” a bug in the sense that `state.pendingcb` is no longer
incremented for write attempts that fail like this.

PR-URL: #17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

MylesBorins pushed a commit that referenced this pull request

Jan 8, 2018 
This is superfluous now that typechecking in `net` and
`stream` are aligned.

PR-URL: #17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

MylesBorins pushed a commit that referenced this pull request

Jan 9, 2018 
`validChunk` allowed `undefined` as a chunk in object mode; however,
this was redundant, since:

- `validChunk()` is only used by `.write()`
- If the `validChunk()` check passes for `undefined`, `.write()`
  calls `writeOrBuffer()`
- If `writeOrBuffer()` does not receive a Buffer, it calls
  `decodeChunk()`
  - `decodeChunk()` ignores `undefined` because it checks
    `typeof chunk === 'string'`
  - After that call, `chunk.length` is accessed, which throws an
    error if `chunk` is undefined`.

This “fixes” a bug in the sense that `state.pendingcb` is no longer
incremented for write attempts that fail like this.

PR-URL: #17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

MylesBorins pushed a commit that referenced this pull request

Jan 9, 2018 
This is superfluous now that typechecking in `net` and
`stream` are aligned.

PR-URL: #17644
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

MylesBorins added a commit that referenced this pull request

Jan 10, 2018 
Notable change:

* async_hooks:
  - deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
    API were documented. (Andreas Madsen)
    #16972
* deps:
  - update nghttp2 to 1.29.0 (James M Snell)
    #17908
  - upgrade npm to 5.6.0 (Kat Marchán)
    #17535
  - cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
    #16591
* events:
  - remove reaches into _events internals (Anatoli Papirovski)
    #17440
* http:
  - add rawPacket in err of `clientError` event (XadillaX)
    #17672
* http2:
  - implement maxSessionMemory (James M Snell)
    #17967
  - add initial support for originSet (James M Snell)
    #17935
  - add altsvc support (James M Snell)
    #17917
  - perf_hooks integration (James M Snell)
    #17906
* net:
  - remove Socket.prototype.write (Anna Henningsen)
    #17644
  - remove Socket.prototype.listen (Ruben Bridgewater)
    #13735
* repl:
  - show lexically scoped vars in tab completion (Michaël Zasso)
    #16591
* stream:
  - rm {writeable/readable}State.length (Calvin Metcalf)
    #12857
  - add flow and buffer properties to streams (Calvin Metcalf)
    #12855
* util:
  - allow wildcards in NODE_DEBUG variable (Tyler)
    #17609
* zlib:
  - add ArrayBuffer support (Jem Bezooyen)
    #16042
* Addedew collaborator**
  - [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
  - [danbev](https://github.com/danbev) Daniel Bevenius

PR-URL: #18069

MylesBorins added a commit that referenced this pull request

Jan 10, 2018 
Notable change:

* async_hooks:
  - deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
    API were documented. (Andreas Madsen)
    #16972
* deps:
  - update nghttp2 to 1.29.0 (James M Snell)
    #17908
  - upgrade npm to 5.6.0 (Kat Marchán)
    #17535
  - cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
    #16591
* events:
  - remove reaches into _events internals (Anatoli Papirovski)
    #17440
* http:
  - add rawPacket in err of `clientError` event (XadillaX)
    #17672
* http2:
  - implement maxSessionMemory (James M Snell)
    #17967
  - add initial support for originSet (James M Snell)
    #17935
  - add altsvc support (James M Snell)
    #17917
  - perf_hooks integration (James M Snell)
    #17906
  - Refactoring and cleanup of Http2Session and Http2Stream destroy
    (James M Snell) #17406
* net:
  - remove Socket.prototype.write (Anna Henningsen)
    #17644
  - remove Socket.prototype.listen (Ruben Bridgewater)
    #13735
* repl:
  - show lexically scoped vars in tab completion (Michaël Zasso)
    #16591
* stream:
  - rm {writeable/readable}State.length (Calvin Metcalf)
    #12857
  - add flow and buffer properties to streams (Calvin Metcalf)
    #12855
* util:
  - allow wildcards in NODE_DEBUG variable (Tyler)
    #17609
* zlib:
  - add ArrayBuffer support (Jem Bezooyen)
    #16042
* Addedew collaborator**
  - [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
  - [danbev](https://github.com/danbev) Daniel Bevenius

PR-URL: #18069

MylesBorins added a commit that referenced this pull request

Jan 10, 2018 
Notable change:

* async_hooks:
  - deprecate AsyncHooks Sensitive API and runInAsyncIdScope. Neither
    API were documented. (Andreas Madsen)
    #16972
* deps:
  - update nghttp2 to 1.29.0 (James M Snell)
    #17908
  - upgrade npm to 5.6.0 (Kat Marchán)
    #17535
  - cherry-pick 50f7455 from upstream V8 (Michaël Zasso)
    #16591
* events:
  - remove reaches into _events internals (Anatoli Papirovski)
    #17440
* http:
  - add rawPacket in err of `clientError` event (XadillaX)
    #17672
* http2:
  - implement maxSessionMemory (James M Snell)
    #17967
  - add initial support for originSet (James M Snell)
    #17935
  - add altsvc support (James M Snell)
    #17917
  - perf_hooks integration (James M Snell)
    #17906
  - Refactoring and cleanup of Http2Session and Http2Stream destroy
    (James M Snell) #17406
* net:
  - remove Socket.prototype.write (Anna Henningsen)
    #17644
  - remove Socket.prototype.listen (Ruben Bridgewater)
    #13735
* repl:
  - show lexically scoped vars in tab completion (Michaël Zasso)
    #16591
* stream:
  - rm {writeable/readable}State.length (Calvin Metcalf)
    #12857
  - add flow and buffer properties to streams (Calvin Metcalf)
    #12855
* util:
  - allow wildcards in NODE_DEBUG variable (Tyler)
    #17609
* zlib:
  - add ArrayBuffer support (Jem Bezooyen)
    #16042
* Addedew collaborator**
  - [starkwang](https://github.com/starkwang) Weijia Wang
* Addedew TSC member**
  - [danbev](https://github.com/danbev) Daniel Bevenius

PR-URL: #18069