-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add domain validation during Client initialization #42
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any changes here to rip out the old "magic" behavior of automatically handling protocols and trailing slashes. It looks like that logic is being handled at the end of the get_host
method. Can we make sure that gets removed as well?
lib/imgix/client.rb
Outdated
@@ -75,6 +75,11 @@ def validate_hosts! | |||
unless @hosts.length > 0 | |||
raise ArgumentError, "The :host or :hosts option must be specified" | |||
end | |||
@hosts.each do |host| | |||
unless host.match(DOMAIN_REGEX) != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This double-negative seems unnecessarily confusing. Can we do if host.match(DOMAIN_REGEX) == nil
instead?
…onditional on domain validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 from me once the tests pass
Adds a validation that will throw an error if a user attempts to initialize a new
Client
object with an invalid domain.The validation criteria is based off of the same regex used on dashboard.imgix.com