diff --git a/Gemfile b/Gemfile index 105270f7..97dd488f 100644 --- a/Gemfile +++ b/Gemfile @@ -31,16 +31,6 @@ else gem 'sqlite3', '~> 1.4', :platform => [:ruby, :mswin, :mingw] # rubocop:disable Bundler/DuplicatedGem end -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' # rubocop:disable Bundler/DuplicatedGem - else - gem 'oj', '~> 2.12.14' # rubocop:disable Bundler/DuplicatedGem - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails30.gemfile b/gemfiles/rails30.gemfile index 89c95a67..a7abfd2f 100644 --- a/gemfiles/rails30.gemfile +++ b/gemfiles/rails30.gemfile @@ -20,14 +20,6 @@ gem 'hitimes', '< 1.2.2' gem 'rake', '< 11' gem 'rspec-rails', '>= 2.14.0' -unless is_jruby - if RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails31.gemfile b/gemfiles/rails31.gemfile index e55da050..d762cfda 100644 --- a/gemfiles/rails31.gemfile +++ b/gemfiles/rails31.gemfile @@ -19,14 +19,6 @@ gem 'rails', '3.1.12' gem 'rspec-rails', '~> 3.4' gem 'rake' -unless is_jruby - if RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails32.gemfile b/gemfiles/rails32.gemfile index 6d1996d4..f2859576 100644 --- a/gemfiles/rails32.gemfile +++ b/gemfiles/rails32.gemfile @@ -21,14 +21,6 @@ gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw] # Please see https://github.com/rspec/rspec-rails/issues/1273 gem 'test-unit' -unless is_jruby - if RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails40.gemfile b/gemfiles/rails40.gemfile index 496655a5..bb26d1fa 100644 --- a/gemfiles/rails40.gemfile +++ b/gemfiles/rails40.gemfile @@ -21,14 +21,6 @@ gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw] # Please see https://github.com/rspec/rspec-rails/issues/1273 gem 'test-unit' -unless is_jruby - if RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails41.gemfile b/gemfiles/rails41.gemfile index 478b60ed..5a9ce132 100644 --- a/gemfiles/rails41.gemfile +++ b/gemfiles/rails41.gemfile @@ -19,14 +19,6 @@ gem 'rake' gem 'rspec-rails', '~> 3.4' gem 'sqlite3', '< 1.4.0', :platform => [:ruby, :mswin, :mingw] -unless is_jruby - if RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails42.gemfile b/gemfiles/rails42.gemfile index 0c339ad3..25905178 100644 --- a/gemfiles/rails42.gemfile +++ b/gemfiles/rails42.gemfile @@ -27,16 +27,6 @@ platforms :rbx do gem 'rubysl', '~> 2.0' unless RUBY_VERSION.start_with?('1') end -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - if RUBY_VERSION < '2.2.2' gem 'sidekiq', '~> 2.13.0' else diff --git a/gemfiles/rails50.gemfile b/gemfiles/rails50.gemfile index 9b36d66f..e8c3b0eb 100644 --- a/gemfiles/rails50.gemfile +++ b/gemfiles/rails50.gemfile @@ -27,16 +27,6 @@ gem 'rake' gem 'sidekiq', '>= 2.13.0' -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - platforms :rbx do gem 'minitest' gem 'racc' diff --git a/gemfiles/rails51.gemfile b/gemfiles/rails51.gemfile index c84a6566..436b161d 100644 --- a/gemfiles/rails51.gemfile +++ b/gemfiles/rails51.gemfile @@ -27,16 +27,6 @@ gem 'rake' gem 'sidekiq', '>= 2.13.0' -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - platforms :rbx do gem 'minitest' gem 'racc' diff --git a/gemfiles/rails52.gemfile b/gemfiles/rails52.gemfile index 872281b6..952d5de2 100644 --- a/gemfiles/rails52.gemfile +++ b/gemfiles/rails52.gemfile @@ -25,16 +25,6 @@ gem 'rspec-mocks', '~> 3.8.0' gem 'rake' -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - gem 'sidekiq', '>= 2.13.0' platforms :rbx do diff --git a/gemfiles/rails60.gemfile b/gemfiles/rails60.gemfile index 302fb062..2719e74a 100644 --- a/gemfiles/rails60.gemfile +++ b/gemfiles/rails60.gemfile @@ -27,16 +27,6 @@ gem 'rspec-rails', :git => 'https://github.com/rspec/rspec-rails', :ref => 'v4.0 gem 'rake' -unless is_jruby - if RUBY_VERSION >= '2.5' - gem 'oj' - elsif RUBY_VERSION >= '2.4.0' - gem 'oj', '~> 2.16.1' - else - gem 'oj', '~> 2.12.14' - end -end - gem 'sidekiq', '>= 2.13.0' platforms :rbx do diff --git a/lib/rollbar/json.rb b/lib/rollbar/json.rb index f612abdf..6081ec31 100644 --- a/lib/rollbar/json.rb +++ b/lib/rollbar/json.rb @@ -1,13 +1,5 @@ -require 'multi_json' -require 'rollbar/json/oj' -require 'rollbar/json/default' require 'rollbar/language_support' -begin - require 'oj' -rescue LoadError -end - module Rollbar module JSON # :nodoc: extend self @@ -15,54 +7,13 @@ module JSON # :nodoc: attr_writer :options_module def dump(object) - # `basic_socket` plugin addresses the following issue: https://github.com/rollbar/rollbar-gem/issues/845 Rollbar.plugins.get('basic_socket').load_scoped!(true) do - with_adapter { MultiJson.dump(object, adapter_options) } + ::JSON.generate(object) end end def load(string) - with_adapter { MultiJson.load(string, adapter_options) } - end - - def with_adapter(&block) - MultiJson.with_adapter(detect_multi_json_adapter, &block) - end - - def detect_multi_json_adapter - options = {} - options[:adapter] = :oj if defined?(::Oj) - - MultiJson.current_adapter(options) - end - - def adapter_options - options_module.options - end - - def options_module - @options_module ||= find_options_module - end - - def find_options_module - module_name = multi_json_adapter_module_name - - if LanguageSupport.const_defined?(Rollbar::JSON, module_name, false) - LanguageSupport.const_get(Rollbar::JSON, module_name, false) - else - Default - end - end - - # MultiJson adapters have this name structure: - # "MultiJson::Adapters::{AdapterModule}" - # - # Ex: MultiJson::Adapters::Oj - # Ex: MultiJson::Adapters::JsonGem - # - # In this method we just get the last module name. - def multi_json_adapter_module_name - detect_multi_json_adapter.name[/^MultiJson::Adapters::(.*)$/, 1] + ::JSON.parse(string) end end end diff --git a/lib/rollbar/json/default.rb b/lib/rollbar/json/default.rb deleted file mode 100644 index d10192eb..00000000 --- a/lib/rollbar/json/default.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Rollbar - module JSON - module Default - module_function - - def options - {} - end - end - end -end diff --git a/lib/rollbar/json/oj.rb b/lib/rollbar/json/oj.rb deleted file mode 100644 index 42a64073..00000000 --- a/lib/rollbar/json/oj.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Rollbar - module JSON - module Oj - module_function - - def options - { - :mode => :compat, - :use_to_json => false, - :symbol_keys => false, - :circular => false - } - end - end - end -end diff --git a/lib/rollbar/plugins/basic_socket.rb b/lib/rollbar/plugins/basic_socket.rb index 91855286..5cb16f84 100644 --- a/lib/rollbar/plugins/basic_socket.rb +++ b/lib/rollbar/plugins/basic_socket.rb @@ -6,7 +6,7 @@ # Needed to avoid active_support (< 4.1.0) bug serializing JSONs dependency do defined?(ActiveSupport::VERSION::STRING) && - Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('5.2.0') + Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('4.1.0') end execute do diff --git a/lib/rollbar/util/hash.rb b/lib/rollbar/util/hash.rb index 8bf2925b..336c6200 100644 --- a/lib/rollbar/util/hash.rb +++ b/lib/rollbar/util/hash.rb @@ -5,6 +5,7 @@ def self.deep_stringify_keys(hash, seen = {}) return if seen[hash.object_id] seen[hash.object_id] = true + replace_seen_children(hash, seen) hash.reduce({}) do |h, (key, value)| h[key.to_s] = map_value(value, :deep_stringify_keys, seen) @@ -22,12 +23,26 @@ def self.map_value(thing, meth, seen) thing else seen[thing.object_id] = true + replace_seen_children(thing, seen) thing.map { |v| map_value(v, meth, seen) } end else thing end end + + def self.replace_seen_children(thing, seen) + case thing + when ::Hash + thing.keys.each do |key| + thing[key] = "removed circular reference: #{thing[key]}" if seen[thing[key].object_id] + end + when Array + thing.each_with_index do |_, i| + thing[i] = "removed circular reference: #{thing[i]}" if seen[thing[i].object_id] + end + end + end end end end diff --git a/rollbar.gemspec b/rollbar.gemspec index d3f567f6..15ce6b28 100644 --- a/rollbar.gemspec +++ b/rollbar.gemspec @@ -18,8 +18,6 @@ Gem::Specification.new do |gem| gem.required_ruby_version = '>= 1.9.3' gem.version = Rollbar::VERSION - gem.add_runtime_dependency 'multi_json' - if gem.respond_to?(:metadata) gem.metadata['changelog_uri'] = 'https://github.com/rollbar/rollbar-gem/releases' gem.metadata['source_code_uri'] = 'https://github.com/rollbar/rollbar-gem' diff --git a/spec/rollbar/item_spec.rb b/spec/rollbar/item_spec.rb index e8cf66be..9dc1ed2d 100644 --- a/spec/rollbar/item_spec.rb +++ b/spec/rollbar/item_spec.rb @@ -687,17 +687,34 @@ def as_json(*) end let(:item) { Rollbar::Item.build_with(payload) } - it 'fails in ActiveSupport with stack too deep' do + it "doesn't fail in ActiveSupport >= 4.1" do begin _json = item.dump - rescue NoMemoryError, SystemStackError, Java::JavaLang::StackOverflowError + + # If you get an uninitialized constant "Java" error, it just means an unexpected exception + # occurred (i.e. one not in the below list) and caused Java::JavaLang::StackOverflowError. + # If the test case is working correctly, this shouldn't happen ang the Java error type + # will only be evaluated on JRuby builds. + rescue NoMemoryError, + SystemStackError, + ActiveSupport::JSON::Encoding::CircularReferenceError, + Java::JavaLang::StackOverflowError + + # If ActiveSupport is invoked, we'll end up here. # Item#dump fails with SystemStackError (ActiveSupport > 4.0) # or NoMemoryError (ActiveSupport <= 4.0) which, as system exceptions # not a StandardError, cannot be tested by `expect().to raise_error` error = :SystemError end - expect(error).to be_eql(:SystemError) + if Gem::Version.new(ActiveSupport::VERSION::STRING) >= Gem::Version.new('4.1.0') + expect(error).not_to be_eql(:SystemError) + else + # This ActiveSupport is vulnerable to circular reference errors, and is + # virtually impossible to correct, because these versions of AS even hook into + # core Ruby JSON. + expect(error).to be_eql(:SystemError) + end end end diff --git a/spec/rollbar/json/oj_spec.rb b/spec/rollbar/json/oj_spec.rb deleted file mode 100644 index 936ec8f9..00000000 --- a/spec/rollbar/json/oj_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -require 'spec_helper' - -require 'rollbar/json/oj' - -describe Rollbar::JSON::Oj do - let(:options) do - { - :mode => :compat, - :use_to_json => false, - :symbol_keys => false, - :circular => false - } - end - - it 'returns correct options' do - expect(described_class.options).to be_eql(options) - end -end diff --git a/spec/rollbar/json_spec.rb b/spec/rollbar/json_spec.rb index 0423db77..2007d303 100644 --- a/spec/rollbar/json_spec.rb +++ b/spec/rollbar/json_spec.rb @@ -1,122 +1,26 @@ require 'spec_helper' -require 'multi_json' require 'rollbar/json' require 'rollbar/configuration' -class Rollbar::JSON::MockAdapter - def self.options - { 'mock' => 'adapter' } - end -end - -module MultiJson - module Adapters - module MockAdapter - end - end -end - -module MultiJson - module Adapters - module MissingCustomOptions - end - end -end - -module MissingCustomOptions - # Consider the fact that there's MultiJson::Adapters::Yajl but not - # Rollbar::JSON::Yajl, it should not look for ::Yajl but only - # Rollbar::JSON::Yajl. -end - describe Rollbar::JSON do let(:payload) do { :foo => :bar } end - let(:adapter_options) { { 'option' => 'value' } } describe '.dump' do - before do - allow(described_class).to receive(:adapter_options).and_return(adapter_options) - end - - it 'calls MultiJson.dump' do - expect(::MultiJson).to receive(:dump).once.with(payload, adapter_options) + it 'calls JSON.generate' do + expect(::JSON).to receive(:generate).once.with(payload) described_class.dump(payload) end - - context 'with basic_socket plugin unloadable' do - before do - Rollbar.configuration.disable_core_monkey_patch = true - end - - it 'still calls MultiJson.dump' do - expect(::MultiJson).to receive(:dump).once.with(payload, adapter_options) - - described_class.dump(payload) - end - end end describe '.load' do - before do - allow(described_class).to receive(:adapter_options).and_return(adapter_options) - end - - it 'calls MultiJson.load' do - expect(::MultiJson).to receive(:load).once.with(payload, adapter_options) + it 'calls JSON.parse' do + expect(::JSON).to receive(:parse).once.with(payload) described_class.load(payload) end end - - describe '.with_adapter' do - let(:object) { double(:foo => 'bar') } - let(:callback) do - proc { object.foo } - end - let(:adapter) { described_class.detect_multi_json_adapter } - - it 'calls mock.something with an adapter' do - expect(MultiJson).to receive(:with_adapter).with(adapter).and_call_original - expect(object).to receive(:foo).once - - described_class.with_adapter(&callback) - end - end - - describe '.adapter_options' do - it 'calls .options in adapter module' do - expect(described_class.options_module).to receive(:options) - - described_class.adapter_options - end - end - - describe '.options_module' do - before do - described_class.options_module = nil - allow(MultiJson).to receive(:current_adapter).and_return(multi_json_module) - end - - context 'with a defined rollbar adapter' do - let(:multi_json_module) { MultiJson::Adapters::MockAdapter } - let(:expected_adapter) { Rollbar::JSON::MockAdapter } - - it 'returns the correct options' do - expect(described_class.options_module).to be(expected_adapter) - end - end - - context 'without a defined rollbar adapter' do - let(:multi_json_module) { MultiJson::Adapters::MissingCustomOptions } - let(:expected_adapter) { Rollbar::JSON::Default } - - it 'returns the correct options' do - expect(described_class.options_module).to be(expected_adapter) - end - end - end end diff --git a/spec/rollbar/plugins/basic_socket_spec.rb b/spec/rollbar/plugins/basic_socket_spec.rb index 286a18f3..f1eedc96 100644 --- a/spec/rollbar/plugins/basic_socket_spec.rb +++ b/spec/rollbar/plugins/basic_socket_spec.rb @@ -34,8 +34,8 @@ context 'with core monkey patching enabled' do before { subject.configuration.disable_core_monkey_patch = false } - if Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('5.2.0') - context 'using active_support < 5.2' do + if Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('4.1.0') + context 'using active_support < 4.1' do it 'changes implementation of ::BasicSocket#as_json temporarily' do original_implementation = BasicSocket. public_instance_method(:as_json). @@ -54,7 +54,7 @@ end end else - context 'using active_support >= 5.2' do + context 'using active_support >= 4.1' do context 'when called as transparent' do it 'executes provided block even when dependencies are unmet' do result = false @@ -72,8 +72,8 @@ end describe '#load!' do - if Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('5.2.0') - context 'using active_support < 5.2' do + if Gem::Version.new(ActiveSupport::VERSION::STRING) < Gem::Version.new('4.1.0') + context 'using active_support < 4.1' do context 'with core monkey patching enabled' do before { subject.configuration.disable_core_monkey_patch = false } @@ -103,7 +103,7 @@ end end else - context 'using active_support >= 5.2' do + context 'using active_support >= 4.1' do it_should_behave_like 'unloadable' end end diff --git a/spec/rollbar/plugins/rack_spec.rb b/spec/rollbar/plugins/rack_spec.rb index 0adfc2a2..61b96468 100644 --- a/spec/rollbar/plugins/rack_spec.rb +++ b/spec/rollbar/plugins/rack_spec.rb @@ -89,11 +89,11 @@ class RackMockError < RuntimeError; end end context 'with not array or hash POST parameters' do - let(:params) { 1000 } + let(:params) { 'test string' } it 'sends a body.multi key in params' do expect do - request.post('/will_crash', :input => params.to_json, 'CONTENT_TYPE' => 'application/json') + request.post('/will_crash', :input => params, 'CONTENT_TYPE' => 'application/json') end.to raise_error(exception) reported_body = Rollbar.last_report[:request][:body] diff --git a/spec/rollbar/util/hash_spec.rb b/spec/rollbar/util/hash_spec.rb index ee83f95e..1456b82e 100644 --- a/spec/rollbar/util/hash_spec.rb +++ b/spec/rollbar/util/hash_spec.rb @@ -19,4 +19,22 @@ expect(new_hash['bar']['foo']).to be_eql('bar') expect(new_hash['bar']['bar'][0]['foo']).to be_eql('bar') end + + it 'should replace circular references' do + a = { :foo => 'bar' } + b = { :a => a } + c = { :b => b } + a[:c] = c # Introduces a cycle + + array1 = %w[a b] + array2 = ['c', 'd', array1] + a[:array] = array1 + + array1 << array2 # Introduces a cycle + + new_hash = described_class.deep_stringify_keys(a) + + expect(new_hash['c']['b']['a'].include?('removed circular reference')).to be_truthy + expect(new_hash['array'][2][2].include?('removed circular reference')).to be_truthy + end end diff --git a/spec/rollbar_spec.rb b/spec/rollbar_spec.rb index 885fbe67..cca2abdc 100644 --- a/spec/rollbar_spec.rb +++ b/spec/rollbar_spec.rb @@ -1128,22 +1128,13 @@ def backtrace a = { :foo => 'bar' } b = { :a => a } c = { :b => b } - a[:c] = c + a[:c] = c # Introduces a cycle array1 = %w[a b] - _array2 = ['c', 'd', array1] + array2 = ['c', 'd', array1] a[:array] = array1 - # The line below will introduce a cycle in the array, which the rollbar code can handle. - # However, both OJ and ActiveSupport `as_json` crash on this when serializing the payload. - # We could do without OJ, but it's a moot point because of the ActiveSupport issue. - # The gemspec currently uses multi_json to give the user as much control as possible over - # choice of JSON serializer. We should continue to allow this flexibility unless it is - # determined that cycles of arrays directly referencing arrays (without other objects - # in between) must be supported. Then in that case, an appropriate serializer should - # be added to the gemspec. - # - # array1 << array2 + array1 << array2 # Introduces a cycle expect(logger_mock).to_not receive(:error).with( /\[Rollbar\] Reporting internal error encountered while sending data to Rollbar./