diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 1d07636a507..d10b63f034d 100644 --- a/app/models/user_profile.rb +++ b/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 diff --git a/lib/validators/url_validator.rb b/lib/validators/url_validator.rb index c3295e63684..7c91bf206ba 100644 --- a/lib/validators/url_validator.rb +++ b/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 diff --git a/spec/components/validators/url_validator_spec.rb b/spec/components/validators/url_validator_spec.rb new file mode 100644 index 00000000000..5554700b774 --- /dev/null +++ b/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 diff --git a/spec/models/user_profile_spec.rb b/spec/models/user_profile_spec.rb index 7ae25e14cdc..0fffc455aaa 100644 --- a/spec/models/user_profile_spec.rb +++ b/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 - expect(Fabricate.build(:user_profile, user: user, website: "http://https://google.com")).not_to be_valid - expect(Fabricate.build(:user_profile, user: user, website: "http://discourse.productions")).to be_valid - expect(Fabricate.build(:user_profile, user: user, website: "https://google.com")).to be_valid + it "should not allow invalid URLs" do + user_profile.website = "http://https://google.com" + expect(user_profile).to_not 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