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.

Sign up for GitHub

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

Make UUID regex match only exact UUIDs (currently, it'll match strings with UUIDs anywhere in the string) #548

Merged
merged 3 commits into from
May 29, 2014
Merged

Make UUID regex match only exact UUIDs (currently, it'll match strings with UUIDs anywhere in the string) #548

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a390c49
Make UUID regex match only exact UUIDs (currently, it'll match strings
jhdavids8 Apr 29, 2014
78be486
Add test to verify a friendly find with a string that contains a UUID is
jhdavids8 Apr 30, 2014
1220959
Update UUID regex
jhdavids8 May 26, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/friendly_id/finder_methods.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def potential_primary_key?(id)
when :integer
Integer(id, 10) rescue false
when :uuid
id.match /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/
id.match /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this use the following regular expression instead:

id.match /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/

In Ruby, ^ will only match the start of a line and $ will only match the end of a line whereas \A will match the absolute start of input and \z will match the absolute end of input. This projects from particular injection attacks but in general, in my opinion, is just more complete.

else
true
end
Expand Down
8 changes: 4 additions & 4 deletions test/schema.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ def up
end
end

tables_with_string_primary_key.each do |table_name|
create_table table_name, primary_key: :string_key, id: false do |t|
tables_with_uuid_primary_key.each do |table_name|
create_table table_name, primary_key: :uuid_key, id: false do |t|
t.string :name
t.string :string_key, null: false
t.string :uuid_key, null: false
t.string :slug
end
add_index table_name, :slug, unique: true
Expand Down Expand Up @@ -73,7 +73,7 @@ def slugged_tables
%w[journalists articles novelists novels manuals]
end

def tables_with_string_primary_key
def tables_with_uuid_primary_key
["menu_items"]
end

Expand Down
28 changes: 21 additions & 7 deletions test/slugged_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class Journalist < ActiveRecord::Base
end
end

class StringAsPrimaryKeyFindTest < MiniTest::Unit::TestCase
class UuidAsPrimaryKeyFindTest < MiniTest::Unit::TestCase
include FriendlyId::Test

class MenuItem < ActiveRecord::Base
Expand All @@ -250,29 +250,43 @@ class MenuItem < ActiveRecord::Base
before_create :init_primary_key

def self.primary_key
"string_key"
"uuid_key"
end

# Overwrite the method added by FriendlyId
def self.primary_key_type
:uuid
end

private
def init_primary_key
self.string_key = SecureRandom.uuid
self.uuid_key = SecureRandom.uuid
end
end

def model_class
MenuItem
end

test "should have a string as a primary key" do
assert_equal model_class.primary_key, "string_key"
assert_equal model_class.columns.find(&:primary).name, "string_key"
test "should have a uuid_key as a primary key" do
assert_equal model_class.primary_key, "uuid_key"
assert_equal model_class.columns.find(&:primary).name, "uuid_key"
assert_equal model_class.primary_key_type, :uuid
end

test "should be findable by the string primary key" do
test "should be findable by the UUID primary key" do
with_instance_of(model_class) do |record|
assert model_class.friendly.find record.id
end
end

test "should handle a string that simply contains a UUID correctly" do
with_instance_of(model_class) do |record|
assert_raises(ActiveRecord::RecordNotFound) do
model_class.friendly.find "test-#{SecureRandom.uuid}"
end
end
end
end

class UnderscoreAsSequenceSeparatorRegressionTest < MiniTest::Unit::TestCase
Expand Down