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: fix randomInt range check #35052

Closed

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Sep 4, 2020

The current implementation allows crypto.randomInt(0) and returns NaN.

Refs: #34600

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Sep 4, 2020
@tniessen tniessen mentioned this pull request Sep 4, 2020
4 tasks
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2020
@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 4, 2020
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2020

Commit Queue failed
- Loading data for nodejs/node/pull/35052
✔  Done loading data for nodejs/node/pull/35052
----------------------------------- PR info ------------------------------------
Title      crypto: fix randomInt range check (#35052)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:crypto-fix-randomint-range-check -> nodejs:master
Labels     author ready, crypto
Commits    2
 - crypto: fix randomInt range check
 - fixup! crypto: fix randomInt range check
Committers 1
 - Tobias Nießen 
PR-URL: https://github.com/nodejs/node/pull/35052
Refs: https://github.com/nodejs/node/pull/34600
Reviewed-By: Richard Lau 
Reviewed-By: Denys Otrishko 
Reviewed-By: Colin Ihrig 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/35052
Refs: https://github.com/nodejs/node/pull/34600
Reviewed-By: Richard Lau 
Reviewed-By: Denys Otrishko 
Reviewed-By: Colin Ihrig 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fixup! crypto: fix randomInt range check
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-09-07T10:54:14Z: https://ci.nodejs.org/job/node-test-pull-request/33088/
- Querying data for job/node-test-pull-request/33088/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Fri, 04 Sep 2020 10:18:12 GMT
   ✔  Approvals: 3
   ✔  - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/35052#pullrequestreview-482554674
   ✔  - Denys Otrishko (@lundibundi): https://github.com/nodejs/node/pull/35052#pullrequestreview-482579029
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/35052#pullrequestreview-482658703
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

@github-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 7, 2020
@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2020

Landed in cb2b82b

@github-actions github-actions bot closed this Sep 7, 2020
nodejs-github-bot pushed a commit that referenced this pull request Sep 7, 2020
Refs: #34600

PR-URL: #35052
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
richardlau pushed a commit that referenced this pull request Sep 7, 2020
Refs: #34600

PR-URL: #35052
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@richardlau richardlau removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 7, 2020
@richardlau richardlau mentioned this pull request Sep 7, 2020
4 tasks
richardlau pushed a commit that referenced this pull request Sep 7, 2020
Refs: #34600

PR-URL: #35052
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Refs: #34600

PR-URL: #35052
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
Refs: nodejs#34600

PR-URL: nodejs#35052
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 14, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants