Validate URLs to be valid HTTP or HTTPS, don't accept others. Closes a…

…riejan#8

HISTORY

@@ -1,5 +1,6 @@
 = HEAD
 
+ * 2010-04-30 - Validate URLs to be valid HTTP or HTTPS, don't accept others. [ariejan]
  * 2010-04-30 - Don't ask for the API key after shortening a URL with the bookmarklet. [ariejan]
 
  * 2010-04-15 - Show the highlighted URL separately. Closes #7. [ariejan]

lib/firefly/server.rb

@@ -53,8 +53,13 @@ def generate_short_url(url = nil)
 
         if !url.nil? && url != ""
           ff_url  = Firefly::Url.shorten(url)
-          code    = ff_url.code
-          result  = "http://#{config[:hostname]}/#{code}"
+          if !ff_url.nil?
+            code    = ff_url.code
+            result  = "http://#{config[:hostname]}/#{code}"
+          else
+            code    = nil
+            result  = "ERROR: The URL you posted is invalid."
+          end
         end
 
         return code, result
@@ -85,7 +90,8 @@ def store_api_key(key)
     end
 
     get '/' do
-      @highlight ||= Firefly::Url.first(:code => params[:highlight])
+      @highlight = Firefly::Url.first(:code => params[:highlight])
+      @error     = params[:highlight] == "error"
       @urls = Firefly::Url.all(:limit => config[:recent_urls], :order => [ :created_at.desc ])
       haml :index
     end
@@ -107,6 +113,7 @@ def store_api_key(key)
 
       if params[:visual]
         store_api_key(params[:api_key])
+        @code ||= "error"
         redirect "/?highlight=#{@code}"
       else
         @result

lib/firefly/url.rb

@@ -1,7 +1,9 @@
 module Firefly
   class Url
     include DataMapper::Resource
-
+
+    VALID_URL_REGEX = /^(http|https):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,5}(:[0-9]{1,5})?(\/.*)?$/ix
+
     property :id,           Serial
     property :url,          String,     :index => true, :length => 255
     property :code,         String,     :index => true, :length => 16
@@ -15,9 +17,18 @@ def register_click!
 
     # Shorten a long_url and return a new FireFly::Url
     def self.shorten(long_url)
+      return nil unless valid_url?(long_url)
+
       the_url = Firefly::Url.first(:url => long_url) || Firefly::Url.create(:url => long_url)
       the_url.update(:code => Firefly::Base62.encode(the_url.id.to_i)) if the_url.code.nil?
       the_url
     end  
+
+    private
+
+      # Validates the URL to be a valid http or https one. 
+      def self.valid_url?(url)
+        url.match(Firefly::Url::VALID_URL_REGEX)
+      end
   end
 end

spec/firefly/api_spec.rb

@@ -16,11 +16,23 @@ def app
 
       it "should return the shortened URL" do
         self.send verb, '/api/add', :url => 'http://example.org', :api_key => 'test'
-
         url = Firefly::Url.first(:url => "http://example.org")
 
         last_response.body.should eql("http://test.host/#{url.code}")
       end
+
+      it "should show an error when shortening an invalid URL" do
+        self.send verb, '/api/add', :url => 'ftp://example.org', :api_key => 'test'
+
+        last_response.body.should match("The URL you posted is invalid")
+      end
+
+      it "should show an error when shortening an invalid URL in visual mode" do
+        self.send verb, '/api/add', :url => 'ftp://example.org', :api_key => 'test', :visual => "1"
+        follow_redirect!
+
+        last_response.body.should match("The URL you posted is invalid")
+      end
 
       it "should redirect to the highlighted URL when visual is enabled" do
         self.send verb, '/api/add', :url => 'http://example.org', :api_key => 'test', :visual => "1"
@@ -64,8 +76,8 @@ def app
         }.should_not change(Firefly::Url, :count).by(1)    
       end
     end
-  end
-    
+  end  
+
   describe "getting information" do
     before(:each) do
       @created_at = Time.now

spec/firefly/url_spec.rb

@@ -3,7 +3,6 @@
 describe "Url" do
 
   describe "shortening" do
-
     it "should generate a code after create" do
       url = Firefly::Url.shorten("http://example.com/")
       Firefly::Url.first(:url => "http://example.com/").code.should_not be_nil
@@ -28,6 +27,29 @@
     end
   end
 
+  describe "long url validation" do
+    [ "http://ariejan.net", 
+      "https://ariejan.net",
+      "http://ariejan.net/page/1",
+      "http://ariejan.net/page/1?q=x&p=123",
+      "http://ariejan.net:8080/"
+    ].each do |url|
+      it "should accept #{url}" do
+        Firefly::Url.shorten(url).should_not be_nil
+      end
+    end
+
+    [ "ftp://ariejan.net", 
+      "irc://freenode.org/rails",
+      "skype:adevroom",
+      "ariejan.net",
+    ].each do |url|
+      it "should not accept #{url}" do
+        Firefly::Url.shorten(url).should be_nil
+      end
+    end    
+  end
+
   describe "clicking" do
     before(:each) do
       Firefly::Url.create(

views/index.haml

@@ -17,6 +17,12 @@
         %input.big_url{ :type => 'text', :name => 'url', :id => 'url', :size => 50, :autocomplete => "off", :spellcheck => 'false' }
       %p
         %input.big_url{ :type => 'submit', :name => 'submit', :id => 'submit', :value => "Make it short!" }
+
+  - if @error
+    %h2 Whoops!
+
+    %p
+      The URL you posted is invalid. Please post a valid HTTP url, including the protocol (http://) prefix.
 
   - if @highlight
     %h2 Your short URL