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

Support disabling of hashid signing feature #37

Merged
merged 2 commits into from
Oct 5, 2017

Conversation

olliebennett
Copy link
Contributor

This PR fixes #30 by implementing a configuration option, sign_hashids, which controls how the underlying hashid is generated. The choice is:

sign_hashids = true (default)

  • hashid is generated by encoding a "signed" array; encode_id([42, id]).
  • this logic was implemented in 941bc3e.
  • this signed array is validated during decoding, to identify that the correct ID has been obtained (i.e. when decoding, we confirm that the decoded value is of the form [42, x] and return the x.
  • the benefit is that when decoding a value which could be either a raw ID or a hashid, it's possible to determine which it is; a raw ID that may (by chance) be a valid hashid (for a different ID) will not be mistakenly decoded into that ID.
  • the disadvantage is a slightly longer hashid being generated.

sign_hashids = false

  • hashid is generated simply from the id itself; encode_id(id).
  • this was the behaviour for hashid-rails < 1.0.0; in order to maintain backwards compatibility for existing hashids, this must be set.

Copy link
Owner

@jcypret jcypret left a comment

Choose a reason for hiding this comment

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

Nice work. The changes look good.

@jcypret jcypret merged commit 222c5e9 into jcypret:master Oct 5, 2017
@jcypret
Copy link
Owner

jcypret commented Oct 5, 2017

@olliebennett I'm going to add an additional note in the readme and get a new version of the gem pushed. Thanks for the contribution!

@jcypret
Copy link
Owner

jcypret commented Oct 5, 2017

Added a note to the readme with upgrade instructions and warning others about the changing ID behavior.

@olliebennett olliebennett deleted the disable-hashid-signing branch October 5, 2017 23:15
@olliebennett
Copy link
Contributor Author

Thanks a lot for the quick turnaround, Justin! :) Glad to be back on the bleeding edge of the gem.

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.

Supporting disabling of hashid signing
2 participants