FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Reject unsupported salt costs by OskarEichler · Pull Request #302 · bcrypt-ruby/bcrypt-ruby · GitHub

Repository navigation

Reject unsupported salt costs - #302

Open
OskarEichler wants to merge 1 commit into
bcrypt-ruby:masterfrom
OskarEichler:codex/reject-excessive-cost
Open

OskarEichler wants to merge 1 commit into
bcrypt-ruby:masterfrom
OskarEichler:codex/reject-excessive-cost

Conversation

Copy link
Copy Markdown

Summary

Raise BCrypt::Errors::InvalidCost when BCrypt::Engine.generate_salt receives a cost above MAX_COST.

On MRI, unsupported costs currently reach the native salt generator and return nil; JRuby raises from its Java implementation. Validating the documented upper bound in the shared Ruby layer gives both engines a deliberate bcrypt error and prevents a nil salt from escaping the public method. This is the same boundary originally reported in #27.

Reproduction

On bcrypt 3.1.22, BCrypt::Engine.generate_salt(32) returns nil on MRI. An external model checks costs 32, 99, and 2**30; all now raise BCrypt::Errors::InvalidCost, while the existing sub-minimum clamp and Password.create upper-bound behavior remain unchanged.

Verification

  • bundle exec rake compile spec: 39 examples, 0 failures on Ruby 4.0.6
  • focused upper-bound model: pass

Compatibility

Valid costs are unchanged. The only behavior change is replacing an unusable nil result or engine-specific native exception for costs above 31 with the gem's documented invalid-cost error.

This branch has not been deployed

No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL