Skip to content

Commit

Permalink
Merge pull request #22 from Aranda-Cyber-Solutions-LLC/disable-find-c…
Browse files Browse the repository at this point in the history
…onfiguration

Disable find method with a configuration option.
  • Loading branch information
jcypret authored Feb 16, 2017
2 parents 6cef974 + c4d0c3c commit f1c54f9
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 2 deletions.
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,23 @@ Hashid::Rails.configure do |config|
# config.alphabet is optional, hashids provides a default
# alphabet that consists of all characters [a-zA-Z0-9]
config.alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

# If your alphbet contains any numerals [0-9], then we recommend diabling the find method
#config.disable_find = true
end
```
### Disable Find Method

If your alphabet includes numerals (0-9) and if the ids in your database are the same length as your hashes, then there could be valid
hashids that are identical to valid ids. This ambiguity could lead the `find` method to behave unpredictably. Since `find` accepts both
hashids and ids, an input argument that is potentially both a valid hashid and id, will cause `find` to treat the argument as a hashid
in some cases, and as an id in others. This unpredictably is usually not desired and can lead to subtle bugs appearing at runtime

In order to avoid this problem, users can add `config.disable_find = true` to their initializer. This will disable the hashid
functionality of the `find` method and force `find` to only accept normal (unhashed) ids. Under this configuration, programmer
will need to use the `find_by_hashid` method when desiring to explicitly search by hashid.

It is recommended that `config.disable_find = true` be set when the alphabet contains any numerals.

## Development

Expand Down
6 changes: 5 additions & 1 deletion lib/hashid/rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ def decode_id(ids)
end

def find(hashid)
model_reload? ? super(hashid) : super(decode_id(hashid) || hashid)
if model_reload? || Hashid::Rails.configuration.disable_find
super(hashid)
else
super(decode_id(hashid) || hashid)
end
end

def find_by_hashid(hashid)
Expand Down
3 changes: 2 additions & 1 deletion lib/hashid/rails/configuration.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
module Hashid
module Rails
class Configuration
attr_accessor :secret, :length, :alphabet
attr_accessor :secret, :length, :alphabet, :disable_find

def initialize
@secret = ""
@length = 6
@alphabet = nil
@disable_find = false
end
end
end
Expand Down
43 changes: 43 additions & 0 deletions spec/hashid/rails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,49 @@
end
end
end

describe "disable_find" do
let(:expected_alphabet) { "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" }
let(:config) { Hashid::Rails.configuration }

before :each do
Hashid::Rails.configure do |config|
config.alphabet = expected_alphabet
config.length = 6
end
end

context "unset" do
it "unhashes argument in find method" do
# 100_024 was selected because it unhashes to 187_412
model = FakeModel.create!
model.update!(id: 187_412)
expect(FakeModel.find(100_024).id).to eql 187_412
end
end

context "set to false" do
it "unhashes argument in find method" do
Hashid::Rails.configure do |config|
config.disable_find = false
end
# 100_024 was selected because it unhashes to 187_412
expect(FakeModel.find(100_024).id).to eql 187_412
end
end

context "set to true" do
it "does not unhash argument in find method" do
Hashid::Rails.configure do |config|
config.disable_find = true
end
# 100024 was selected because it unhashes to 187412
model = FakeModel.create!
model.update!(id: 100_024)
expect(FakeModel.find(100_024).id).to eql 100_024
end
end
end
end

describe "#reset" do
Expand Down

0 comments on commit f1c54f9

Please sign in to comment.