Skip to content

Commit c43d8ba

Browse files
authored
Backport HTML safety fix for 2.x (#1962)
1 parent 6c8d2ad commit c43d8ba

17 files changed

+115
-12
lines changed

docs/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ nav_order: 5
1010

1111
## main
1212

13+
* Ensure HTML output safety.
14+
15+
*Cameron Dutro*
16+
1317
## 2.82.0
1418

1519
* Revert "Avoid loading ActionView::Base during initialization (#1528)"

lib/view_component/base.rb

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,12 @@ def render_in(view_context, &block)
130130
before_render
131131

132132
if render?
133-
render_template_for(@__vc_variant).to_s + output_postamble
133+
# Avoid allocating new string when output_postamble is blank
134+
if output_postamble.blank?
135+
safe_render_template_for(@__vc_variant).to_s
136+
else
137+
safe_render_template_for(@__vc_variant).to_s + safe_output_postamble
138+
end
134139
else
135140
""
136141
end
@@ -157,7 +162,7 @@ def render_parent
157162
#
158163
# @return [String]
159164
def output_postamble
160-
""
165+
@@default_output_postamble ||= "".html_safe
161166
end
162167

163168
# Called before rendering the component. Override to perform operations that
@@ -309,6 +314,38 @@ def content_evaluated?
309314
@__vc_content_evaluated
310315
end
311316

317+
def maybe_escape_html(text)
318+
return text if request && !request.format.html?
319+
return text if text.blank?
320+
321+
if text.html_safe?
322+
text
323+
else
324+
yield
325+
html_escape(text)
326+
end
327+
end
328+
329+
def safe_render_template_for(variant)
330+
if compiler.renders_template_for_variant?(variant)
331+
render_template_for(variant)
332+
else
333+
maybe_escape_html(render_template_for(variant)) do
334+
Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.")
335+
end
336+
end
337+
end
338+
339+
def safe_output_postamble
340+
maybe_escape_html(output_postamble) do
341+
Kernel.warn("WARNING: The #{self.class} component was provided an HTML-unsafe postamble. The postamble will be automatically escaped, but you may want to investigate.")
342+
end
343+
end
344+
345+
def compiler
346+
@compiler ||= self.class.compiler
347+
end
348+
312349
# Set the controller used for testing components:
313350
#
314351
# ```ruby

lib/view_component/compiler.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class Compiler
1616
def initialize(component_class)
1717
@component_class = component_class
1818
@redefinition_lock = Mutex.new
19+
@variants_rendering_templates = Set.new
1920
end
2021

2122
def compiled?
@@ -61,6 +62,7 @@ def compile(raise_errors: false, force: false)
6162
# Remove existing compiled template methods,
6263
# as Ruby warns when redefining a method.
6364
method_name = call_method_name(template[:variant])
65+
@variants_rendering_templates << template[:variant]
6466

6567
redefinition_lock.synchronize do
6668
component_class.silence_redefinition_of_method(method_name)
@@ -81,6 +83,10 @@ def #{method_name}
8183
CompileCache.register(component_class)
8284
end
8385

86+
def renders_template_for_variant?(variant)
87+
@variants_rendering_templates.include?(variant)
88+
end
89+
8490
private
8591

8692
attr_reader :component_class, :redefinition_lock

test/sandbox/app/components/after_render_component.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
class AfterRenderComponent < ViewComponent::Base
44
def call
5-
"Hello, "
5+
"Hello, ".html_safe
66
end
77

88
def output_postamble
9-
"World!"
9+
"World!".html_safe
1010
end
1111
end

test/sandbox/app/components/custom_test_controller_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22

33
class CustomTestControllerComponent < ViewComponent::Base
44
def call
5-
helpers.foo
5+
html_escape(helpers.foo)
66
end
77
end

test/sandbox/app/components/deprecated_slots_setter_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ class DeprecatedSlotsSetterComponent < ViewComponent::Base
88

99
def call
1010
header
11-
items
11+
html_escape(items)
1212
end
1313
end

test/sandbox/app/components/inherited_from_uncompilable_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22

33
class InheritedFromUncompilableComponent < UncompilableComponent
44
def call
5-
"<div>hello world</div>"
5+
"<div>hello world</div>".html_safe
66
end
77
end

test/sandbox/app/components/inline_render_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ def initialize(items:)
66
end
77

88
def call
9-
@items.map { |c| render(c) }.join
9+
@items.map { |c| render(c) }.join.html_safe
1010
end
1111
end

test/sandbox/app/components/message_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ def initialize(message:)
66
end
77

88
def call
9-
@message
9+
html_escape(@message)
1010
end
1111
end
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# frozen_string_literal: true
2+
3+
class UnsafeComponent < ViewComponent::Base
4+
def call
5+
user_input = "<script>alert('hello!')</script>"
6+
7+
"<div>hello #{user_input}</div>"
8+
end
9+
end

0 commit comments

Comments
 (0)