From dffdd22bab341c18749033c36a7e476911d1dca2 Mon Sep 17 00:00:00 2001 From: Mike Perham Date: Wed, 27 Aug 2014 09:31:55 -0700 Subject: [PATCH] Cleanse error message bytes, fixes #1705 --- Changes.md | 1 + lib/sidekiq/middleware/server/retry_jobs.rb | 11 ++++++++++- test/test_retry.rb | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Changes.md b/Changes.md index 2dd9a230..e2379b1e 100644 --- a/Changes.md +++ b/Changes.md @@ -1,6 +1,7 @@ 3.2.3 ----------- +- Clean invalid bytes from error message before converting to JSON (requires Ruby 2.1+) [#1705] - Add queues list for each process to the Busy page. [davetoxa, #1897] - Fix for crash caused by empty config file. [jordan0day, #1901] - Add Rails Worker generator, `rails g sidekiq:worker User` will create `app/workers/user_worker.rb`. [seuros, #1909] diff --git a/lib/sidekiq/middleware/server/retry_jobs.rb b/lib/sidekiq/middleware/server/retry_jobs.rb index e1e9c758..b595848b 100644 --- a/lib/sidekiq/middleware/server/retry_jobs.rb +++ b/lib/sidekiq/middleware/server/retry_jobs.rb @@ -86,7 +86,16 @@ module Sidekiq else queue end - msg['error_message'] = e.message[0..10_000] + + # App code can stuff all sorts of crazy binary data into the error message + # that won't convert to JSON. + m = e.message[0..10_000] + if m.respond_to?(:scrub!) + m.force_encoding("utf-8") + m.scrub! + end + + msg['error_message'] = m msg['error_class'] = e.class.name count = if msg['retry_count'] msg['retried_at'] = Time.now.to_f diff --git a/test/test_retry.rb b/test/test_retry.rb index f511f220..281ef6a3 100644 --- a/test/test_retry.rb +++ b/test/test_retry.rb @@ -50,6 +50,22 @@ class TestRetry < Sidekiq::Test @redis.verify end + it 'handles zany characters in error message, #1705' do + @redis.expect :zadd, 1, ['retry', String, String] + msg = { 'class' => 'Bob', 'args' => [1,2,'foo'], 'retry' => 2 } + msg2 = msg.dup + handler = Sidekiq::Middleware::Server::RetryJobs.new + assert_raises RuntimeError do + handler.call(worker, msg2, 'default') do + raise "kerblammo! #{195.chr}" + end + end + msg2.delete('failed_at') + assert_equal({"class"=>"Bob", "args"=>[1, 2, "foo"], "retry"=>2, "queue"=>"default", "error_message"=>"kerblammo! �", "error_class"=>"RuntimeError", "retry_count"=>0}, msg2) + @redis.verify + end + + it 'allows a max_retries option in initializer' do max_retries = 7 1.upto(max_retries) do