Merge pull request rails#53696 from martinemde/martinemde/handle-multi-line-template-errors

Avoid crashing when highlighting ERB templates with multi-line methods

Author: byroot

actionview/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix a crash in ERB template error highlighting when the error occurs on a
+    line in the compiled template that is past the end of the source template.
+
+    *Martin Emde*
+
 *   Improve reliability of ERB template error highlighting.
     Fix infinite loops and crashes in highlighting and
     improve tolerance for alternate ERB handlers.

actionview/lib/action_view/template/handlers/erb.rb

@@ -42,7 +42,9 @@ def handles_encoding?
         # source location inside the template.
         def translate_location(spot, backtrace_location, source)
           # Tokenize the source line
-          tokens = ::ERB::Util.tokenize(source.lines[backtrace_location.lineno - 1])
+          source_lines = source.lines
+          return nil if source_lines.size < backtrace_location.lineno
+          tokens = ::ERB::Util.tokenize(source_lines[backtrace_location.lineno - 1])
           new_first_column = find_offset(spot[:snippet], tokens, spot[:first_column])
           lineno_delta = spot[:first_lineno] - backtrace_location.lineno
           spot[:first_lineno] -= lineno_delta
@@ -51,7 +53,7 @@ def translate_location(spot, backtrace_location, source)
           column_delta = spot[:first_column] - new_first_column
           spot[:first_column] -= column_delta
           spot[:last_column] -= column_delta
-          spot[:script_lines] = source.lines
+          spot[:script_lines] = source_lines
 
           spot
         rescue NotImplementedError, LocationParsingError

actionview/test/template/template_test.rb

@@ -339,10 +339,11 @@ def test_template_translate_location
     assert_equal expected, new_template(source).translate_location(backtrace_location, spot)
   end
 
+  # merely tests for the case where the backtrace and spot disagree about lineno
   def test_template_translate_location_lineno_offset
     highlight = "nomethoderror"
     source = "<%= nomethoderror %>"
-    compiled = "\n'.freeze; @output_buffer.append=  nomethoderror ; @output_buffer.safe_append='\n"
+    compiled = "'.freeze; @output_buffer.append=  nomethoderror ; @output_buffer.safe_append='"
 
     backtrace_location = Data.define(:lineno).new(lineno: 1)
     spot = spot_highlight(compiled, highlight, first_lineno: 2, last_lineno: 2)
@@ -351,6 +352,21 @@ def test_template_translate_location_lineno_offset
     assert_equal expected, new_template(source).translate_location(backtrace_location, spot)
   end
 
+  # We are testing the failure case here. `find_offset` doesn't correctly handle the case
+  # where the line number is not the same in the backtrace and template.
+  def test_template_translate_location_with_multiline_code_source
+    highlight = "nomethoderror"
+    source = "<%=\ngood(\n nomethoderror\n) %>"
+    extracted_line = " nomethoderror\n"
+    compiled = "ValidatedOutputBuffer.wrap(@output_buffer, ({}), ' \ngood(\n nomethoderror\n" \
+               ") '.freeze, true).safe_none_append=( \ngood(\n nomethoderror\n) );\n@output_buffer"
+
+    backtrace_location = Data.define(:lineno).new(lineno: 6)
+    spot = spot_highlight(compiled, highlight, first_column: 1, first_lineno: 6, last_lineno: 6, snippet: extracted_line)
+
+    assert_equal spot, new_template(source).translate_location(backtrace_location, spot)
+  end
+
   def test_template_translate_location_with_multibye_string_before_highlight
     highlight = "nomethoderror"
     source = String.new("\u{a5}<%= nomethoderror %>", encoding: Encoding::UTF_8) # yen symbol