honeybadger-io / honeybadger-ruby

Ruby gem for reporting errors to honeybadger.io
https://docs.honeybadger.io/lib/ruby/
MIT License
250 stars 146 forks source link

HB::TraceCleaner::ActiveRecord blows up if SQL query contains invalid UTF-8 bytes #165

Closed aselder closed 8 years ago

aselder commented 8 years ago
s = "\xB2\xCE>\x1A\x8Aa\x1D"
Video.find_by_handle(s)

where Video is an ActiveRecord model and handle is a column on video returns nil if HoneyBadger NOT installed raises ArgumentError if HoneyBadger installed

     ArgumentError:
       invalid byte sequence in UTF-8
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/abstract_mysql_adapter.rb:253:in `split'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/abstract_mysql_adapter.rb:253:in `rescue in execute'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/abstract_mysql_adapter.rb:242:in `execute'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/mysql2_adapter.rb:213:in `execute'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/mysql2_adapter.rb:217:in `exec_query'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/mysql2_adapter.rb:226:in `select'
     # /Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/abstract/database_statements.rb:18:in `select_all'

This is actually an exception in the course of rescuing another exception. Using the debugger, the original exception is:

ActiveRecord::StatementInvalid: "ArgumentError: invalid byte sequence in UTF-8: SELECT videos.* FROM videos WHERE videos.handle = '\xB2\\\xCE>\\Z\x8Aa\u001D' LIMIT 1"

 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/honeybadger-2.3.0/lib/honeybadger/trace.rb:175:in `match'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/honeybadger-2.3.0/lib/honeybadger/trace.rb:175:in `match'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/honeybadger-2.3.0/lib/honeybadger/trace.rb:175:in `render?'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/honeybadger-2.3.0/lib/honeybadger/trace.rb:65:in `add_query'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/honeybadger-2.3.0/lib/honeybadger/init/rails.rb:36:in `block (2 levels) in <class:Railtie>'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/fanout.rb:47:in `call'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/fanout.rb:47:in `publish'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/fanout.rb:25:in `block in publish'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/fanout.rb:25:in `each'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/fanout.rb:25:in `publish'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activesupport-3.2.22/lib/active_support/notifications/instrumenter.rb:25:in `instrument'",
 "/Users/aselder/.rvm/gems/ruby-2.2.3@usertesting-orders/gems/activerecord-3.2.22/lib/active_record/connection_adapters/abstract_adapter.rb:275:in `log'"

I realize that we shouldn't be constructing strings that are not invalid UTF-8, however there are cases where we have parameters that are encoded, and if changed can decode to invalid UTF-8. Still, installing the HoneyBadger gem shouldn't cause errors in cases where the base rails app doesn't

aselder commented 8 years ago

Looking at the code both the render? and to_s methods will blow up if the sql contains invalid UTF-8 characters

joshuap commented 8 years ago

@aselder can you try locking your gem to github: 'honeybadger-io/honeybadger-ruby' and re-run your tests to confirm that 1d5f137d0135322cac0224bc5e0be78015850001 fixes this for you?

aselder commented 8 years ago

@joshuap Will do, give me about an hour

aselder commented 8 years ago

@joshuap It looks good to me

joshuap commented 8 years ago

Great, thanks! Working on a release.

joshuap commented 8 years ago

2.3.1 has been released.