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

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

Conversation

jhdavids8
Copy link
Contributor

I implemented the original fix to handle UUIDs sometime last year, and just now realized this issue. The UUID regex needs to match exact UUIDs. Right now, the UUID regex passes when there's a UUID at the end of a slug, which is what occurs for duplicates. This can result in errors like

PG::InvalidTextRepresentation: ERROR:  invalid input syntax for uuid

when it should instead result in a RecordNotFound error.

@norman
Copy link
Owner

norman commented Apr 30, 2014

Thanks for this - good catch. Could you possibly add a test for this so that we are sure to avoid future regressions?

@jhdavids8
Copy link
Contributor Author

Done! I changed the tests I put together last time to actually think they're using a UUID (rather than a string).

@@ -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.

norman added a commit that referenced this pull request May 29, 2014
Make UUID regex match only exact UUIDs (currently, it'll match strings with UUIDs anywhere in the string)
@norman norman merged commit 27f389f into norman:master May 29, 2014
@norman
Copy link
Owner

norman commented May 29, 2014

Merged, thanks!

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.

3 participants