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

crypto: return this in setAuthTag/setAAD #9398

Closed
wants to merge 1 commit into from

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Nov 1, 2016

Checklist
  • make -j8 test (UNIX)
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

crypto

Description of change

Allow method chaining as with setAutoPadding and other methods.

@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Nov 1, 2016
@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 1, 2016
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -139,3 +139,14 @@ testCipher2(Buffer.from('0123456789abcdef'));
assert.doesNotThrow(() => txt += decipher.final('utf-16le'));
assert.strictEqual(txt, plaintext, 'decrypted result in utf-16le');
}

// setAutoPadding/setAuthTag/setAAD should return this
Copy link
Member

@addaleax addaleax Nov 1, 2016

Choose a reason for hiding this comment

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

Can I suggest that you enclose this in backticks or something? Otherwise the “this” reads like a demonstrative pronoun in this sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@addaleax updated

Copy link
Member

Choose a reason for hiding this comment

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

@fanatid thanks!

@fanatid fanatid force-pushed the crypto/return-this branch from 521e065 to 32a46cc Compare November 1, 2016 12:33
@cjihrig
Copy link
Contributor

cjihrig commented Nov 1, 2016

@sam-github
Copy link
Contributor

LGTM

@mscdex
Copy link
Contributor

mscdex commented Nov 1, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Nov 2, 2016

Docs should be updated also.

Allow method chaining as with setAutoPadding and other methods.
@fanatid fanatid force-pushed the crypto/return-this branch from 32a46cc to e22693d Compare November 2, 2016 17:05
@fanatid
Copy link
Contributor Author

fanatid commented Nov 2, 2016

@jasnell docs updated

@addaleax
Copy link
Member

Landed in 6b86ecc, thanks for the PR!

@addaleax addaleax closed this Nov 20, 2016
addaleax pushed a commit that referenced this pull request Nov 20, 2016
Allow method chaining as with setAutoPadding and other methods.

PR-URL: #9398
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
Allow method chaining as with setAutoPadding and other methods.

PR-URL: #9398
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Nov 22, 2016
2 tasks
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
This is a security release impacting Windows 10 users.

Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
  - Fixed a potential buffer overflow when writing data to console on
Windows 10. (CVE-2016-9551)
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* tls: Fixed a memory leak when writes were queued on TLS connection
that was destroyed during handshake. (Fedor Indutny)
#9626
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
Fishrock123 added a commit that referenced this pull request Nov 22, 2016
This is a security release impacting Windows 10 users.

Notable changes:

* crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
`this`. (Kirill Fomichev) #9398
* dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
(Ben Noordhuis) #9296 &
#9296
* libuv: Upgrade to v1.10.1 (cjihrig)
#9647
  - Fixed a potential buffer overflow when writing data to console on
Windows 10. (CVE-2016-9551)
* process: Added a new `external` property to the data returned by
`memoryUsage()`. (Fedor Indutny)
#9587
* tls: Fixed a memory leak when writes were queued on TLS connection
that was destroyed during handshake. (Fedor Indutny)
#9626
* V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
#9697
* v8: The data returned by `getHeapStatistics()` now includes three new
fields: `malloced_memory`, `peak_malloced_memory`, and
`does_zap_garbage`. (Gareth Ellis)
#8610

PR-URL: #9745
imyller added a commit to imyller/meta-nodejs that referenced this pull request Nov 28, 2016
This is a security release impacting Windows 10 users.

    Notable changes:

    * crypto: The `Decipher` methods `setAuthTag()` and `setAAD` now return
    `this`. (Kirill Fomichev) nodejs/node#9398
    * dns: Implemented `{ttl: true}` for `resolve4()` and `resolve6()`.
    (Ben Noordhuis) nodejs/node#9296 &
    nodejs/node#9296
    * libuv: Upgrade to v1.10.1 (cjihrig)
    nodejs/node#9647
      - Fixed a potential buffer overflow when writing data to console on
    Windows 10. (CVE-2016-9551)
    * process: Added a new `external` property to the data returned by
    `memoryUsage()`. (Fedor Indutny)
    nodejs/node#9587
    * tls: Fixed a memory leak when writes were queued on TLS connection
    that was destroyed during handshake. (Fedor Indutny)
    nodejs/node#9626
    * V8 (dep): Upgrade to v5.4.500.43 (Michaël Zasso)
    nodejs/node#9697
    * v8: The data returned by `getHeapStatistics()` now includes three new
    fields: `malloced_memory`, `peak_malloced_memory`, and
    `does_zap_garbage`. (Gareth Ellis)
    nodejs/node#8610

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2017
Allow method chaining as with setAutoPadding and other methods.

PR-URL: #9398
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
Allow method chaining as with setAutoPadding and other methods.

PR-URL: #9398
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
MylesBorins added a commit that referenced this pull request Jun 6, 2017
This LTS release comes with 126 commits. This includes 40 which
are test related, 32 which are doc related, 12 which are
build / tool related and 4 commits which are updates to
dependencies.

Notable Changes:

* build:
  - support for building mips64el (nanxiongchao)
    #10991
* cluster:
  - disconnect() now returns a reference to the disconnected
    worker. (Sean Villars)
    #10019
* crypto:
  - ability to select cert store at runtime (Adam Majer)
    #8334
  - Use system CAs instead of using bundled ones (Adam Majer)
    #8334
  - The `Decipher` methods `setAuthTag()` and `setAAD` now return
    `this`. (Kirill Fomichev)
    #9398
  - adding support for OPENSSL_CONF again (Sam Roberts)
    #11006
  - make LazyTransform compabile with Streams1 (Matteo Collina)
    #12380
* deps:
  - upgrade libuv to 1.11.0 (cjihrig)
    #11094
  - upgrade libuv to 1.10.2 (cjihrig)
    #10717
  - upgrade libuv to 1.10.1 (cjihrig)
    #9647
  - upgrade libuv to 1.10.0 (cjihrig)
    #9267
* dns:
  - Implemented `{ttl: true}` for `resolve4()` and `resolve6()`
    (Ben Noordhuis)
    #9296
* process:
  - add NODE_NO_WARNINGS environment variable (cjihrig)
    #10842
* readline:
  - add option to stop duplicates in history (Danny Nemer)
    #2982
* src:
  - support "--" after "-e" as end-of-options (John Barboza)
    #10651
* tls:
  - new tls.TLSSocket() supports sec ctx options (Sam Roberts)
    #11005
  - Allow obvious key/passphrase combinations. (Sam Roberts)
    #10294

PR-URL: #13059
MylesBorins added a commit that referenced this pull request Jun 6, 2017
This LTS release comes with 126 commits. This includes 40 which
are test related, 32 which are doc related, 12 which are
build / tool related and 4 commits which are updates to
dependencies.

Notable Changes:

* build:
  - support for building mips64el (nanxiongchao)
    #10991
* cluster:
  - disconnect() now returns a reference to the disconnected
    worker. (Sean Villars)
    #10019
* crypto:
  - ability to select cert store at runtime (Adam Majer)
    #8334
  - Use system CAs instead of using bundled ones (Adam Majer)
    #8334
  - The `Decipher` methods `setAuthTag()` and `setAAD` now return
    `this`. (Kirill Fomichev)
    #9398
  - adding support for OPENSSL_CONF again (Sam Roberts)
    #11006
  - make LazyTransform compabile with Streams1 (Matteo Collina)
    #12380
* deps:
  - upgrade libuv to 1.11.0 (cjihrig)
    #11094
  - upgrade libuv to 1.10.2 (cjihrig)
    #10717
  - upgrade libuv to 1.10.1 (cjihrig)
    #9647
  - upgrade libuv to 1.10.0 (cjihrig)
    #9267
* dns:
  - Implemented `{ttl: true}` for `resolve4()` and `resolve6()`
    (Ben Noordhuis)
    #9296
* process:
  - add NODE_NO_WARNINGS environment variable (cjihrig)
    #10842
* readline:
  - add option to stop duplicates in history (Danny Nemer)
    #2982
* src:
  - support "--" after "-e" as end-of-options (John Barboza)
    #10651
* tls:
  - new tls.TLSSocket() supports sec ctx options (Sam Roberts)
    #11005
  - Allow obvious key/passphrase combinations. (Sam Roberts)
    #10294

PR-URL: #13059
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
Allow method chaining as with setAutoPadding and other methods.

PR-URL: nodejs/node#9398
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
This LTS release comes with 126 commits. This includes 40 which
are test related, 32 which are doc related, 12 which are
build / tool related and 4 commits which are updates to
dependencies.

Notable Changes:

* build:
  - support for building mips64el (nanxiongchao)
    nodejs/node#10991
* cluster:
  - disconnect() now returns a reference to the disconnected
    worker. (Sean Villars)
    nodejs/node#10019
* crypto:
  - ability to select cert store at runtime (Adam Majer)
    nodejs/node#8334
  - Use system CAs instead of using bundled ones (Adam Majer)
    nodejs/node#8334
  - The `Decipher` methods `setAuthTag()` and `setAAD` now return
    `this`. (Kirill Fomichev)
    nodejs/node#9398
  - adding support for OPENSSL_CONF again (Sam Roberts)
    nodejs/node#11006
  - make LazyTransform compabile with Streams1 (Matteo Collina)
    nodejs/node#12380
* deps:
  - upgrade libuv to 1.11.0 (cjihrig)
    nodejs/node#11094
  - upgrade libuv to 1.10.2 (cjihrig)
    nodejs/node#10717
  - upgrade libuv to 1.10.1 (cjihrig)
    nodejs/node#9647
  - upgrade libuv to 1.10.0 (cjihrig)
    nodejs/node#9267
* dns:
  - Implemented `{ttl: true}` for `resolve4()` and `resolve6()`
    (Ben Noordhuis)
    nodejs/node#9296
* process:
  - add NODE_NO_WARNINGS environment variable (cjihrig)
    nodejs/node#10842
* readline:
  - add option to stop duplicates in history (Danny Nemer)
    nodejs/node#2982
* src:
  - support "--" after "-e" as end-of-options (John Barboza)
    nodejs/node#10651
* tls:
  - new tls.TLSSocket() supports sec ctx options (Sam Roberts)
    nodejs/node#11005
  - Allow obvious key/passphrase combinations. (Sam Roberts)
    nodejs/node#10294

PR-URL: nodejs/node#13059
@fanatid fanatid deleted the crypto/return-this branch September 7, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants