From bb626ffca82317ae2cd02543785a7b78c2bba9c1 Mon Sep 17 00:00:00 2001 From: Akhil G Krishnan Date: Wed, 6 Sep 2023 19:10:49 +0000 Subject: [PATCH] HTML tag validation added for tags Added validation for HTML tag names in the `tag` and `content_tag` helper method. The `tag` and `content_tag` method now checks that the provided tag name adheres to the HTML specification. If an invalid HTML tag name is provided, the method raises an `ArgumentError` with an appropriate error message. Examples: ```ruby content_tag("12p") # Starting with a number content_tag("") # Empty tag name tag("div/") # Contains a solidus tag("image file") # Contains a space ``` --- actionview/CHANGELOG.md | 21 ++++++++++ .../lib/action_view/helpers/tag_helper.rb | 10 +++-- actionview/test/template/tag_helper_test.rb | 42 ++++++++++++++----- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index ed66f6466c58f..e3b04726b00a6 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,2 +1,23 @@ +* Added validation for HTML tag names in the `tag` and `content_tag` helper method. The `tag` and + `content_tag` method now checks that the provided tag name adheres to the HTML specification. If + an invalid HTML tag name is provided, the method raises an `ArgumentError` with an appropriate error + message. + Examples: + + ```ruby + # Raises ArgumentError: Invalid HTML5 tag name: 12p + content_tag("12p") # Starting with a number + + # Raises ArgumentError: Invalid HTML5 tag name: "" + content_tag("") # Empty tag name + + # Raises ArgumentError: Invalid HTML5 tag name: div/ + tag("div/") # Contains a solidus + + # Raises ArgumentError: Invalid HTML5 tag name: "image file" + tag("image file") # Contains a space + ``` + + *Akhil G Krishnan* Please check [7-1-stable](https://github.com/rails/rails/blob/7-1-stable/actionview/CHANGELOG.md) for previous changes. diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb index 095f05813d98e..f59086bb7c908 100644 --- a/actionview/lib/action_view/helpers/tag_helper.rb +++ b/actionview/lib/action_view/helpers/tag_helper.rb @@ -79,11 +79,10 @@ def tag_string(name, content = nil, escape: true, **options, &block) def content_tag_string(name, content, options, escape = true) tag_options = tag_options(options, escape) if options + TagHelper.ensure_valid_html5_tag_name(name) if escape - name = ERB::Util.xml_name_escape(name) content = ERB::Util.unwrapped_html_escape(content) end - "<#{name}#{tag_options}>#{PRE_CONTENT_STRINGS[name]}#{content}".html_safe end @@ -310,7 +309,7 @@ def tag(name = nil, options = nil, open = false, escape = true) if name.nil? tag_builder else - name = ERB::Util.xml_name_escape(name) if escape + ensure_valid_html5_tag_name(name) "<#{name}#{tag_builder.tag_options(options, escape) if options}#{open ? ">" : " />"}".html_safe end end @@ -400,6 +399,11 @@ def escape_once(html) end private + def ensure_valid_html5_tag_name(name) + raise ArgumentError, "Invalid HTML5 tag name: #{name.inspect}" unless /\A[a-zA-Z][^\s\/>]*\z/.match?(name) + end + module_function :ensure_valid_html5_tag_name + def build_tag_values(*args) tag_values = [] diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 1b18b6d546e2c..0685ebeb0324e 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -8,6 +8,7 @@ class TagHelperTest < ActionView::TestCase tests ActionView::Helpers::TagHelper COMMON_DANGEROUS_CHARS = "&<>\"' %*+,/;=^|" + INVALID_TAG_CHARS = "> /" def test_tag assert_equal "
", tag("br") @@ -122,20 +123,21 @@ def test_tag_builder_do_not_modify_html_safe_options end def test_tag_with_dangerous_name - assert_equal "<#{"_" * COMMON_DANGEROUS_CHARS.size} />", - tag(COMMON_DANGEROUS_CHARS) - - assert_equal "<#{COMMON_DANGEROUS_CHARS} />", - tag(COMMON_DANGEROUS_CHARS, nil, false, false) + INVALID_TAG_CHARS.each_char do |char| + tag_name = "asdf-#{char}" + assert_raise(ArgumentError, "expected #{tag_name.inspect} to be invalid") do + tag(tag_name) + end + end end def test_tag_builder_with_dangerous_name - escaped_dangerous_chars = "_" * COMMON_DANGEROUS_CHARS.size - assert_equal "<#{escaped_dangerous_chars}>", - tag.public_send(COMMON_DANGEROUS_CHARS.to_sym) - - assert_equal "<#{COMMON_DANGEROUS_CHARS}>", - tag.public_send(COMMON_DANGEROUS_CHARS.to_sym, nil, escape: false) + INVALID_TAG_CHARS.each_char do |char| + tag_name = "asdf-#{char}".to_sym + assert_raise(ArgumentError, "expected #{tag_name.inspect} to be invalid") do + tag.public_send(tag_name) + end + end end def test_tag_with_dangerous_aria_attribute_name @@ -426,6 +428,24 @@ def test_content_tag_with_unescaped_conditional_hash_classes assert_equal "

\">limelight

", str end + def test_content_tag_with_invalid_html_tag + invalid_tags = ["12p", "", "image file", "div/", "my>element", "_header"] + invalid_tags.each do |tag_name| + assert_raise(ArgumentError, "expected #{tag_name.inspect} to be invalid") do + content_tag(tag_name) + end + end + end + + def test_tag_with_invalid_html_tag + invalid_tags = ["12p", "", "image file", "div/", "my>element", "_header"] + invalid_tags.each do |tag_name| + assert_raise(ArgumentError, "expected #{tag_name.inspect} to be invalid") do + tag(tag_name) + end + end + end + def test_tag_builder_with_unescaped_conditional_hash_classes str = tag.p "limelight", class: { "song": true, "play>": true }, escape: false assert_equal "

\">limelight

", str