fixing URL validation for import_url on projects

3 files changed, 55 insertions, 3 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 0bb815e64e7..a3f78349e98 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -153,9 +153,7 @@ class Project < ActiveRecord::Base
   validates :namespace, presence: true
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
-  validates :import_url,
-    url: { protocols: %w(ssh git http https) },
-    if: :external_import?
+  validates :import_url, addressable_url: true, if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :avatar_type,
diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb
new file mode 100644
index 00000000000..4e1a01a1bff
--- /dev/null
+++ b/app/validators/addressable_url_validator.rb
@@ -0,0 +1,49 @@
+# UrlValidator
+#
+# Custom validator for URLs.
+#
+# By default, only URLs for the HTTP(S) protocols will be considered valid.
+# Provide a `:protocols` option to configure accepted protocols.
+#
+# Example:
+#
+#   class User < ActiveRecord::Base
+#     validates :personal_url, url: true
+#
+#     validates :ftp_url, url: { protocols: %w(ftp) }
+#
+#     validates :git_url, url: { protocols: %w(http https ssh git) }
+#   end
+#
+class AddressableUrlValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    unless valid_url?(value)
+      record.errors.add(attribute, "must be a valid URL")
+    end
+  end
+
+  private
+
+  def default_options
+    @default_options ||= { protocols: %w(http https ssh git) }
+  end
+
+  def valid_url?(value)
+    return false unless value
+
+    value.strip!
+
+    valid_uri?(value) && valid_protocol?(value)
+  rescue Addressable::URI::InvalidURIError
+    false
+  end
+
+  def valid_uri?(value)
+    Addressable::URI.parse(strip).is_a?(Addressable::URI)
+  end
+
+  def valid_protocol?(value)
+    options = default_options.merge(self.options)
+    value =~ /\A#{URI.regexp(options[:protocols])}\z/
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index fedab1f913b..c99fd7c633e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -63,6 +63,11 @@ describe Project, models: true do
       expect(project2).not_to be_valid
       expect(project2.errors[:limit_reached].first).to match(/Personal project creation is not allowed/)
     end
+
+    it 'should not allow an invalid URI as import_url' do
+      project2 = build(:project)
+      expect(project2).to be_valid
+    end
   end
 
   describe 'default_scope' do