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 incorrect use of INT_MAX in validation #22581

Closed

Conversation

tniessen
Copy link
Member

The native crypto module doesn't export INT_MAX, so all occurrences in the JavaScript layer evaluated to undefined. This change removes all such occurrences and replaces validateInt32 with validateUint32 since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants module, but that would be pointless as far as I can tell. If someone sees a reason to keep using INT_MAX here, I will gladly change it.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.
@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Aug 29, 2018
@tniessen
Copy link
Member Author

cc @nodejs/crypto

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 31, 2018
@BridgeAR
Copy link
Member

@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

Landed in 761bbfb

@addaleax addaleax closed this Sep 2, 2018
addaleax pushed a commit that referenced this pull request Sep 2, 2018
The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.

PR-URL: #22581
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 2, 2018
The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.

PR-URL: #22581
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 3, 2018
The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.

PR-URL: #22581
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Sep 6, 2018
The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.

PR-URL: #22581
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2018
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.

4 participants