Skip to content

Issue #797: Handle multi-byte body in Net::HTTP on ruby 2.6.0 #814

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 1, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions lib/rollbar/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,10 @@ def extract_arguments(args)
exception = arg
elsif arg.is_a?(Hash)
extra = arg

context = extra[:custom_data_method_context]
extra.delete :custom_data_method_context

extra = nil if extra.empty?
end
end
Expand Down Expand Up @@ -411,7 +411,7 @@ def report(level, message, exception, extra, context)
# If that fails, we'll fall back to a more static failsafe response.
def report_internal_error(exception)
log_error '[Rollbar] Reporting internal error encountered while sending data to Rollbar.'

configuration.execute_hook(:on_report_internal_error, exception)

begin
Expand Down Expand Up @@ -517,12 +517,29 @@ def do_post(uri, body, access_token)

request = Net::HTTP::Post.new(uri.request_uri)

request.body = body
request.body = pack_ruby260_bytes(body)
request.add_field('X-Rollbar-Access-Token', access_token)

handle_net_retries { http.request(request) }
end

def pack_ruby260_bytes(body)
# Ruby 2.6.0 shipped with a bug affecting multi-byte body for Net::HTTP.
# Fix (committed one day after 2.6.0p0 shipped) is here:
# https://github.com/ruby/ruby/commit/1680a13a926b17661329beec1ded6b32aad16c1b#diff-00a99d8c71daaf5fc60a050da41f7261
#
# We work around this by repacking the body as single byte chars if needed.
if RUBY_VERSION == '2.6.0' && multibyte?(body)
body.unpack('C*').pack('C*')
else
body
end
end

def multibyte?(str)
str.chars.length != str.bytes.length
end

def http_proxy_for_em(uri)
proxy = http_proxy(uri)
{
Expand Down
24 changes: 24 additions & 0 deletions spec/rollbar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,30 @@
notifier.log('error', exception, extra_data, 'exception description')
end

# See Notifier#pack_ruby260_bytes for more information.
context 'with multi-byte characters in the report' do
extra_data = { :key => "\u3042", :hash => { :inner_key => 'あああ' } }

it 'should send as multi-byte on ruby != 2.6.0',
:if => RUBY_VERSION >= '2.0.0' && RUBY_VERSION != '2.6.0' do
expect_any_instance_of(Net::HTTP::Post).to receive(:body=).with(
satisfy do |body|
body.chars.length < body.bytes.length && body.include?('あああ')
end
).and_call_original
notifier.log('error', 'test message', extra_data)
end

it 'should unpack multi-byte and send as single byte on ruby == 2.6.0', :if => RUBY_VERSION == '2.6.0' do
expect_any_instance_of(Net::HTTP::Post).to receive(:body=).with(
satisfy do |body|
body.chars.length == body.bytes.length && body.force_encoding('utf-8').include?('あああ')
end
).and_call_original
notifier.log('error', 'test message', extra_data)
end
end

context 'with :on_error_response hook configured' do
let!(:notifier) { Rollbar::Notifier.new }
let(:configuration) do
Expand Down