From df6d2fbf2eb9a7bacd6f8a5ddecc1e78e1108f75 Mon Sep 17 00:00:00 2001 From: Mario Caropreso Date: Wed, 16 Aug 2023 20:05:45 +0000 Subject: [PATCH] Addressed an issue where syntax errors generated inside an eval method cause an Internal Server Error due to a TypeError. In order to display the extraced source of a syntax error, we try to locate the node id for the backtrace location. The call to RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location is expecting a Thread::Backtrace::Location object, but we are passing a SourceMapLocation instead. This commit does two things: 1) it addresses the issue by making sure that we are always passing a Thread::Backtrace::Location instead 2) it allows the development view to show the extracted source fragment --- .../middleware/exception_wrapper.rb | 9 ++++++-- .../test/dispatch/exception_wrapper_test.rb | 14 +++++++++++++ .../lib/active_support/syntax_error_proxy.rb | 21 ++++++++++++++++++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 02554cc6f2081..240ca0dce0d04 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -242,7 +242,7 @@ def initialize(location, template) end def spot(exc) - if RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location) + if RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location) && __getobj__.is_a?(Thread::Backtrace::Location) location = @template.spot(__getobj__) else location = super @@ -267,7 +267,12 @@ def build_backtrace (@exception.backtrace_locations || []).map do |loc| if built_methods.key?(loc.label.to_s) - SourceMapLocation.new(loc, built_methods[loc.label.to_s]) + thread_backtrace_location = if loc.respond_to?(:__getobj__) + loc.__getobj__ + else + loc + end + SourceMapLocation.new(thread_backtrace_location, built_methods[loc.label.to_s]) else loc end diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 676a1ccfb1b51..33d30d5f9577b 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -69,6 +69,20 @@ def backtrace end end + class_eval "def throw_syntax_error; eval %( + 'abc' + pluralize 'def' + ); end", "lib/file.rb", 42 + + test "#source_extracts works with eval syntax error" do + exception = begin throw_syntax_error; rescue SyntaxError => ex; ex; end + + wrapper = ExceptionWrapper.new(nil, TopErrorProxy.new(exception, 1)) + + assert_called_with(wrapper, :source_fragment, ["lib/file.rb", 42], returns: "foo") do + assert_equal [ code: "foo", line_number: 42 ], wrapper.source_extracts + end + end + if defined?(ErrorHighlight) && Gem::Version.new(ErrorHighlight::VERSION) >= Gem::Version.new("0.4.0") test "#source_extracts works with error_highlight" do lineno = __LINE__ diff --git a/activesupport/lib/active_support/syntax_error_proxy.rb b/activesupport/lib/active_support/syntax_error_proxy.rb index 8bfa37167de01..87eaca4376fe9 100644 --- a/activesupport/lib/active_support/syntax_error_proxy.rb +++ b/activesupport/lib/active_support/syntax_error_proxy.rb @@ -43,7 +43,26 @@ def backtrace_locations private def parse_message_for_trace - __getobj__.to_s.split("\n") + if source_location_eval? + # If the exception is coming from a call to eval, we need to keep + # the path of the file in which eval was called to ensure we can + # return the right source fragment to show the location of the + # error + location = __getobj__.backtrace_locations[0] + ["#{location.path}:#{location.lineno}: #{__getobj__}"] + else + __getobj__.to_s.split("\n") + end + end + + if SyntaxError.method_defined?(:path) # Ruby 3.3+ + def source_location_eval? + __getobj__.path.start_with?("(eval") + end + else # 3.2 and older versions of Ruby + def source_location_eval? + __getobj__.to_s.start_with?("(eval") + end end end end