FIX: URLs containing two # would fail to work

Some URLs in browsers are non compliant and contain twos `#` this commit adds
special handling for this edge case by auto encoding any fragments containing `#`

app/models/topic_link.rb

@@ -117,11 +117,7 @@ SQL
     PrettyText
       .extract_links(post.cooked)
       .map do |u|
-        uri = begin
+        uri = UrlHelper.relaxed_parse(u.url)
-          URI.parse(u.url)
-        rescue URI::Error
-        end
-
         [u, uri]
       end
       .reject { |_, p| p.nil? || "mailto".freeze == p.scheme }

app/models/topic_link_click.rb

@@ -15,11 +15,7 @@ class TopicLinkClick < ActiveRecord::Base
     url = args[:url][0...TopicLink.max_url_length]
     return nil if url.blank?
 
-    uri = begin
+    uri = UrlHelper.relaxed_parse(url)
-      URI.parse(url)
-    rescue URI::Error
-    end
-
     urls = Set.new
     urls << url
     if url =~ /^http/

lib/url_helper.rb

@@ -1,5 +1,20 @@
 class UrlHelper
 
+  # At the moment this handles invalid URLs that browser address bar accepts
+  # where second # is not encoded
+  #
+  # Longer term we can add support of simpleidn and encode unicode domains
+  def self.relaxed_parse(url)
+    url, fragment = url.split("#", 2)
+    uri = URI.parse(url)
+    if uri
+      fragment = URI.escape(fragment) if fragment&.include?('#')
+      uri.fragment = fragment
+      uri
+    end
+  rescue URI::Error
+  end
+
   def self.is_local(url)
     url.present? && (
       Discourse.store.has_been_uploaded?(url) ||

spec/components/url_helper_spec.rb

@@ -3,6 +3,15 @@ require_dependency 'url_helper'
 
 describe UrlHelper do
 
+  describe "#relaxed parse" do
+
+    it "can handle double #" do
+      url = UrlHelper.relaxed_parse("https://test.com#test#test")
+      expect(url.to_s).to eq("https://test.com#test%23test")
+    end
+
+  end
+
   describe "#is_local" do
 
     it "is true when the file has been uploaded" do

spec/fabricators/post_fabricator.rb

@@ -95,27 +95,28 @@ Fabricator(:post_with_uploads, from: :post) do
 end
 
 Fabricator(:post_with_uploads_and_links, from: :post) do
-  raw '
+  raw <<~RAW
-<a href="/uploads/default/original/2X/2345678901234567.jpg">Link</a>
+    <a href="/uploads/default/original/2X/2345678901234567.jpg">Link</a>
-<img src="/uploads/default/original/1X/1234567890123456.jpg">
+    <img src="/uploads/default/original/1X/1234567890123456.jpg">
-<a href="http://www.google.com">Google</a>
+    <a href="http://www.google.com">Google</a>
-<img src="http://foo.bar/image.png">
+    <img src="http://foo.bar/image.png">
-<a class="attachment" href="/uploads/default/original/1X/af2c2618032c679333bebf745e75f9088748d737.txt">text.txt</a> (20 Bytes)
+    <a class="attachment" href="/uploads/default/original/1X/af2c2618032c679333bebf745e75f9088748d737.txt">text.txt</a> (20 Bytes)
-:smile:
+    :smile:
-'
+  RAW
 end
 
 Fabricator(:post_with_external_links, from: :post) do
   user
   topic
-  raw "
+  raw <<~RAW
-Here's a link to twitter: http://twitter.com
+    Here's a link to twitter: http://twitter.com
-And a link to google: http://google.com
+    And a link to google: http://google.com
-And a secure link to google: https://google.com
+    And a secure link to google: https://google.com
-And a markdown link: [forumwarz](http://forumwarz.com)
+    And a markdown link: [forumwarz](http://forumwarz.com)
-And a markdown link with a period after it [codinghorror](http://www.codinghorror.com/blog).
+    And a markdown link with a period after it [codinghorror](http://www.codinghorror.com/blog).
-And one with a hash http://discourse.org#faq
+    And one with a hash http://discourse.org#faq
-  "
+    And one with a two hash http://discourse.org#a#b
+  RAW
 end
 
 Fabricator(:private_message_post, from: :post) do

spec/models/topic_link_click_spec.rb

@@ -54,6 +54,10 @@ describe TopicLinkClick do
           TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.0', user_id: @post.user_id)
         }.not_to change(TopicLinkClick, :count)
 
+        # can handle double # in a url
+        # NOTE: this is not compliant but exists in the wild
+        click = TopicLinkClick.create_from(url: "http://discourse.org#a#b", post_id: @post.id, ip: '127.0.0.1')
+        expect(click).to eq("http://discourse.org#a#b")
       end
 
       context 'with a valid url and post_id' do