Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent malformed exception messages from exiting RSpec #2761

Merged
merged 1 commit into from Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Changelog.md
Expand Up @@ -19,6 +19,8 @@ Bug Fixes:
* Predicates for pending examples, (in `RSpec::Core::Example`, `#pending?`, `#skipped?` and
`#pending_fixed?`) now return boolean values rather than truthy values.
(Marc-André Lafortune, #2756, #2758)
* Exceptions which have a message which cannot be cast to a string will no longer
cause a crash. (Jon Rowe, #2761)

### 3.9.2 / 2020-05-02
[Full Changelog](http://github.com/rspec/rspec-core/compare/v3.9.1...v3.9.2)
Expand Down
12 changes: 10 additions & 2 deletions lib/rspec/core/formatters/exception_presenter.rb
Expand Up @@ -51,7 +51,7 @@ def formatted_cause(exception)
cause << '--- Caused by: ---'
cause << "#{exception_class_name(last_cause)}:" unless exception_class_name(last_cause) =~ /RSpec/

encoded_string(last_cause.message.to_s).split("\n").each do |line|
encoded_string(exception_message_string(last_cause)).split("\n").each do |line|
cause << " #{line}"
end

Expand Down Expand Up @@ -174,11 +174,19 @@ def failure_slash_error_lines
lines
end

# rubocop:disable Lint/RescueException
def exception_message_string(exception)
exception.message.to_s
rescue Exception => other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NoMemoryError.superclass # => Exception

maybe rescue from StandardError here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the point is to avoid crashing due to calling this line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean we can't recover from a NoMemoryError by swallowing it.

On the other hand, SystemStackError is also an Exception, and it may be caused by an incorrectly implemented message. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well we might be able to because we're not necessarily assigning memory, but Ruby doesn't have a system level of exception unfortunately

"A #{exception.class} for which `exception.message.to_s` raises #{other.class}."
end
# rubocop:enable Lint/RescueException

def exception_lines
@exception_lines ||= begin
lines = []
lines << "#{exception_class_name}:" unless exception_class_name =~ /RSpec/
encoded_string(exception.message.to_s).split("\n").each do |line|
encoded_string(exception_message_string(exception)).split("\n").each do |line|
lines << (line.empty? ? line : " #{line}")
end
lines
Expand Down
26 changes: 26 additions & 0 deletions spec/rspec/core/formatters/exception_presenter_spec.rb
Expand Up @@ -260,6 +260,32 @@ def initialize(message, backtrace = [], cause = nil)
EOS
end

it 'will work then the message to_s raises a looped exception' do
raising_to_s_klass =
Class.new do
def to_s
raise StandardError, self
end
end

if RSpec::Support::Ruby.jruby?
expected_error = Java::JavaLang::StackOverflowError
else
expected_error = StandardError
end

incorrect_message_exception = FakeException.new(raising_to_s_klass.new, [])

the_presenter = Formatters::ExceptionPresenter.new(incorrect_message_exception, example)

expect(the_presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, ''))
|
| 1) Example
| Failure/Error: Unable to find matching line from backtrace
| A #{FakeException} for which `exception.message.to_s` raises #{expected_error}.
EOS
end

it "adds extra failure lines from the example metadata" do
extra_example = example.clone
failure_line = 'http://www.example.com/job_details/123'
Expand Down