From e93eab81a58282db5306de6fec42703795c85523 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Mon, 8 Feb 2010 20:17:38 +0100 Subject: Fix #3155 - prevent error when using two matching regex in cascade The following manifest: case $var { /match/: { if $var =~ /matchagain/ { } } } is failing because the "=~" operators when matching sets an ephemeral variable in the scope. But the case regex also did it, and since they both belong to the same scope, and Puppet variables are immutables, the scope raises an error. This patch fixes this issue by adding to the current scope a stack of ephemeral symbol tables. Each new match operator or case/selector with regex adds a new scope. When we get out of the case/if/selector structure the scope is reset to the ephemeral level we were when entering it. This way the following manifest produces the correct output: case $var { /match(rematch)/: { notice("1. \$0 = $0, \$1 = $1") if $var =~ /matchagain/ { notice("2. \$0 = $0, \$1 = $1") } notice("3. \$0 = $0, \$1 = $1") } } notice("4. \$0 = $0") And the output is: 1. $0 = match, $1 = rematch 2. $0 = matchagain, $1 = rematch 3. $0 = match, $1 = rematch 4. $0 = Signed-off-by: Brice Figureau --- spec/unit/parser/ast/casestatement.rb | 6 ++- spec/unit/parser/ast/ifstatement.rb | 3 +- spec/unit/parser/ast/selector.rb | 6 ++- spec/unit/parser/scope.rb | 97 +++++++++++++++++++++++++++++++++-- 4 files changed, 104 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/spec/unit/parser/ast/casestatement.rb b/spec/unit/parser/ast/casestatement.rb index c2e9a6929..240d3bfea 100755 --- a/spec/unit/parser/ast/casestatement.rb +++ b/spec/unit/parser/ast/casestatement.rb @@ -104,19 +104,21 @@ describe Puppet::Parser::AST::CaseStatement do end it "should unset scope ephemeral variables after option evaluation" do + @scope.stubs(:ephemeral_level).returns(:level) @opval1.stubs(:evaluate_match).with { |*arg| arg[0] == "value" and arg[1] == @scope }.returns(true) @option1.stubs(:safeevaluate).with(@scope).returns(:result) - @scope.expects(:unset_ephemeral_var) + @scope.expects(:unset_ephemeral_var).with(:level) @casestmt.evaluate(@scope) end it "should not leak ephemeral variables even if evaluation fails" do + @scope.stubs(:ephemeral_level).returns(:level) @opval1.stubs(:evaluate_match).with { |*arg| arg[0] == "value" and arg[1] == @scope }.returns(true) @option1.stubs(:safeevaluate).with(@scope).raises - @scope.expects(:unset_ephemeral_var) + @scope.expects(:unset_ephemeral_var).with(:level) lambda { @casestmt.evaluate(@scope) }.should raise_error end diff --git a/spec/unit/parser/ast/ifstatement.rb b/spec/unit/parser/ast/ifstatement.rb index 10d877a4b..83121cddd 100755 --- a/spec/unit/parser/ast/ifstatement.rb +++ b/spec/unit/parser/ast/ifstatement.rb @@ -64,10 +64,11 @@ describe Puppet::Parser::AST::IfStatement do end it "should reset ephemeral statements after evaluation" do + @scope.expects(:ephemeral_level).returns(:level) Puppet::Parser::Scope.stubs(:true?).returns(true) @stmt.expects(:safeevaluate).with(@scope) - @scope.expects(:unset_ephemeral_var) + @scope.expects(:unset_ephemeral_var).with(:level) @ifstmt.evaluate(@scope) end diff --git a/spec/unit/parser/ast/selector.rb b/spec/unit/parser/ast/selector.rb index 23989b902..f32310b92 100755 --- a/spec/unit/parser/ast/selector.rb +++ b/spec/unit/parser/ast/selector.rb @@ -102,19 +102,21 @@ describe Puppet::Parser::AST::Selector do end it "should unset scope ephemeral variables after option evaluation" do + @scope.stubs(:ephemeral_level).returns(:level) @param1.stubs(:evaluate_match).with { |*arg| arg[0] == "value" and arg[1] == @scope }.returns(true) @value1.stubs(:safeevaluate).with(@scope).returns(:result) - @scope.expects(:unset_ephemeral_var) + @scope.expects(:unset_ephemeral_var).with(:level) @selector.evaluate(@scope) end it "should not leak ephemeral variables even if evaluation fails" do + @scope.stubs(:ephemeral_level).returns(:level) @param1.stubs(:evaluate_match).with { |*arg| arg[0] == "value" and arg[1] == @scope }.returns(true) @value1.stubs(:safeevaluate).with(@scope).raises - @scope.expects(:unset_ephemeral_var) + @scope.expects(:unset_ephemeral_var).with(:level) lambda { @selector.evaluate(@scope) }.should raise_error end diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb index 56bcd18fb..c030f2552 100755 --- a/spec/unit/parser/scope.rb +++ b/spec/unit/parser/scope.rb @@ -286,6 +286,85 @@ describe Puppet::Parser::Scope do @scope.lookupvar("myvar", false).should == :value1 end + + it "should raise an error when setting it again" do + @scope.setvar("1", :value2, :ephemeral => true) + lambda { @scope.setvar("1", :value3, :ephemeral => true) }.should raise_error + end + + it "should declare ephemeral number only variable names" do + @scope.ephemeral?("0").should be_true + end + + it "should not declare ephemeral other variable names" do + @scope.ephemeral?("abc0").should be_nil + end + + describe "with more than one level" do + it "should prefer latest ephemeral scopes" do + @scope.setvar("0", :earliest, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("0", :latest, :ephemeral => true) + @scope.lookupvar("0", false).should == :latest + end + + it "should be able to report the current level" do + @scope.ephemeral_level.should == 1 + @scope.new_ephemeral + @scope.ephemeral_level.should == 2 + end + + it "should check presence of an ephemeral variable accross multiple levels" do + @scope.new_ephemeral + @scope.setvar("1", :value1, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("0", :value2, :ephemeral => true) + @scope.new_ephemeral + @scope.ephemeral_include?("1").should be_true + end + + it "should return false when an ephemeral variable doesn't exist in any ephemeral scope" do + @scope.new_ephemeral + @scope.setvar("1", :value1, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("0", :value2, :ephemeral => true) + @scope.new_ephemeral + @scope.ephemeral_include?("2").should be_false + end + + it "should get ephemeral values from earlier scope when not in later" do + @scope.setvar("1", :value1, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("0", :value2, :ephemeral => true) + @scope.lookupvar("1", false).should == :value1 + end + + describe "when calling unset_ephemeral_var without a level" do + it "should remove all the variables values" do + @scope.setvar("1", :value1, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("1", :value2, :ephemeral => true) + + @scope.unset_ephemeral_var + + @scope.lookupvar("1", false).should == :undefined + end + end + + describe "when calling unset_ephemeral_var with a level" do + it "should remove ephemeral scopes up to this level" do + @scope.setvar("1", :value1, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("1", :value2, :ephemeral => true) + @scope.new_ephemeral + @scope.setvar("1", :value3, :ephemeral => true) + + @scope.unset_ephemeral_var(2) + + @scope.lookupvar("1", false).should == :value2 + end + end + end end describe "when interpolating string" do @@ -451,6 +530,11 @@ describe Puppet::Parser::Scope do @scope.ephemeral_from(@match) end + + it "should create a new ephemeral level" do + @scope.expects(:new_ephemeral) + @scope.ephemeral_from(@match) + end end describe "when unsetting variables" do @@ -461,9 +545,16 @@ describe Puppet::Parser::Scope do end it "should be able to unset ephemeral variables" do - @scope.setvar("foo", "bar", :ephemeral => true) - @scope.unsetvar("foo") - @scope.lookupvar("foo").should == "" + @scope.setvar("0", "bar", :ephemeral => true) + @scope.unsetvar("0") + @scope.lookupvar("0").should == "" + end + + it "should not unset ephemeral variables in previous ephemeral scope" do + @scope.setvar("0", "bar", :ephemeral => true) + @scope.new_ephemeral + @scope.unsetvar("0") + @scope.lookupvar("0").should == "bar" end end -- cgit