search

urbanairship / ruby-library

A Ruby wrapper for the Urban Airship API.
Other
200 stars 117 forks source link

Incorrect logic and warning in validate_url method #121

Closed syoder closed 4 years ago

syoder commented 4 years ago

Expected Behavior

No warnings should be emitted

Current Behavior

A warning is emitted when starting our Rails app which includes this gem. Here's the warning:

/app/vendor/bundle/ruby/2.5.0/gems/urbanairship-5.5.0/lib/urbanairship/devices/mms_notification.rb:35: warning: string literal in condition

Possible Solution

the logic in this method is not correct and doesn't do what it looks like it's supposed to: https://github.com/urbanairship/ruby-library/blob/master/lib/urbanairship/devices/mms_notification.rb#L32-L36

Instead, it should be something like:

def validate_url
  unless ['.jpg', '.gif', '.png', '.jpeg'].include?(@url[-4..-1])
    fail ArgumentError, 'url must end in .gif, .jpg, .png, or .jpeg'
  end
end

The way it's written, it will never throw an ArgumentError because all those string literals will always evaluate to true.

Steps to Reproduce

Include your gem in a Rails 5 / Ruby 2.5.x app and start a rails console.

syoder commented 4 years ago

Should I submit a PR for this?

sarahdactyl71 commented 4 years ago

@syoder Thank you for brining this to our attention! We will take a look an implement your solution!

sarahdactyl71 commented 4 years ago

Hey @syoder we have added your requested fixes and have pushed them to the newest version of the urbanairship gem here: https://rubygems.org/gems/urbanairship. Thank you!

syoder commented 4 years ago

Thanks for the quick response!