From b059a0f789d1953cc0c6e6c2ae86a39e98f42790 Mon Sep 17 00:00:00 2001
From: Arpit Jalan <arpit@techapj.com>
Date: Sat, 29 Jul 2017 22:12:04 +0530
Subject: [PATCH] extract url escaping to a dedicated class method and improved
 tests

---
 lib/final_destination.rb                  |  7 ++++++-
 spec/components/final_destination_spec.rb | 25 +++++++++--------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/final_destination.rb b/lib/final_destination.rb
index 7d4e841c908..ad02e6fbdf8 100644
--- a/lib/final_destination.rb
+++ b/lib/final_destination.rb
@@ -10,9 +10,10 @@ class FinalDestination
   attr_reader :cookie
 
   def initialize(url, opts = nil)
+    @url = url
     @uri =
       begin
-        URI(URI.escape(CGI.unescapeHTML(url), Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]"))) if url
+        URI(escape_url) if @url
       rescue URI::InvalidURIError
       end
 
@@ -176,6 +177,10 @@ class FinalDestination
     false
   end
 
+  def escape_url
+    URI.escape(CGI.unescapeHTML(@url), Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]"))
+  end
+
   def private_ranges
     FinalDestination.standard_private_ranges +
       SiteSetting.blacklist_ip_blocks.split('|').map { |r| IPAddr.new(r) rescue nil }.compact
diff --git a/spec/components/final_destination_spec.rb b/spec/components/final_destination_spec.rb
index 42456e05835..baa544fdc03 100644
--- a/spec/components/final_destination_spec.rb
+++ b/spec/components/final_destination_spec.rb
@@ -60,21 +60,6 @@ describe FinalDestination do
         stub_request(:head, "https://eviltrout.com").to_return(doc_response)
       end
 
-      it "escapes url" do
-        url = 'https://eviltrout.com?s=180&#038;d=mm&#038;r=g'
-        escaped_url = URI.escape(CGI.unescapeHTML(url), Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]"))
-        stub_request(:head, escaped_url).to_return(doc_response)
-
-        expect(fd(url).resolve.to_s).to eq(escaped_url)
-      end
-
-      it "preserves url fragment identifier" do
-        url = 'https://eviltrout.com/2016/02/25/fixing-android-performance.html#discourse-comments'
-        stub_request(:head, 'https://eviltrout.com/2016/02/25/fixing-android-performance.html').to_return(doc_response)
-
-        expect(fd(url).resolve.to_s).to eq(url)
-      end
-
       it "returns the final url" do
         final = FinalDestination.new('https://eviltrout.com', opts)
         expect(final.resolve.to_s).to eq('https://eviltrout.com')
@@ -288,4 +273,14 @@ describe FinalDestination do
     end
   end
 
+  describe ".escape_url" do
+    it "correctly escapes url" do
+      fragment_url = "https://eviltrout.com/2016/02/25/fixing-android-performance.html#discourse-comments"
+
+      expect(fd(fragment_url).escape_url.to_s).to eq(fragment_url)
+      expect(fd("https://eviltrout.com?s=180&#038;d=mm&#038;r=g").escape_url.to_s).to eq("https://eviltrout.com?s=180&d=mm&r=g")
+      expect(fd("http://example.com/?a=\11\15").escape_url.to_s).to eq("http://example.com/?a=%09%0D")
+    end
+  end
+
 end