FIX: preserve github fragment URL

FIX: preserve github fragment URL
diff --git a/lib/final_destination.rb b/lib/final_destination.rb
index d863344..a098df3 100644
--- a/lib/final_destination.rb
+++ b/lib/final_destination.rb
@@ -34,6 +34,7 @@ class FinalDestination
 
     @opts = opts || {}
     @force_get_hosts = @opts[:force_get_hosts] || []
+    @preserve_fragment_url_hosts = @opts[:preserve_fragment_url_hosts] || []
     @opts[:max_redirects] ||= 5
     @opts[:lookup_ip] ||= lambda { |host| FinalDestination.lookup_ip(host) }
 
@@ -59,6 +60,7 @@ class FinalDestination
     @limited_ips = []
     @verbose = @opts[:verbose] || false
     @timeout = @opts[:timeout] || nil
+    @preserve_fragment_url = @preserve_fragment_url_hosts.any? { |host| hostname_matches?(host) }
   end
 
   def self.connection_timeout
@@ -210,6 +212,7 @@ class FinalDestination
 
     if location
       old_port = @uri.port
+      location = "#{location}##{@uri.fragment}" if @preserve_fragment_url && @uri.fragment.present?
       location = "#{@uri.scheme}://#{@uri.host}#{location}" if location[0] == "/"
       @uri = uri(location)
       @limit -= 1
diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index f2781dd..69011d0 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -259,9 +259,13 @@ module Oneboxer
     SiteSetting.onebox_domains_blacklist.split("|")
   end
 
+  def self.preserve_fragment_url_hosts
+    ['http://github.com']
+  end
+
   def self.external_onebox(url)
     Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do
-      fd = FinalDestination.new(url, ignore_redirects: ignore_redirects, ignore_hostnames: blacklisted_domains, force_get_hosts: force_get_hosts)
+      fd = FinalDestination.new(url, ignore_redirects: ignore_redirects, ignore_hostnames: blacklisted_domains, force_get_hosts: force_get_hosts, preserve_fragment_url_hosts: preserve_fragment_url_hosts)
       uri = fd.resolve
       return blank_onebox if uri.blank? || blacklisted_domains.map { |hostname| uri.hostname.match?(hostname) }.any?
 
diff --git a/spec/components/final_destination_spec.rb b/spec/components/final_destination_spec.rb
index 78105c7..0b8c410 100644
--- a/spec/components/final_destination_spec.rb
+++ b/spec/components/final_destination_spec.rb
@@ -9,6 +9,8 @@ describe FinalDestination do
 
       force_get_hosts: ['https://force.get.com'],
 
+      preserve_fragment_url_hosts: ['https://eviltrout.com'],
+
       # avoid IP lookups in test
       lookup_ip: lambda do |host|
         case host
@@ -266,6 +268,18 @@ describe FinalDestination do
       expect(final.status).to eq(:resolved)
       expect(final.cookie).to eq("bar=1; baz=2; foo=219ffwef9w0f")
     end
+
+    it "persists fragment url" do
+      origin_url = "https://eviltrout.com/origin/lib/code/foobar.rb"
+      upstream_url = "https://eviltrout.com/upstream/lib/code/foobar.rb"
+
+      redirect_response(origin_url, upstream_url)
+      stub_request(:head, upstream_url).to_return(doc_response)
+
+      final = FinalDestination.new("#{origin_url}#L154-L205", opts)
+      expect(final.resolve.to_s).to eq("#{upstream_url}#L154-L205")
+      expect(final.status).to eq(:resolved)
+    end
   end
 
   describe '.get' do

GitHub
sha: 1ab91f04

1 Like

This should probably be a constant. Otherwise, we’ll allocating the same array each time we call the method.

1 Like

There is a balancing act, at most we call this a few times a week and as a constant we will hold the array object in memory forever and scan the RVALUE each full GC not sure we need to change anything here

2 Likes

at most we call this a few times a week

Isn’t this is called whenever there is an external onebox? That is surely more than few times a week.

1 Like

True feel free to change it

1 Like

Done in:

1 Like