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#secureCompare : use the primitive type as the return value could not be null #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benbenw
Copy link
Contributor

@benbenw benbenw commented Nov 27, 2020

Summary

Fix Crypto#secureCompare signature to use the primitive type as the return value could not be null

Checklist

  • Added changelog entry
  • Ran unit tests (mvn verify -DskipITs)

@billwerges
Copy link
Contributor

Hi @benbenw,

Thanks for opening this PR. Since this change modifies the return type of a publicly visible method, we won't be able to merge this until we release the next major version of the project.

@benbenw
Copy link
Contributor Author

benbenw commented Nov 30, 2020

no pb
but due to java type boxing it should be backward compatible to the callers

@crookedneighbor
Copy link
Contributor

@benbenw sorry for the long delay on this. I'm just curious about what scenarios the return type could be null here. I see 2 places we return:

An early return of false:

        return false;

And a final return where it asserts on the value of result:

        return result == 0;

In what instances could this ever return null?

@hollabaq86
Copy link
Contributor

for internal tracking, ticket 666

@benbenw
Copy link
Contributor Author

benbenw commented Oct 10, 2022

@benbenw sorry for the long delay on this. I'm just curious about what scenarios the return type could be null here. I see 2 places we return:

An early return of false:

        return false;

And a final return where it asserts on the value of result:

        return result == 0;

In what instances could this ever return null?

It could not return null that what this issue is about !
As stated in the description :
Fix Crypto#secureCompare signature to use the primitive type as the return value could not be null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants