Include configured base path in speedscope iframe URL (#587)

When rack-mini-profiler was used under a different base URL (e.g. when deployed in a Rails app with `relative_url_root` configured), the stackprof iframe URL was missing the base path. This commit fixes the path for the flamegraph by sharing logic with the `#get_profile_script` method, and adds a spec.
MiniProfiler · Jul 31, 2023 · 1d2bfce · 1d2bfce
1 parent 0d34c02
commit 1d2bfce
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## Unreleased
+
+- [FIX] Include configured base path in speedscope iframe URL
+
 ## 3.1.0 - 2023-04-11
 
 - [FEATURE] The query parameter that RMP uses (by default, pp) is now configurable [#553](https://github.com/MiniProfiler/rack-mini-profiler/pull/553)

diff --git a/lib/mini_profiler.rb b/lib/mini_profiler.rb
@@ -462,7 +462,7 @@ def call(env)
 
       if flamegraph && query_string =~ /#{@config.profile_parameter}=flamegraph/
         body.close if body.respond_to? :close
-        return client_settings.handle_cookie(self.flamegraph(flamegraph, path))
+        return client_settings.handle_cookie(self.flamegraph(flamegraph, path, env))
       elsif flamegraph # async-flamegraph
         page_struct[:has_flamegraph] = true
         page_struct[:flamegraph] = flamegraph
@@ -705,8 +705,9 @@ def help(client_settings, env)
       [200, headers, [html]]
     end
 
-    def flamegraph(graph, path)
+    def flamegraph(graph, path, env)
       headers = { 'Content-Type' => 'text/html' }
+      iframe_src = "#{public_base_path(env)}speedscope/index.html"
       html = <<~HTML
         <!DOCTYPE html>
         <html>
@@ -726,7 +727,7 @@ def flamegraph(graph, path)
               var iframe = document.createElement('IFRAME');
               iframe.setAttribute('id', 'speedscope-iframe');
               document.body.appendChild(iframe);
-              var iframeUrl = '#{@config.base_url_path}speedscope/index.html#profileURL=' + objUrl + '&title=' + 'Flamegraph for #{CGI.escape(path)}';
+              var iframeUrl = '#{iframe_src}#profileURL=' + objUrl + '&title=' + 'Flamegraph for #{CGI.escape(path)}';
               iframe.setAttribute('src', iframeUrl);
             </script>
           </body>
@@ -755,7 +756,7 @@ def ids_comma_separated(env)
     # * you have disabled auto append behaviour throught :auto_inject => false flag
     # * you do not want script to be automatically appended for the current page. You can also call cancel_auto_inject
     def get_profile_script(env)
-      path = "#{env['RACK_MINI_PROFILER_ORIGINAL_SCRIPT_NAME']}#{@config.base_url_path}"
+      path = public_base_path(env)
       version = MiniProfiler::ASSET_VERSION
       if @config.assets_url
         url = @config.assets_url.call('rack-mini-profiler.js', version, env)
@@ -887,7 +888,7 @@ def serve_flamegraph(env)
         return [404, {}, ["No flamegraph available for #{ERB::Util.html_escape(id)}"]]
       end
 
-      self.flamegraph(page_struct[:flamegraph], page_struct[:request_path])
+      self.flamegraph(page_struct[:flamegraph], page_struct[:request_path], env)
     end
 
     def rails_route_from_path(path, method)
@@ -945,5 +946,9 @@ def take_snapshot(env, start)
       self.current = nil
       results
     end
+
+    def public_base_path(env)
+      "#{env['RACK_MINI_PROFILER_ORIGINAL_SCRIPT_NAME']}#{@config.base_url_path}"
+    end
   end
 end
diff --git a/spec/integration/mini_profiler_spec.rb b/spec/integration/mini_profiler_spec.rb
@@ -162,6 +162,12 @@ def app
       expect(last_response.body).to include("<title>Rack::MiniProfiler Flamegraph</title>")
       expect(last_response.body).to include("var graph = {")
 
+      # Should have correct iframe URL when the base URL changes
+      get "/mini-profiler-resources/flamegraph?id=#{id}", nil, { 'SCRIPT_NAME' => '/my/base/url' }
+      expect(last_response).to be_ok
+      expect(last_response.body).to include("<title>Rack::MiniProfiler Flamegraph</title>")
+      expect(last_response.body).to include("iframeUrl = '/my/base/url/mini-profiler-resources/speedscope/")
+
       # Should not store/return flamegraph for regular requests
       get '/html'
       expect(last_response).to be_ok