diff --git a/lib/oga.rb b/lib/oga.rb index 089a43a..d87cc87 100644 --- a/lib/oga.rb +++ b/lib/oga.rb @@ -55,6 +55,7 @@ require 'oga/ruby/generator' require 'oga/xpath/lexer' require 'oga/xpath/parser' +require 'oga/xpath/context' require 'oga/xpath/compiler' require 'oga/xpath/conversion' diff --git a/lib/oga/xpath/compiler.rb b/lib/oga/xpath/compiler.rb index a0a4eca..2fc22cf 100644 --- a/lib/oga/xpath/compiler.rb +++ b/lib/oga/xpath/compiler.rb @@ -12,6 +12,11 @@ module Oga # @return [Oga::LRU] CACHE = LRU.new + # Context for compiled Procs. As compiled Procs do not mutate the + # enclosing environment we can just re-use the same instance without + # synchronization. + CONTEXT = Context.new + # Wildcard for node names/namespace prefixes. STAR = '*' @@ -86,7 +91,7 @@ module Oga generator = Ruby::Generator.new source = generator.process(proc_ast) - eval(source) + CONTEXT.evaluate(source) ensure reset end diff --git a/lib/oga/xpath/context.rb b/lib/oga/xpath/context.rb new file mode 100644 index 0000000..59834c8 --- /dev/null +++ b/lib/oga/xpath/context.rb @@ -0,0 +1,17 @@ +module Oga + module XPath + # Class used as the context for compiled XPath Procs. + # + # The binding of this class is used for the binding of Procs compiled by + # {Compiler}. Not using a specific binding would result in the procs + # using the binding of {Compiler#compile}, which could lead to race + # conditions. + class Context + # @param [String] string + # @return [Proc] + def evaluate(string) + binding.eval(string) + end + end # Context + end # XPath +end # Oga diff --git a/spec/oga/xpath/compiler_spec.rb b/spec/oga/xpath/compiler_spec.rb index 0d690ce..d982672 100644 --- a/spec/oga/xpath/compiler_spec.rb +++ b/spec/oga/xpath/compiler_spec.rb @@ -49,5 +49,27 @@ describe Oga::XPath::Compiler do block.call(doc).should be_an_instance_of(Oga::XML::NodeSet) end end + + # This test relies on Binding#receiver (Binding#self on Rubinius), which + # sadly isn't available on all tested Ruby versions. + # + # Procs should have their own Binding as otherwise concurrent usage could + # lead to race conditions due to variable scopes and such being re-used. + [:receiver, :self].each do |binding_method| + if Binding.method_defined?(binding_method) + describe 'the returned Proc' do + it 'uses an isolated Binding' do + ast = parse_xpath('foo') + block = @compiler.compile(ast) + + binding_context = block.binding.send(binding_method) + + binding_context.should be_an_instance_of(Oga::XPath::Context) + end + end + + break + end + end end end diff --git a/spec/oga/xpath/context_spec.rb b/spec/oga/xpath/context_spec.rb new file mode 100644 index 0000000..6c9ca98 --- /dev/null +++ b/spec/oga/xpath/context_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Oga::XPath::Context do + describe '#evaluate' do + it 'returns the result of eval()' do + context = described_class.new + + context.evaluate('10').should == 10 + end + + describe 'assigning a variable in a lambda' do + it 'assigns the variable locally to the lambda' do + context = described_class.new + + block = context.evaluate('lambda { number = 10 }') + + block.call + + expect { context.evaluate('number') }.to raise_error(NameError) + end + end + end +end