Improve URL validation to check for a valid host.

Parsing a URL with `URI` is not sufficient as the following cases
are considered valid:

URI.parse("http://https://google.com")
=> #<URI::HTTP http://https//google.com>

app/models/user_profile.rb

@@ -1,13 +1,8 @@
 class UserProfile < ActiveRecord::Base
   belongs_to :user, inverse_of: :user_profile
 
-  # This is not very picky about most DNS labels (the bits between the
-  # periods), but isn't taking much guff from the TLD. No leading
-  # digit, and no hyphens unless IDN.
-  WEBSITE_REGEXP = /(^$)|(^(https?:\/\/)?([a-z0-9][a-z0-9-]*\.)+([a-z][a-z0-9]+|xn--[a-z0-9-]+)(\/.*)?$)/i
-
   validates :bio_raw, length: { maximum: 3000 }
-  validates :website, format: { with: WEBSITE_REGEXP }, allow_blank: true, if: Proc.new { |c| c.new_record? || c.website_changed? }
+  validates :website, url: true, allow_blank: true, if: Proc.new { |c| c.new_record? || c.website_changed? }
   validates :user, presence: true
   before_save :cook
   after_save :trigger_badges

lib/validators/url_validator.rb

@@ -1,9 +1,15 @@
 class UrlValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     if value.present?
-      uri = URI.parse(value) rescue nil
+      valid =
+        begin
+          uri = URI.parse(value)
+          uri.is_a?(URI::HTTP) && !uri.host.nil? && uri.host.include?(".")
+        rescue
+          nil
+        end
 
-      unless uri
+      unless valid
         record.errors[attribute] << (options[:message] || I18n.t('errors.messages.invalid'))
       end
     end

spec/components/validators/url_validator_spec.rb

@@ -0,0 +1,33 @@
+require 'rails_helper'
+require 'validators/topic_title_length_validator'
+
+RSpec.describe UrlValidator do
+  let(:record) { Fabricate.build(:user_profile, user: Fabricate.build(:user)) }
+  let(:validator) { described_class.new(attributes: :website) }
+  subject(:validate) { validator.validate_each(record, :website, record.website) }
+
+  [
+    "http://https://google.com",
+    "http://google/",
+    "ftp://ftp.google.com",
+    "http:///what.is.this",
+    'http://meta.discourse.org TEST'
+  ].each do |invalid_url|
+    it "#{invalid_url} should not be valid" do
+      record.website = invalid_url
+      validate
+      expect(record.errors[:website]).to be_present
+    end
+  end
+
+  [
+    "http://discourse.productions",
+    "https://google.com",
+  ].each do |valid_url|
+    it "#{valid_url} should be valid" do
+      record.website = valid_url
+      validate
+      expect(record.errors[:website]).to_not be_present
+    end
+  end
+end

spec/models/user_profile_spec.rb

@@ -55,18 +55,21 @@ describe UserProfile do
     end
 
     context "website validation" do
-      let(:user) { Fabricate(:user) }
+      let(:user_profile) { Fabricate.build(:user_profile, user: Fabricate(:user)) }
 
-      it "ensures website is valid" do
+      it "should not allow invalid URLs" do
-        expect(Fabricate.build(:user_profile, user: user, website: "http://https://google.com")).not_to be_valid
+        user_profile.website = "http://https://google.com"
-        expect(Fabricate.build(:user_profile, user: user, website: "http://discourse.productions")).to be_valid
+        expect(user_profile).to_not be_valid
-        expect(Fabricate.build(:user_profile, user: user, website: "https://google.com")).to be_valid
       end
 
       it "validates website domain if user_website_domains_whitelist setting is present" do
         SiteSetting.user_website_domains_whitelist = "discourse.org"
-        expect(Fabricate.build(:user_profile, user: user, website: "https://google.com")).not_to be_valid
+
-        expect(Fabricate.build(:user_profile, user: user, website: "http://discourse.org")).to be_valid
+        user_profile.website = "https://google.com"
+        expect(user_profile).not_to be_valid
+
+        user_profile.website = "http://discourse.org"
+        expect(user_profile).to be_valid
       end
     end