From 3150e541fd655252ff0839b39cb37d6ae2412a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 15 Aug 2022 17:40:47 +0000 Subject: [PATCH 1/4] Update gemspec Fix email and remove deprecated test_files field. --- better_html.gemspec | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/better_html.gemspec b/better_html.gemspec index 054f653..6e08661 100644 --- a/better_html.gemspec +++ b/better_html.gemspec @@ -8,7 +8,7 @@ Gem::Specification.new do |s| s.name = "better_html" s.version = BetterHtml::VERSION s.authors = ["Francois Chagnon"] - s.email = ["francois.chagnon@shopify.com"] + s.email = ["ruby@shopify.com"] s.homepage = "https://github.com/Shopify/better-html" s.summary = "Better HTML for Rails." s.description = "Better HTML for Rails. Provides sane html helpers that make it easier to do the right thing." @@ -23,7 +23,6 @@ Gem::Specification.new do |s| s.extensions = ['ext/better_html_ext/extconf.rb'] s.files = Dir["{app,config,db,lib,ext}/**/*", "MIT-LICENSE", "Rakefile", "README.rdoc"] - s.test_files = Dir["test/**/*"] s.require_paths = ["lib"] s.add_dependency 'ast', '~> 2.0' From 260241ca5df609655d79c4770e52225c53f829a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 15 Aug 2022 17:41:59 +0000 Subject: [PATCH 2/4] Drop support to Ruby 2.6 --- .github/workflows/ruby.yml | 2 +- better_html.gemspec | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index afebfc7..817cac5 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -17,7 +17,7 @@ jobs: strategy: matrix: - ruby: [3.1, '3.0', 2.7, 2.6] + ruby: [3.1, '3.0', 2.7] steps: - uses: actions/checkout@v2 - name: Set up Ruby diff --git a/better_html.gemspec b/better_html.gemspec index 6e08661..4e573d3 100644 --- a/better_html.gemspec +++ b/better_html.gemspec @@ -14,6 +14,8 @@ Gem::Specification.new do |s| s.description = "Better HTML for Rails. Provides sane html helpers that make it easier to do the right thing." s.license = "MIT" + s.required_ruby_version = ">= 2.7.0" + s.metadata = { "bug_tracker_uri" => "https://github.com/Shopify/better-html/issues", "changelog_uri" => "https://github.com/Shopify/better-html/releases", From 4a21ee4c904f3ab86cb94b2e6cee94963ffd542f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 16 Aug 2022 00:05:17 +0000 Subject: [PATCH 3/4] Test with all supported Rails version and drop Rails < 6.0 --- .github/workflows/ruby.yml | 13 +- Gemfile.lock | 36 ++--- better_html.gemspec | 4 +- gemfiles/Gemfile-rails-6-0 | 13 ++ gemfiles/Gemfile-rails-6-1 | 13 ++ lib/better_html/better_erb.rb | 23 +--- .../better_erb/erubis_implementation.rb | 44 ------ lib/better_html/better_erb/runtime_checks.rb | 2 +- lib/better_html/config.rb | 1 + .../better_erb/implementation_test.rb | 128 +++++++++++------- 10 files changed, 139 insertions(+), 138 deletions(-) create mode 100644 gemfiles/Gemfile-rails-6-0 create mode 100644 gemfiles/Gemfile-rails-6-1 delete mode 100644 lib/better_html/better_erb/erubis_implementation.rb diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 817cac5..b218731 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -7,17 +7,22 @@ name: Ruby -on: [push, pull_requests] +on: [push, pull_request] jobs: test: - - name: Ruby ${{ matrix.ruby }} + name: Ruby ${{ matrix.ruby }} - ${{ matrix.gemfile }} runs-on: ubuntu-latest - strategy: + fail-fast: false matrix: ruby: [3.1, '3.0', 2.7] + gemfile: + - Gemfile + - gemfiles/Gemfile-rails-6-0 + - gemfiles/Gemfile-rails-6-1 + env: + BUNDLE_GEMFILE: ${{ matrix.gemfile }} steps: - uses: actions/checkout@v2 - name: Set up Ruby diff --git a/Gemfile.lock b/Gemfile.lock index 6a94788..8c6651f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,8 +2,8 @@ PATH remote: . specs: better_html (1.0.16) - actionview (>= 4.0) - activesupport (>= 4.0) + actionview (>= 6.0) + activesupport (>= 6.0) ast (~> 2.0) erubi (~> 1.4) parser (>= 2.4) @@ -12,35 +12,36 @@ PATH GEM remote: https://rubygems.org/ specs: - actionview (5.1.2) - activesupport (= 5.1.2) + actionview (7.0.3.1) + activesupport (= 7.0.3.1) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.0.3) - activesupport (5.1.2) + rails-html-sanitizer (~> 1.1, >= 1.2.0) + activesupport (7.0.3.1) concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (~> 0.7) - minitest (~> 5.1) - tzinfo (~> 1.1) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) ast (2.4.0) - builder (3.2.3) + builder (3.2.4) byebug (9.1.0) coderay (1.1.2) - concurrent-ruby (1.0.5) + concurrent-ruby (1.1.10) crass (1.0.6) - erubi (1.6.1) - i18n (0.8.6) + erubi (1.11.0) + i18n (1.12.0) + concurrent-ruby (~> 1.0) loofah (2.18.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) metaclass (0.0.4) method_source (0.9.0) mini_portile2 (2.8.0) - minitest (5.10.3) + minitest (5.16.2) mocha (1.2.1) metaclass (~> 0.0.1) - nokogiri (1.13.6) + nokogiri (1.13.8) mini_portile2 (~> 2.8.0) racc (~> 1.4) parser (2.6.3.0) @@ -61,9 +62,8 @@ GEM rake-compiler (1.2.0) rake smart_properties (1.15.0) - thread_safe (0.3.6) - tzinfo (1.2.10) - thread_safe (~> 0.1) + tzinfo (2.0.5) + concurrent-ruby (~> 1.0) PLATFORMS ruby diff --git a/better_html.gemspec b/better_html.gemspec index 4e573d3..547cd08 100644 --- a/better_html.gemspec +++ b/better_html.gemspec @@ -29,8 +29,8 @@ Gem::Specification.new do |s| s.add_dependency 'ast', '~> 2.0' s.add_dependency 'erubi', '~> 1.4' - s.add_dependency 'activesupport', '>= 4.0' - s.add_dependency 'actionview', '>= 4.0' + s.add_dependency 'activesupport', '>= 6.0' + s.add_dependency 'actionview', '>= 6.0' s.add_dependency 'parser', '>= 2.4' s.add_dependency 'smart_properties' diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0 new file mode 100644 index 0000000..d6ef028 --- /dev/null +++ b/gemfiles/Gemfile-rails-6-0 @@ -0,0 +1,13 @@ +source 'https://rubygems.org' + +gemspec path: ".." + +gem 'actionview', '~> 6.0.0' +gem 'rake' +gem 'rake-compiler' +gem 'minitest' +gem 'mocha' + +group :deployment, :test do + gem 'pry-byebug' +end diff --git a/gemfiles/Gemfile-rails-6-1 b/gemfiles/Gemfile-rails-6-1 new file mode 100644 index 0000000..27c7be6 --- /dev/null +++ b/gemfiles/Gemfile-rails-6-1 @@ -0,0 +1,13 @@ +source 'https://rubygems.org' + +gemspec path: ".." + +gem 'actionview', '~> 6.1.0' +gem 'rake' +gem 'rake-compiler' +gem 'minitest' +gem 'mocha' + +group :deployment, :test do + gem 'pry-byebug' +end diff --git a/lib/better_html/better_erb.rb b/lib/better_html/better_erb.rb index 7d8d23f..ca124bd 100644 --- a/lib/better_html/better_erb.rb +++ b/lib/better_html/better_erb.rb @@ -1,11 +1,7 @@ require 'action_view' require 'better_html_ext' -if ActionView.version < Gem::Version.new("5.1") -require 'better_html/better_erb/erubis_implementation' -else require 'better_html/better_erb/erubi_implementation' -end require 'better_html/better_erb/validated_output_buffer' module BetterHtml @@ -23,15 +19,10 @@ def initialize(message, position, line, column) class BetterHtml::BetterErb cattr_accessor :content_types - if ActionView.version < Gem::Version.new("5.1") - self.content_types = { - 'html.erb' => BetterHtml::BetterErb::ErubisImplementation - } - else - self.content_types = { - 'html.erb' => BetterHtml::BetterErb::ErubiImplementation - } - end + + self.content_types = { + 'html.erb' => BetterHtml::BetterErb::ErubiImplementation + } def self.prepend! ActionView::Template::Handlers::ERB.prepend(ConditionalImplementation) @@ -71,11 +62,7 @@ def generate(template, source) klass = BetterHtml::BetterErb.content_types[exts] unless excluded_template klass ||= self.class.erb_implementation - escape = if ActionView::VERSION::MAJOR <= 5 - self.class.escape_whitelist.include?(template.type) - else - self.class.escape_ignore_list.include?(template.type) - end + escape = self.class.escape_ignore_list.include?(template.type) generator = klass.new( erb, :escape => escape, diff --git a/lib/better_html/better_erb/erubis_implementation.rb b/lib/better_html/better_erb/erubis_implementation.rb deleted file mode 100644 index 99424a8..0000000 --- a/lib/better_html/better_erb/erubis_implementation.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'action_view' -require_relative 'runtime_checks' - -class BetterHtml::BetterErb - class ErubisImplementation < ActionView::Template::Handlers::Erubis - include RuntimeChecks - - def add_text(src, text) - return if text.empty? - - if text == "\n" - @parser.parse("\n") - @newline_pending += 1 - else - src << "@output_buffer.safe_append='" - src << "\n" * @newline_pending if @newline_pending > 0 - src << escape_text(text) - src << "'.freeze;" - - @parser.parse(text) do |*args| - check_token(*args) - end - - @newline_pending = 0 - end - end - - def add_expr_literal(src, code) - add_expr_auto_escaped(src, code, true) - end - - def add_expr_escaped(src, code) - add_expr_auto_escaped(src, code, false) - end - - def add_stmt(src, code) - flush_newline_if_pending(src) - - block_check(src, "<%#{code}%>") - @parser.append_placeholder(code) - super - end - end -end diff --git a/lib/better_html/better_erb/runtime_checks.rb b/lib/better_html/better_erb/runtime_checks.rb index 35b3ae0..6bd6b70 100644 --- a/lib/better_html/better_erb/runtime_checks.rb +++ b/lib/better_html/better_erb/runtime_checks.rb @@ -9,7 +9,7 @@ def initialize(erb, config: BetterHtml.config, **options) end def validate! - check_parser_errors + check_parser_errors unless @config.disable_parser_validation unless @parser.context == :none raise BetterHtml::HtmlError, 'Detected an open tag at the end of this document.' diff --git a/lib/better_html/config.rb b/lib/better_html/config.rb index ce418f9..234d9a0 100644 --- a/lib/better_html/config.rb +++ b/lib/better_html/config.rb @@ -12,6 +12,7 @@ class Config property :javascript_attribute_names, default: -> { [/\Aon/i] } property :template_exclusion_filter property :lodash_safe_javascript_expression, default: -> { [/\AJSON\.stringify\(/] } + property :disable_parser_validation, default: false def javascript_attribute_name?(name) javascript_attribute_names.any?{ |other| other === name.to_s } diff --git a/test/better_html/better_erb/implementation_test.rb b/test/better_html/better_erb/implementation_test.rb index a096484..11df8fb 100644 --- a/test/better_html/better_erb/implementation_test.rb +++ b/test/better_html/better_erb/implementation_test.rb @@ -3,6 +3,8 @@ require 'better_html/better_erb' require 'json' +BetterHtml::BetterErb.prepend! + class BetterHtml::BetterErb::ImplementationTest < ActiveSupport::TestCase test "simple template rendering" do assert_equal "some value", @@ -25,18 +27,20 @@ class BetterHtml::BetterErb::ImplementationTest < ActiveSupport::TestCase end test "interpolate html_safe inside attribute is magically force-escaped" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("\">", locals: { value: ' \'">x '.html_safe }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the interpolation "\ "into a quoted attribute value. The value cannot contain the character \".", e.message end test "interpolate html_safe inside single quoted attribute" do config = build_config(allow_single_quoted_attributes: true) - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("\'>", config: config, locals: { value: ' \'">x '.html_safe }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the interpolation "\ "into a quoted attribute value. The value cannot contain the character '.", e.message end @@ -47,25 +51,28 @@ class BetterHtml::BetterErb::ImplementationTest < ActiveSupport::TestCase end test "interpolate in attribute name with unsafe value with spaces" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: "un safe" }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the interpolation "\ "into a attribute name around 'data-<%= value %>'.", e.message end test "interpolate in attribute name with unsafe value with equal sign" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: "un=safe" }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the "\ "interpolation into a attribute name around 'data-<%= value %>'.", e.message end test "interpolate in attribute name with unsafe value with quote" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: "un\"safe" }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the "\ "interpolation into a attribute name around 'data-<%= value %>'.", e.message end @@ -76,37 +83,41 @@ class BetterHtml::BetterErb::ImplementationTest < ActiveSupport::TestCase end test "interpolate after an attribute name with equal sign" do - config = build_config(allow_unquoted_attributes: true) - e = assert_raises(BetterHtml::DontInterpolateHere) do - render(">", config: config) + config = build_config(allow_unquoted_attributes: true, disable_parser_validation: true) + e = assert_raises(ActionView::Template::Error) do + render(">", config: config) end + assert_kind_of BetterHtml::DontInterpolateHere, e.cause assert_equal "Do not interpolate without quotes after "\ "attribute around 'data-foo=<%= html_attributes(foo: 'bar') %>'.", e.message end test "interpolate after an attribute value" do - e = assert_raises(BetterHtml::DontInterpolateHere) do + e = assert_raises(ActionView::Template::Error) do render(">") end + assert_kind_of BetterHtml::DontInterpolateHere, e.cause assert_equal "Add a space after this attribute value. "\ "Instead of >"\ " try >.", e.message end test "interpolate in attribute without quotes" do - config = build_config(allow_unquoted_attributes: true) - e = assert_raises(BetterHtml::DontInterpolateHere) do + config = build_config(allow_unquoted_attributes: true, disable_parser_validation: true) + e = assert_raises(ActionView::Template::Error) do render(">", config: config, locals: { value: "un safe" }) end + assert_kind_of BetterHtml::DontInterpolateHere, e.cause assert_equal "Do not interpolate without quotes after "\ "attribute around 'href=<%= value %>'.", e.message end test "interpolate in attribute after value" do config = build_config(allow_unquoted_attributes: true) - e = assert_raises(BetterHtml::DontInterpolateHere) do + e = assert_raises(ActionView::Template::Error) do render(">", config: config, locals: { value: "" }) end + assert_kind_of BetterHtml::DontInterpolateHere, e.cause assert_equal "Do not interpolate without quotes around this "\ "attribute value. Instead of > "\ "try \">.", e.message @@ -118,25 +129,28 @@ class BetterHtml::BetterErb::ImplementationTest < ActiveSupport::TestCase end test "interpolate in tag name with space" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: "un safe" }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the interpolation "\ "into a tag name around: >.", e.message end test "interpolate in tag name with slash" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: "un/safe" }) end + assert_kind_of BetterHtml::UnsafeHtmlError, e.cause assert_equal "Detected invalid characters as part of the interpolation "\ "into a tag name around: >.", e.message end test "interpolate in tag name with end of tag" do - e = assert_raises(BetterHtml::UnsafeHtmlError) do + e = assert_raises(ActionView::Template::Error) do render("-foo>", locals: { value: ">