Ensure that urls are only valid if the entire string is a url

This replaces our homegrown regexps (that didn't quite work) with
ruby built-in regexps, and uses the termination anchor to ensure
that the entire string, not just the first part, is validated.

diff --git a/app/models/client_application.rb b/app/models/client_application.rb
index 67ff075f1adb5bd1d8d6109077cdefd08233c26d..4e3dffb998aba0081a61dd56b145420f47a7f3df 100644 (file)
--- a/app/models/client_application.rb
+++ b/app/models/client_application.rb
@@ -39,9 +39,9 @@ class ClientApplication < ApplicationRecord
 
   validates :key, :presence => true, :uniqueness => true
   validates :name, :url, :secret, :presence => true
-  validates :url, :format => %r{\Ahttp(s?)://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i
-  validates :support_url, :allow_blank => true, :format => %r{\Ahttp(s?)://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i
-  validates :callback_url, :allow_blank => true, :format => %r{\A[a-z][a-z0-9.+-]*://(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!\-/]))?}i
+  validates :url, :format => /\A#{URI::DEFAULT_PARSER.make_regexp(%w[http https])}\z/
+  validates :support_url, :allow_blank => true, :format => /\A#{URI::DEFAULT_PARSER.make_regexp(%w[http https])}\z/
+  validates :callback_url, :allow_blank => true, :format => /\A#{URI::DEFAULT_PARSER.make_regexp}\z/
 
   before_validation :generate_keys, :on => :create
 

diff --git a/test/models/client_application_test.rb b/test/models/client_application_test.rb
index 8e5b21aff7c55500b376644bf7db16218e03fe03..c0b7b7977fcd6d84a7e4b6c5a9422768c5caccce 100644 (file)
--- a/test/models/client_application_test.rb
+++ b/test/models/client_application_test.rb
@@ -3,7 +3,7 @@ require "test_helper"
 class ClientApplicationTest < ActiveSupport::TestCase
   def test_url_valid
     ok = ["http://example.com/test", "https://example.com/test"]
-    bad = ["", "ftp://example.com/test", "myapp://somewhere"]
+    bad = ["", "ftp://example.com/test", "myapp://somewhere", "http://example.com\nhttp://example.net"]
 
     ok.each do |url|
       app = build(:client_application)
@@ -20,7 +20,7 @@ class ClientApplicationTest < ActiveSupport::TestCase
 
   def test_support_url_valid
     ok = ["", "http://example.com/test", "https://example.com/test"]
-    bad = ["ftp://example.com/test", "myapp://somewhere", "gibberish"]
+    bad = ["ftp://example.com/test", "myapp://somewhere", "gibberish", "http://example.com\nhttp://example.net"]
 
     ok.each do |url|
       app = build(:client_application)
@@ -37,7 +37,7 @@ class ClientApplicationTest < ActiveSupport::TestCase
 
   def test_callback_url_valid
     ok = ["", "http://example.com/test", "https://example.com/test", "ftp://example.com/test", "myapp://somewhere"]
-    bad = ["gibberish"]
+    bad = ["gibberish", "http://example.com\nhttp://example.net"]
 
     ok.each do |url|
       app = build(:client_application)