Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix "syntax error" with dynamic fields #2588

Closed
wants to merge 3 commits into from

2 participants

@mzsanford

While running under JRuby I noticed the following error while setting dynamic fields via []:

SyntaxError: (eval):1: syntax error, unexpected tMINUS

        def sci-fi=(value)
               ^
        org/jruby/RubyModule.java:2286:in `module_eval'
        /usr/local/lib/jruby/gems/gems/activesupport-3.2.8/lib/active_support/core_ext/kernel/singleton_class.rb:11:in `class_eval'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes.rb:214:in `define_dynamic_writer'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes.rb:228:in `method_missing'
        org/jruby/RubyBasicObject.java:1704:in `__send__'
        org/jruby/RubyKernel.java:2130:in `send'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:124:in `process_attribute'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:28:in `process_attributes'
        org/jruby/RubyHash.java:1211:in `each_pair'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:26:in `process_attributes'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:190:in `with_mass_assignment'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:187:in `with_mass_assignment'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes/processing.rb:22:in `process_attributes'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes.rb:158:in `assign_attributes'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/threaded/lifecycle.rb:26:in `_assigning'
        /usr/local/lib/jruby/gems/gems/mongoid-3.0.10/lib/mongoid/attributes.rb:157:in `assign_attributes'

The root cause seems to be the call to define_dynamic_writer without verifying that the method name is a valid Ruby identifier. This then calls class_eval with invalid syntax. I put a guard in process_attribute as well as the define_dynamic_* methods as a defense in depth against future errors. The new spec test was added and failing before this change.

Somewhat related, is there any worry about these class_eval calls causing bloat over time when using high numbers of dynamic attributes?

@durran
Owner

Thanks for the pull - I've rebased and merged in.

As for the note on the class_eval calls. Not worried too much as I'd only think it would become an issue if the documents being stored had no true structure at all and in the case of that I wouldn't necessarily think MongoDB would be a good fit. More of something Hadoop would be good at handling... But there's always a way around it which would be to use [] accessor methods which would never end up defining the methods themselves, ie model["some-attribute"].

@durran durran closed this
@mzsanford

My model has one sub-document that is a collection of terms and associated data. What I end up with is a very structured document with a dynamic-key'd Object attached to it. I've been using [] directly but stumbled across this error using object.attributes= to setup an object based on some new json data. I'll keep using [] and see if I can re-factor the #attributes= call. Thanks for the quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
7 lib/mongoid/attributes.rb
@@ -100,7 +100,8 @@ def respond_to?(name, include_private = false)
super || (
Mongoid.allow_dynamic_fields &&
attributes &&
- attributes.has_key?(name.to_s.reader)
+ attributes.has_key?(name.to_s.reader) &&
+ name.to_s.valid_method_name?
)
end
@@ -191,6 +192,8 @@ def write_attributes(attrs = nil, guard_protected_attributes = true)
#
# @since 3.0.0
def define_dynamic_reader(name)
+ return unless name.valid_method_name?
+
class_eval <<-READER
def #{name}
read_attribute(#{name.inspect})
@@ -209,6 +212,8 @@ def #{name}
#
# @since 3.0.0
def define_dynamic_writer(name)
+ return unless name.valid_method_name?
+
class_eval <<-WRITER
def #{name}=(value)
write_attribute(#{name.inspect}, value)
View
5 lib/mongoid/attributes/processing.rb
@@ -118,12 +118,13 @@ def pending_nested
#
# @since 2.0.0.rc.7
def process_attribute(name, value)
- responds = respond_to?("#{name}=")
+ writer_method = "#{name}="
+ responds = respond_to?(writer_method)
if Mongoid.allow_dynamic_fields && !responds
write_attribute(name, value)
else
raise Errors::UnknownAttribute.new(self.class, name) unless responds
- send("#{name}=", value)
+ send(writer_method, value)
end
end
View
12 lib/mongoid/extensions/string.rb
@@ -120,6 +120,18 @@ def writer?
include?("=")
end
+ # Is this string a valid_method_name?
+ #
+ # @example Is the string a valid Ruby idenfier for use as a method name
+ # "model=".valid_method_name?
+ #
+ # @return [ true, false ] If the string contains a valid Ruby identifier.
+ #
+ # @since 3.0.15
+ def valid_method_name?
+ /[@$"]/ !~ to_sym.inspect
+ end
+
# Is the object not to be converted to bson on criteria creation?
#
# @example Is the object unconvertable?
View
26 spec/mongoid/attributes/processing_spec.rb
@@ -115,6 +115,32 @@
it "updates the 1-n child" do
contractor.name.should eq("Jim")
end
+
+ context "with a key that is not a valid ruby identifier" do
+ let(:dynamic_updates) do
+ {
+ building_address: { :"postal-code" => "V8N 1A1" },
+ contractors: [{ :"license number" => "12345" }]
+ }
+ end
+
+ before do
+ building.process_attributes(dynamic_updates, :default, false)
+ end
+
+ it "updates the 1-1 child" do
+ building_address['postal-code'].should eq("V8N 1A1")
+ end
+
+ it "updates the 1-n child" do
+ contractor['license number'].should eq("12345")
+ end
+
+ it "correctly handles a second call to #process_attributes" do
+ building.process_attributes(dynamic_updates, :default, false)
+ end
+ end
+
end
end
end
Something went wrong with that request. Please try again.