Skip to content

Commit

Permalink
Use HTML5 sanitizer to serve huge payloads through Speedscope remote …
Browse files Browse the repository at this point in the history
…viewer

When we run the app profiler on Shopify core's boot, it is frequent that
the generated profile JSON file exceeds 10MB.

`Rails::Html::Sanitizer` is unable to process such huge payloads
(because libxml2 limits text nodes to 10MB by default?).

The `rails-html-sanitizer` introduced HTML5 parsing in its 1.6.0
version, which lifts the text node size limit.

This change intends to use the new HTML5 parser when it's available
to sanitize the Speedscope remote viewer's HTML, allowing bigger
payloads to be served successfully.
  • Loading branch information
davidstosik committed Oct 24, 2023
1 parent fbf4c3e commit 37e35d9
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 9 deletions.
17 changes: 9 additions & 8 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -104,35 +104,36 @@ GEM
i18n (1.8.2)
concurrent-ruby (~> 1.0)
jwt (2.3.0)
loofah (2.19.1)
loofah (2.21.4)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
nokogiri (>= 1.12.0)
memoist (0.16.2)
method_source (1.0.0)
mini_mime (1.1.2)
mini_portile2 (2.8.1)
mini_portile2 (2.8.5)
minitest (5.15.0)
minitest-stub-const (0.6)
mocha (1.11.2)
multi_json (1.15.0)
multipart-post (2.1.1)
nokogiri (1.14.1)
mini_portile2 (~> 2.8.0)
nokogiri (1.15.4)
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
os (1.1.4)
parallel (1.22.1)
parser (3.1.2.0)
ast (~> 2.4.1)
public_suffix (4.0.6)
racc (1.6.2)
racc (1.7.1)
rack (2.2.6.3)
rack-test (1.1.0)
rack (>= 1.0, < 3)
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
nokogiri (>= 1.6)
rails-html-sanitizer (1.5.0)
loofah (~> 2.19, >= 2.19.1)
rails-html-sanitizer (1.6.0)
loofah (~> 2.21)
nokogiri (~> 1.14)
railties (5.2.4.3)
actionpack (= 5.2.4.3)
activesupport (= 5.2.4.3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ module AppProfiler
module Viewer
class SpeedscopeRemoteViewer < BaseViewer
class BaseMiddleware
class Sanitizer < Rails::Html::SafeListSanitizer
sanitizer_superclass = if Rails::Html::Sanitizer::VERSION >= "1.6.0" && Rails::HTML::Sanitizer.html5_support?
Rails::HTML5::SafeListSanitizer
else
Rails::Html::SafeListSanitizer
end

class Sanitizer < sanitizer_superclass
self.allowed_tags = Set.new([
"strong", "em", "b", "i", "p", "code", "pre", "tt", "samp", "kbd", "var", "sub",
"sup", "dfn", "cite", "big", "small", "address", "hr", "br", "div", "span", "h1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ class MiddlewareTest < TestCase
assert_match(%r(<script type="text/javascript">), html)
end

test "#call show can serve huge payloads" do
frames = { "1" => { name: "a" * 1e7 } }
profile = Profile.new(stackprof_profile(frames: frames))
id = Middleware.id(profile.file)

_, _, html = @app.call({ "PATH_INFO" => "/app_profiler/#{id}" })
html = html.first

assert_match(
%r{'Flamegraph for .*'\);\n</script>},
html[-200..-1],
message: "The generated HTML was incomplete"
)
end

test "#call viewer sets up yarn" do
@app.expects(:system).with("which", "yarn", out: File::NULL).returns(true)
@app.expects(:system).with("yarn", "init", "--yes").returns(true)
Expand Down

0 comments on commit 37e35d9

Please sign in to comment.