rails validating url with URI.parse throwing unexpected results

Question

I'm following this solution to validate a url in a service class in a rails app. The test returns false, but in the console no error is raised when I parse the given url with Ruby's URI class. I don't think there are any typos in my code - any suggestions where I'm going wrong?

class UrlService
  attr_accessor :uri

  def initialize(uri)
    @uri = uri
  end

  def valid?
    begin
      uri = URI.parse(uri)
      !uri.host.nil? && uri.kind_of?(URI::HTTP)
    rescue URI::InvalidURIError
      false
    end
  end

end

test

require "rails_helper"

RSpec.describe UrlService do
  subject {described_class.new(url)}

  describe "#valid?" do
    context "success" do
      let(:url) { "https://www.google.com/" }

      it "returns true" do
        expect(subject.valid?).to be_truthy
      end
    end
  end

end

On a side note - if you are planning to use it in models it would make sense to write it as a [custom validator](http://api.rubyonrails.org/classes/ActiveModel/Validator.html) instead of a service. — max, Jan 30 '17 at 16:15

score 7 · Accepted Answer · answered Jan 30 '17 at 16:10

Your issue is actually a Ruby gotcha, on this line:

uri = URI.parse(uri)

What you wanted to happen was for uri to be redefined as the parsed version of uri. What's actually happening is that Ruby is seeing "oh, you're defining a new local variable uri"; that new local variable now takes precedence over the method uri which you defined with your attr_accessor. And for some reason, Ruby evaluates self-references during local variable assignment as nil. This is an example of shadowing.

So, the statement above causes URI.parse to always execute on a value of nil, which is why your test is failing no matter what you set that URI to. To fix it, just use a different variable name:

parsed_uri = URI.parse(uri)

Appendix: short examples that prove I'm not making this up

irb(main):016:0> z
NameError: undefined local variable or method `z' for main:Object
        from (irb):16
        from /Users/rnubel/.rubies/ruby-2.3.3/bin/irb:11:in `<main>'
irb(main):017:0> z = z
=> nil

The first statement fails, because it just references a local var that doesn't exist. The second statement succeeds, because Ruby evaluates the z variable in the assignment of z as nil.

irb(main):011:0> class Foo; def bar; "http://www.google.com"; end; end
=> :bar
irb(main):012:0> Foo.new.instance_exec { bar }
=> "http://www.google.com"
irb(main):013:0> Foo.new.instance_exec { bar = (puts bar.inspect) }
nil
=> nil

This is the same issue you're having; just with a puts to inspect the value in-line. It prints nil.

I thought it was something like that. Thanks for the shadowing explanation. — margo, Jan 30 '17 at 18:22

score -1 · Answer 2 · answered Jan 30 '17 at 16:36

You may want to write it as a custom validator instead of a service if the intended use is to validate models.

# lib/validators/uri_validator.rb
class UriValidator < ActiveModel::Validator
  def validate_each(record, attribute, value)
    begin
      uri = URI.parse(value)
    rescue URI::InvalidURIError
    end
    if uri && uri.host && uri.kind_of?(URI::HTTP)
      record.errors.add attribute,
    end
  end
end

You can then use it by:

validates :some_attribute, presence: true, url: true

The most straight forward way to test it would be through the model class that uses the validator:

require "rails_helper"
RSpec.describe Thing, type: :model do
  describe "validations" do
    describe "url" do
      it "does not allow an invalid URL" do
        thing = Thing.new(url: 'foo')
        thing.valid
        expect(thing.errors).to have_key :url
      end
      it "allows a valid URL" do
        thing = Thing.new(url: 'https://www.google.com/')
        thing.valid
        expect(thing.errors).to have_key :url
      end
    end
  end
end

Thanks for your comment. I saw that option in the link I originally posted. My class is not an ActiveRecord or ActiveModel class. — margo, Jan 30 '17 at 18:22

rails validating url with URI.parse throwing unexpected results

2 Answers2

Appendix: short examples that prove I'm not making this up