Fix #5127 - error when accessing array elements

Accesing an array with an integer index (ie $array[1]) is producing
a ruby error: can't convert String into Integer

This is because the array index is not properly converted to an number
before the array element lookup is performed.


Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

2 files changed, 52 insertions, 2 deletions

diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 090d75c4e..fcdd219d7 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -148,12 +148,20 @@ class Puppet::Parser::AST
       key.respond_to?(:evaluate) ? key.safeevaluate(scope) : key
     end
 
+    def array_index_or_key(object, key)
+      if object.is_a?(Array)
+        raise Puppet::ParserError, "#{key} is not an integer, but is used as an index of an array" unless key = Puppet::Parser::Scope.number?(key)
+      end
+      key
+    end
+
     def evaluate(scope)
       object = evaluate_container(scope)
+      accesskey = evaluate_key(scope)
 
       raise Puppet::ParseError, "#{variable} is not an hash or array when accessing it with #{accesskey}" unless object.is_a?(Hash) or object.is_a?(Array)
 
-      object[evaluate_key(scope)]
+      object[array_index_or_key(object, accesskey)]
     end
 
     # Assign value to this hashkey or array index
@@ -166,7 +174,7 @@ class Puppet::Parser::AST
       end
 
       # assign to hash or array
-      object[accesskey] = value
+      object[array_index_or_key(object, accesskey)] = value
     end
 
     def to_s
diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb
index eb71f0d37..384ee08eb 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -136,6 +136,22 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
       access.evaluate(@scope).should == "val2"
     end
 
+    it "should be able to return an array member when index is a stringified number" do
+      @scope.stubs(:lookupvar).with("a").returns(["val1", "val2", "val3"])
+
+      access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "1" )
+
+      access.evaluate(@scope).should == "val2"
+    end
+
+    it "should raise an error when accessing an array with a key" do
+      @scope.stubs(:lookupvar).with("a").returns(["val1", "val2", "val3"])
+
+      access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "get_me_the_second_element_please" )
+
+      lambda { access.evaluate(@scope) }.should raise_error
+    end
+
     it "should be able to return an hash value" do
       @scope.stubs(:lookupvar).with("a").returns({ "key1" => "val1", "key2" => "val2", "key3" => "val3" })
 
@@ -144,6 +160,14 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
       access.evaluate(@scope).should == "val2"
     end
 
+    it "should be able to return an hash value with a numerical key" do
+      @scope.stubs(:lookupvar).with("a").returns({ "key1" => "val1", "key2" => "val2", "45" => "45", "key3" => "val3" })
+
+      access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "45" )
+
+      access.evaluate(@scope).should == "45"
+    end
+
     it "should raise an error if the variable lookup didn't return an hash or an array" do
       @scope.stubs(:lookupvar).with("a").returns("I'm a string")
 
@@ -195,6 +219,24 @@ describe Puppet::Parser::AST::HashOrArrayAccess do
       scope.lookupvar("a").should be_include("b")
     end
 
+    it "should raise an error when assigning an array element with a key" do
+      @scope.stubs(:lookupvar).with("a").returns([])
+
+      access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "get_me_the_second_element_please" )
+
+      lambda { access.assign(@scope, "test") }.should raise_error
+    end
+
+    it "should be able to return an array member when index is a stringified number" do
+      scope = Puppet::Parser::Scope.new
+      scope.setvar("a", [])
+
+      access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "0" )
+
+      access.assign(scope, "val2")
+      scope.lookupvar("a").should == ["val2"]
+    end
+
     it "should raise an error when trying to overwrite an hash value" do
       @scope.stubs(:lookupvar).with("a").returns({ "key" => [ "a" , "b" ]})
       access = Puppet::Parser::AST::HashOrArrayAccess.new(:variable => "a", :key => "key")