From d82fe267270dac4ffd4b3d6a6e31312582287a8d Mon Sep 17 00:00:00 2001 From: Peter Suschlik Date: Sun, 15 Dec 2013 21:21:17 +0100 Subject: [PATCH 1/8] Fix YAML deserialization of anonymous structs `DelayedJob` makes freedom patches of `Psych`'s `resolve_class` method. It misses to guard this method from empty class names. `Psych` does it: https://github.com/tenderlove/psych/blob/2c644e18/lib/psych/class_loader.rb#L25 While deserializing an anonymous struct the class name is an empty string (`""`). Constantizing an empty string resolves to `Object`. This leads to this error: #> After this commit deserializing anonymous structs works again. --- lib/delayed/psych_ext.rb | 1 + spec/yaml_ext_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 6e168d791..f471091d5 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -136,6 +136,7 @@ def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable CyclomaticCom alias_method_chain :visit_Psych_Nodes_Mapping, :class def resolve_class_with_constantize(klass_name) + return nil if !klass_name || klass_name.empty? klass_name.constantize rescue resolve_class_without_constantize(klass_name) diff --git a/spec/yaml_ext_spec.rb b/spec/yaml_ext_spec.rb index d2ff5a800..73a6dc369 100644 --- a/spec/yaml_ext_spec.rb +++ b/spec/yaml_ext_spec.rb @@ -22,6 +22,15 @@ end.not_to raise_error end + it 'autoloads the class of an anonymous struct' do + expect do + yaml = "--- !ruby/struct\nn: 1\n" + object = YAML.load(yaml) + expect(object).to be_kind_of(Struct) + expect(object.n).to eq(1) + end.not_to raise_error + end + it 'autoloads the class for the instance' do expect do yaml = "--- !ruby/object:Autoloaded::InstanceClazz {}\n" From da0aa6ca47d97c184a5c02391f1277f7ef265427 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Fri, 20 Sep 2013 15:49:07 -0300 Subject: [PATCH 2/8] Add custom Psych Visitor's behavior only for Delayed, not glabally --- lib/delayed/backend/base.rb | 2 +- lib/delayed/psych_ext.rb | 20 ++++++++++++++++++-- spec/yaml_ext_spec.rb | 12 ++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/delayed/backend/base.rb b/lib/delayed/backend/base.rb index 3723570c0..b01589764 100644 --- a/lib/delayed/backend/base.rb +++ b/lib/delayed/backend/base.rb @@ -83,7 +83,7 @@ def payload_object # When the method is there, we need to load our YAML like this... @payload_object ||= YAML.load(handler, :safe => false) else - @payload_object ||= YAML.load(handler) + @payload_object ||= YAML.load(handler, nil, Delayed::PsychExt::ToRuby) end rescue TypeError, LoadError, NameError, ArgumentError => e raise DeserializationError, "Job failed to load: #{e.message}. Handler: #{handler.inspect}" diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 04b84aa14..23a2af48f 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,8 +31,24 @@ def encode_with(coder) end module Psych - module Visitors - class ToRuby + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml, filename) + result ? result.to_ruby(visitor) : result + end + + module Nodes + class Node + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.new.accept(self) + end + end + end +end + +module Delayed + module PsychExt + class ToRuby < Psych::Visitors::ToRuby def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag] diff --git a/spec/yaml_ext_spec.rb b/spec/yaml_ext_spec.rb index d2ff5a800..ece82c4e5 100644 --- a/spec/yaml_ext_spec.rb +++ b/spec/yaml_ext_spec.rb @@ -4,32 +4,36 @@ it 'autoloads classes' do expect do yaml = "--- !ruby/class Autoloaded::Clazz\n" - expect(YAML.load(yaml)).to eq(Autoloaded::Clazz) + expect(load_with_delayed_visitor(yaml)).to eq(Autoloaded::Clazz) end.not_to raise_error end it 'autoloads the class of a struct' do expect do yaml = "--- !ruby/class Autoloaded::Struct\n" - expect(YAML.load(yaml)).to eq(Autoloaded::Struct) + expect(load_with_delayed_visitor(yaml)).to eq(Autoloaded::Struct) end.not_to raise_error end it 'autoloads the class for the instance of a struct' do expect do yaml = '--- !ruby/struct:Autoloaded::InstanceStruct {}' - expect(YAML.load(yaml).class).to eq(Autoloaded::InstanceStruct) + expect(load_with_delayed_visitor(yaml).class).to eq(Autoloaded::InstanceStruct) end.not_to raise_error end it 'autoloads the class for the instance' do expect do yaml = "--- !ruby/object:Autoloaded::InstanceClazz {}\n" - expect(YAML.load(yaml).class).to eq(Autoloaded::InstanceClazz) + expect(load_with_delayed_visitor(yaml).class).to eq(Autoloaded::InstanceClazz) end.not_to raise_error end it 'does not throw an uninitialized constant Syck::Syck when using YAML.load with poorly formed yaml' do expect { YAML.load(YAML.dump('foo: *bar')) }.not_to raise_error end + + def load_with_delayed_visitor(yaml) + YAML.load(yaml, nil, Delayed::PsychExt::ToRuby) + end end From 78dac017dd62d9fccef6733a5f26446cc1e30593 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Tue, 24 Sep 2013 16:48:53 -0300 Subject: [PATCH 3/8] Fix support for ruby 1.9.2 (Psych < 1.3.0) --- lib/delayed/psych_ext.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 23a2af48f..9d62a2c2e 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,9 +31,16 @@ def encode_with(coder) end module Psych - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml, filename) - result ? result.to_ruby(visitor) : result + if VERSION.to_f < 1.3 + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml) + result ? result.to_ruby(visitor) : result + end + else + def self.load yaml, filename = nil, visitor = nil + result = parse(yaml, filename) + result ? result.to_ruby(visitor) : result + end end module Nodes From 9babdf35792a29f717f2f172ba6f6ced131aa559 Mon Sep 17 00:00:00 2001 From: Brenno Costa Date: Mon, 13 Jan 2014 12:01:25 -0500 Subject: [PATCH 4/8] Add support to Ruby 2.1 --- lib/delayed/psych_ext.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 9d62a2c2e..bde93a7a4 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -45,9 +45,16 @@ def self.load yaml, filename = nil, visitor = nil module Nodes class Node - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.new.accept(self) + if Gem::Version.new(VERSION) >= Gem::Version.new('2.0.2') + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.create.accept(self) + end + else + def to_ruby(visitor) + visitor ||= Visitors::ToRuby + visitor.new.accept(self) + end end end end From 055737b9de6c086009c37d95f018d67a63c4d342 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 15:59:10 -0400 Subject: [PATCH 5/8] Override even fewer YAML methods --- lib/delayed/backend/base.rb | 8 +----- lib/delayed/backend/shared_spec.rb | 2 +- lib/delayed/psych_ext.rb | 45 +++++++++--------------------- lib/delayed/syck_ext.rb | 8 ++++++ spec/yaml_ext_spec.rb | 2 +- 5 files changed, 24 insertions(+), 41 deletions(-) diff --git a/lib/delayed/backend/base.rb b/lib/delayed/backend/base.rb index b01589764..56fe47fa5 100644 --- a/lib/delayed/backend/base.rb +++ b/lib/delayed/backend/base.rb @@ -78,13 +78,7 @@ def payload_object=(object) end def payload_object - if YAML.respond_to?(:unsafe_load) - # See https://github.com/dtao/safe_yaml - # When the method is there, we need to load our YAML like this... - @payload_object ||= YAML.load(handler, :safe => false) - else - @payload_object ||= YAML.load(handler, nil, Delayed::PsychExt::ToRuby) - end + @payload_object ||= YAML.load_dj(handler) rescue TypeError, LoadError, NameError, ArgumentError => e raise DeserializationError, "Job failed to load: #{e.message}. Handler: #{handler.inspect}" end diff --git a/lib/delayed/backend/shared_spec.rb b/lib/delayed/backend/shared_spec.rb index 2a5ce4340..3fb2691bf 100644 --- a/lib/delayed/backend/shared_spec.rb +++ b/lib/delayed/backend/shared_spec.rb @@ -175,7 +175,7 @@ def create_job(opts = {}) it 'raises a DeserializationError when the YAML.load raises argument error' do job = described_class.new :handler => '--- !ruby/struct:GoingToRaiseArgError {}' - expect(YAML).to receive(:load).and_raise(ArgumentError) + expect(YAML).to receive(:load_dj).and_raise(ArgumentError) expect { job.payload_object }.to raise_error(Delayed::DeserializationError) end end diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index bde93a7a4..84e85ac63 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -31,39 +31,22 @@ def encode_with(coder) end module Psych - if VERSION.to_f < 1.3 - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml) - result ? result.to_ruby(visitor) : result - end - else - def self.load yaml, filename = nil, visitor = nil - result = parse(yaml, filename) - result ? result.to_ruby(visitor) : result - end - end - - module Nodes - class Node - if Gem::Version.new(VERSION) >= Gem::Version.new('2.0.2') - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.create.accept(self) - end - else - def to_ruby(visitor) - visitor ||= Visitors::ToRuby - visitor.new.accept(self) - end - end - end + def self.load_dj(yaml) + result = parse(yaml) + result ? Delayed::PsychExt::ToRuby.create.accept(result) : result end end module Delayed module PsychExt class ToRuby < Psych::Visitors::ToRuby - def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName + unless respond_to?(:create) + def self.create + new + end + end + + def visit_Psych_Nodes_Mapping(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag] case object.tag @@ -97,17 +80,15 @@ def visit_Psych_Nodes_Mapping_with_class(object) # rubocop:disable PerceivedComp raise Delayed::DeserializationError, "DataMapper::ObjectNotFoundError, class: #{klass} (#{error.message})" end else - visit_Psych_Nodes_Mapping_without_class(object) + super end end - alias_method_chain :visit_Psych_Nodes_Mapping, :class - def resolve_class_with_constantize(klass_name) + def resolve_class(klass_name) klass_name.constantize rescue - resolve_class_without_constantize(klass_name) + super end - alias_method_chain :resolve_class, :constantize end end end diff --git a/lib/delayed/syck_ext.rb b/lib/delayed/syck_ext.rb index 2a280fc77..fff03a15a 100644 --- a/lib/delayed/syck_ext.rb +++ b/lib/delayed/syck_ext.rb @@ -32,3 +32,11 @@ def self.yaml_tag_read_class(name) "Struct::#{ name }" end end + +module YAML + def load_dj(yaml) + # See https://github.com/dtao/safe_yaml + # When the method is there, we need to load our YAML like this... + respond_to?(:unsafe_load) ? load(yaml, :safe => false) : load(yaml) + end +end diff --git a/spec/yaml_ext_spec.rb b/spec/yaml_ext_spec.rb index ece82c4e5..b46892d18 100644 --- a/spec/yaml_ext_spec.rb +++ b/spec/yaml_ext_spec.rb @@ -34,6 +34,6 @@ end def load_with_delayed_visitor(yaml) - YAML.load(yaml, nil, Delayed::PsychExt::ToRuby) + YAML.load_dj(yaml) end end From a47e0d7b55a30fb5853f20b139caddd39535727f Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 16:55:26 -0400 Subject: [PATCH 6/8] Don't override AR yaml generation --- lib/delayed/psych_ext.rb | 32 ++++++++++++-------------------- spec/helper.rb | 3 +++ 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/lib/delayed/psych_ext.rb b/lib/delayed/psych_ext.rb index 84e85ac63..37d3622e8 100644 --- a/lib/delayed/psych_ext.rb +++ b/lib/delayed/psych_ext.rb @@ -1,22 +1,3 @@ -if defined?(ActiveRecord) - ActiveRecord::Base.class_eval do - # rubocop:disable BlockNesting - if instance_methods.include?(:encode_with) - def encode_with_override(coder) - encode_with_without_override(coder) - coder.tag = "!ruby/ActiveRecord:#{self.class.name}" if coder.respond_to?(:tag=) - end - alias_method :encode_with_without_override, :encode_with - alias_method :encode_with, :encode_with_override - else - def encode_with(coder) - coder['attributes'] = attributes - coder.tag = "!ruby/ActiveRecord:#{self.class.name}" if coder.respond_to?(:tag=) - end - end - end -end - module Delayed class PerformableMethod # serialize to YAML @@ -46,10 +27,21 @@ def self.create end end - def visit_Psych_Nodes_Mapping(object) # rubocop:disable PerceivedComplexity, CyclomaticComplexity, MethodName + def visit_Psych_Nodes_Mapping(object) # rubocop:disable CyclomaticComplexity, MethodName, PerceivedComplexity return revive(Psych.load_tags[object.tag], object) if Psych.load_tags[object.tag] case object.tag + when /^!ruby\/object/ + result = super + if defined?(ActiveRecord::Base) && result.is_a?(ActiveRecord::Base) + begin + result.class.find(result[result.class.primary_key]) + rescue ActiveRecord::RecordNotFound => error # rubocop:disable BlockNesting + raise Delayed::DeserializationError, "ActiveRecord::RecordNotFound, class: #{klass}, primary key: #{id} (#{error.message})" + end + else + result + end when /^!ruby\/ActiveRecord:(.+)$/ klass = resolve_class(Regexp.last_match[1]) payload = Hash[*object.children.map { |c| accept c }] diff --git a/spec/helper.rb b/spec/helper.rb index c040252df..3f8c1c4bc 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -23,6 +23,9 @@ Delayed::Worker.logger = Logger.new('/tmp/dj.log') ENV['RAILS_ENV'] = 'test' +# Trigger AR to initialize +ActiveRecord::Base # rubocop:disable Void + module Rails def self.root '.' From e530194fcc63329ed73127b8938eee342ba4ce85 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 17:08:07 -0400 Subject: [PATCH 7/8] Use a throw away log file --- spec/helper.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/helper.rb b/spec/helper.rb index 3f8c1c4bc..d33ea7f5d 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -20,7 +20,15 @@ require 'delayed_job' require 'delayed/backend/shared_spec' -Delayed::Worker.logger = Logger.new('/tmp/dj.log') +if ENV['DEBUG_LOGS'] + Delayed::Worker.logger = Logger.new(STDOUT) +else + require 'tempfile' + + tf = Tempfile.new('dj.log') + Delayed::Worker.logger = Logger.new(tf.path) + tf.unlink +end ENV['RAILS_ENV'] = 'test' # Trigger AR to initialize From 7a288ec6fd799a46be7994e379d29ef717267205 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Tue, 23 Sep 2014 18:15:22 -0400 Subject: [PATCH 8/8] Load psych in rubinius It doesn't do this on its own --- Gemfile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index c02e4edef..6ec9875c8 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,10 @@ platforms :jruby do gem 'activerecord-jdbcsqlite3-adapter' end +platforms :rbx do + gem 'psych' +end + group :test do if ENV['RAILS_VERSION'] == 'edge' gem 'activerecord', :github => 'rails/rails'