(HI-328) Prevent interpolation recursion

This commit ensures that only one RecursiveGuard instance is
used in a lookup even if that lookup results in multiple
interpolations.

A backend implementation must add and pass on a fifth
'recurse_guard' argument but the implementation will accept
the old API with four arguments as well for backward
compatibility.

lib/hiera/backend.rb

@@ -8,6 +8,17 @@
 
 class Hiera
   module Backend
+    class Backend1xWrapper
+      def initialize(wrapped)
+        @wrapped = wrapped
+      end
+
+      def lookup(key, scope, order_override, resolution_type, recurse_guard)
+        Hiera.debug("Using Hiera 1.x backend API to access instance of class #{@wrapped.class.name}. Lookup recursion will not be detected")
+        @wrapped.lookup(key, scope, order_override, resolution_type)
+      end
+    end
+
     class << self
       # Data lives in /var/lib/hiera by default.  If a backend
       # supplies a datadir in the config it will be used and
@@ -121,31 +132,31 @@ def datasourcefiles(backend, scope, extension, override=nil, hierarchy=nil)
       # @return [String] A copy of the data with all instances of <code>%{...}</code> replaced.
       #
       # @api public
-      def parse_string(data, scope, extra_data={})
-        Hiera::Interpolate.interpolate(data, scope, extra_data)
+      def parse_string(data, scope, extra_data={}, recurse_guard = nil)
+        Hiera::Interpolate.interpolate(data, scope, extra_data, recurse_guard)
       end
 
       # Parses a answer received from data files
       #
       # Ultimately it just pass the data through parse_string but
       # it makes some effort to handle arrays of strings as well
-      def parse_answer(data, scope, extra_data={})
+      def parse_answer(data, scope, extra_data={}, recurse_guard = nil)
         if data.is_a?(Numeric) or data.is_a?(TrueClass) or data.is_a?(FalseClass)
           return data
         elsif data.is_a?(String)
-          return parse_string(data, scope, extra_data)
+          return parse_string(data, scope, extra_data, recurse_guard)
         elsif data.is_a?(Hash)
           answer = {}
           data.each_pair do |key, val|
-            interpolated_key = parse_string(key, scope, extra_data)
-            answer[interpolated_key] = parse_answer(val, scope, extra_data)
+            interpolated_key = parse_string(key, scope, extra_data, recurse_guard)
+            answer[interpolated_key] = parse_answer(val, scope, extra_data, recurse_guard)
           end
 
           return answer
         elsif data.is_a?(Array)
           answer = []
           data.each do |item|
-            answer << parse_answer(item, scope, extra_data)
+            answer << parse_answer(item, scope, extra_data, recurse_guard)
           end
 
           return answer
@@ -203,7 +214,7 @@ def merge_answer(left,right)
       # @param resolution_type [Symbol] One of :hash, :array, or :priority. Can be nil which is the same as :priority
       # @return [Object] The value that corresponds to the given key or nil if no such value cannot be found
       #
-      def lookup(key, default, scope, order_override, resolution_type)
+      def lookup(key, default, scope, order_override, resolution_type, recurse_guard = nil)
         @backends ||= {}
         answer = nil
 
@@ -215,9 +226,10 @@ def lookup(key, default, scope, order_override, resolution_type)
         end
 
         Config[:backends].each do |backend|
-          if constants.include?("#{backend.capitalize}_backend") || constants.include?("#{backend.capitalize}_backend".to_sym)
-            @backends[backend] ||= Backend.const_get("#{backend.capitalize}_backend").new
-            new_answer = @backends[backend].lookup(segments[0], scope, order_override, resolution_type)
+          backend_constant = "#{backend.capitalize}_backend"
+          if constants.include?(backend_constant) || constants.include?(backend_constant.to_sym)
+            backend = (@backends[backend] ||= find_backend(backend_constant))
+            new_answer = backend.lookup(segments[0], scope, order_override, resolution_type, recurse_guard)
             new_answer = qualified_lookup(subsegments, new_answer) unless subsegments.nil?
 
             next if new_answer.nil?
@@ -239,7 +251,7 @@ def lookup(key, default, scope, order_override, resolution_type)
         end
 
         answer = resolve_answer(answer, resolution_type) unless answer.nil?
-        answer = parse_string(default, scope) if answer.nil? and default.is_a?(String)
+        answer = parse_string(default, scope, recurse_guard) if answer.nil? and default.is_a?(String)
 
         return default if answer.nil?
         return answer
@@ -264,6 +276,12 @@ def qualified_lookup(segments, hash)
         end
         value
       end
+
+      def find_backend(backend_constant)
+        backend = Backend.const_get(backend_constant).new
+        return backend.method(:lookup).arity == 4 ? Backend1xWrapper.new(backend) : backend
+      end
+      private :find_backend
     end
   end
 end

lib/hiera/backend/json_backend.rb

@@ -9,7 +9,7 @@ def initialize(cache=nil)
         @cache = cache || Filecache.new
       end
 
-      def lookup(key, scope, order_override, resolution_type)
+      def lookup(key, scope, order_override, resolution_type, recurse_guard)
         answer = nil
 
         Hiera.debug("Looking up #{key} in JSON backend")
@@ -33,7 +33,7 @@ def lookup(key, scope, order_override, resolution_type)
           # the array
           #
           # for priority searches we break after the first found data item
-          new_answer = Backend.parse_answer(data[key], scope)
+          new_answer = Backend.parse_answer(data[key], scope, nil, recurse_guard)
           case resolution_type
           when :array
             raise Exception, "Hiera type mismatch for key '#{key}': expected Array and got #{new_answer.class}" unless new_answer.kind_of? Array or new_answer.kind_of? String

lib/hiera/backend/yaml_backend.rb

@@ -8,7 +8,7 @@ def initialize(cache=nil)
         @cache = cache || Filecache.new
       end
 
-      def lookup(key, scope, order_override, resolution_type)
+      def lookup(key, scope, order_override, resolution_type, recurse_guard)
         answer = nil
 
         Hiera.debug("Looking up #{key} in YAML backend")
@@ -32,7 +32,7 @@ def lookup(key, scope, order_override, resolution_type)
           # the array
           #
           # for priority searches we break after the first found data item
-          new_answer = Backend.parse_answer(data[key], scope)
+          new_answer = Backend.parse_answer(data[key], scope, nil, recurse_guard)
           case resolution_type
           when :array
             raise Exception, "Hiera type mismatch for key '#{key}': expected Array and got #{new_answer.class}" unless new_answer.kind_of? Array or new_answer.kind_of? String

lib/hiera/interpolate.rb

@@ -9,12 +9,13 @@ class << self
     INTERPOLATION = /%\{([^\}]*)\}/
     METHOD_INTERPOLATION = /%\{(scope|hiera|literal|alias)\(['"]([^"']*)["']\)\}/
 
-    def interpolate(data, scope, extra_data)
+    def interpolate(data, scope, extra_data, recurse_guard)
       if data.is_a?(String)
         # Wrapping do_interpolation in a gsub block ensures we process
         # each interpolation site in isolation using separate recursion guards.
+        recurse_guard ||= Hiera::RecursiveGuard.new
         data.gsub(INTERPOLATION) do |match|
-          interp_val = do_interpolation(match, Hiera::RecursiveGuard.new, scope, extra_data)
+          interp_val = do_interpolation(match, recurse_guard, scope, extra_data)
 
           # Get interp method in case we are aliasing
           if data.is_a?(String) && (match = data.match(INTERPOLATION))
@@ -43,7 +44,7 @@ def do_interpolation(data, recurse_guard, scope, extra_data)
         interpolation_variable = match[1]
         recurse_guard.check(interpolation_variable) do
           interpolate_method, key = get_interpolation_method_and_key(data)
-          interpolated_data = send(interpolate_method, data, key, scope, extra_data)
+          interpolated_data = send(interpolate_method, data, key, scope, extra_data, recurse_guard)
 
           # Halt recursion if we encounter a literal.
           return interpolated_data if interpolate_method == :literal_interpolate
@@ -70,25 +71,25 @@ def get_interpolation_method_and_key(data)
     end
     private :get_interpolation_method_and_key
 
-    def scope_interpolate(data, key, scope, extra_data)
+    def scope_interpolate(data, key, scope, extra_data, recurse_guard)
       segments = key.split('.')
       value = Hiera::Backend.qualified_lookup(segments, scope)
       value.nil? ? Hiera::Backend.qualified_lookup(segments, extra_data) : value
     end
     private :scope_interpolate
 
-    def hiera_interpolate(data, key, scope, extra_data)
-      Hiera::Backend.lookup(key, nil, scope, nil, :priority)
+    def hiera_interpolate(data, key, scope, extra_data, recurse_guard)
+      Hiera::Backend.lookup(key, nil, scope, nil, :priority, recurse_guard)
     end
     private :hiera_interpolate
 
-    def literal_interpolate(data, key, scope, extra_data)
+    def literal_interpolate(data, key, scope, extra_data, recurse_guard)
       key
     end
     private :literal_interpolate
 
-    def alias_interpolate(data, key, scope, extra_data)
-      Hiera::Backend.lookup(key, nil, scope, nil, :priority)
+    def alias_interpolate(data, key, scope, extra_data, recurse_guard)
+      Hiera::Backend.lookup(key, nil, scope, nil, :priority, recurse_guard)
     end
     private :alias_interpolate
   end

spec/spec_helper.rb

@@ -28,6 +28,11 @@
   end
 end
 
+# So everyone else doesn't have to include this base constant.
+module HieraSpec
+  FIXTURE_DIR = File.join(dir = File.expand_path(File.dirname(__FILE__)), 'unit', 'fixtures') unless defined?(FIXTURE_DIR)
+end
+
 # In ruby 1.8.5 Dir does not have mktmpdir defined, so this monkey patches
 # Dir to include the 1.8.7 definition of that method if it isn't already defined.
 # Method definition borrowed from ruby-1.8.7-p357/lib/ruby/1.8/tmpdir.rb

spec/unit/backend/json_backend_spec.rb

@@ -25,7 +25,7 @@ module Backend
           Backend.expects(:datafile).with(:json, {}, "one", "json").returns(nil)
           Backend.expects(:datafile).with(:json, {}, "two", "json").returns(nil)
 
-          @backend.lookup("key", {}, nil, :priority)
+          @backend.lookup("key", {}, nil, :priority, nil)
         end
 
         it "should retain the data types found in data files" do
@@ -35,9 +35,9 @@ module Backend
 
           @cache.expects(:read_file).with("/nonexisting/one.json", Hash).returns({"stringval" => "string", "boolval" => true, "numericval" => 1}).times(3)
 
-          @backend.lookup("stringval", {}, nil, :priority).should == "string"
-          @backend.lookup("boolval", {}, nil, :priority).should == true
-          @backend.lookup("numericval", {}, nil, :priority).should == 1
+          @backend.lookup("stringval", {}, nil, :priority, nil).should == "string"
+          @backend.lookup("boolval", {}, nil, :priority, nil).should == true
+          @backend.lookup("numericval", {}, nil, :priority, nil).should == 1
         end
 
         it "should pick data earliest source that has it for priority searches" do
@@ -49,12 +49,12 @@ module Backend
           File.stubs(:exist?).with("/nonexisting/one.json").returns(true)
           @cache.expects(:read_file).with("/nonexisting/one.json", Hash).returns({"key" => "test_%{rspec}"})
 
-          @backend.lookup("key", scope, nil, :priority).should == "test_test"
+          @backend.lookup("key", scope, nil, :priority, nil).should == "test_test"
         end
 
         it "should build an array of all data sources for array searches" do
           Hiera::Backend.stubs(:empty_answer).returns([])
-          Backend.stubs(:parse_answer).with('answer', {}).returns("answer")
+          Backend.stubs(:parse_answer).with('answer', {}, nil, nil).returns("answer")
           Backend.expects(:datafile).with(:json, {}, "one", "json").returns("/nonexisting/one.json")
           Backend.expects(:datafile).with(:json, {}, "two", "json").returns("/nonexisting/two.json")
 
@@ -66,18 +66,18 @@ module Backend
           @cache.expects(:read_file).with("/nonexisting/one.json", Hash).returns({"key" => "answer"})
           @cache.expects(:read_file).with("/nonexisting/two.json", Hash).returns({"key" => "answer"})
 
-          @backend.lookup("key", {}, nil, :array).should == ["answer", "answer"]
+          @backend.lookup("key", {}, nil, :array, nil).should == ["answer", "answer"]
         end
 
         it "should parse the answer for scope variables" do
-          Backend.stubs(:parse_answer).with('test_%{rspec}', {'rspec' => 'test'}).returns("test_test")
+          Backend.stubs(:parse_answer).with('test_%{rspec}', {'rspec' => 'test'}, nil, nil).returns("test_test")
           Backend.expects(:datasources).yields("one")
           Backend.expects(:datafile).with(:json, {"rspec" => "test"}, "one", "json").returns("/nonexisting/one.json")
 
           File.expects(:exist?).with("/nonexisting/one.json").returns(true)
           @cache.expects(:read_file).with("/nonexisting/one.json", Hash).returns({"key" => "test_%{rspec}"})
 
-          @backend.lookup("key", {"rspec" => "test"}, nil, :priority).should == "test_test"
+          @backend.lookup("key", {"rspec" => "test"}, nil, :priority, nil).should == "test_test"
         end
       end
     end

spec/unit/backend/yaml_backend_spec.rb

@@ -41,7 +41,7 @@ def read_file(path, expected_type, &block)
           Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).yields(["one", "/nonexisting/one.yaml"])
           @cache.value = "---\nkey: answer"
 
-          @backend.lookup("key", {}, nil, :priority).should == "answer"
+          @backend.lookup("key", {}, nil, :priority, nil).should == "answer"
         end
 
         describe "handling unexpected YAML values" do
@@ -51,17 +51,17 @@ def read_file(path, expected_type, &block)
 
           it "returns nil when the YAML value is nil" do
             @cache.value = "---\n"
-            @backend.lookup("key", {}, nil, :priority).should be_nil
+            @backend.lookup("key", {}, nil, :priority, nil).should be_nil
           end
 
           it "returns nil when the YAML file is false" do
             @cache.value = ""
-            @backend.lookup("key", {}, nil, :priority).should be_nil
+            @backend.lookup("key", {}, nil, :priority, nil).should be_nil
           end
 
           it "raises a TypeError when the YAML value is not a hash" do
             @cache.value = "---\n[one, two, three]"
-            expect { @backend.lookup("key", {}, nil, :priority) }.to raise_error(TypeError)
+            expect { @backend.lookup("key", {}, nil, :priority, nil) }.to raise_error(TypeError)
           end
         end
 
@@ -70,7 +70,7 @@ def read_file(path, expected_type, &block)
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>"answer"})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>"answer"})
 
-          @backend.lookup("key", {}, nil, :array).should == ["answer", "answer"]
+          @backend.lookup("key", {}, nil, :array, nil).should == ["answer", "answer"]
         end
 
         it "should ignore empty hash of data sources for hash searches" do
@@ -79,7 +79,7 @@ def read_file(path, expected_type, &block)
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
 
-          @backend.lookup("key", {}, nil, :hash).should == {"a" => "answer"}
+          @backend.lookup("key", {}, nil, :hash, nil).should == {"a" => "answer"}
         end
 
         it "should build a merged hash of data sources for hash searches" do
@@ -88,7 +88,7 @@ def read_file(path, expected_type, &block)
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"b"=>"answer", "a"=>"wrong"}})
 
-          @backend.lookup("key", {}, nil, :hash).should == {"a" => "answer", "b" => "answer"}
+          @backend.lookup("key", {}, nil, :hash, nil).should == {"a" => "answer", "b" => "answer"}
         end
 
         it "should fail when trying to << a Hash" do
@@ -97,7 +97,7 @@ def read_file(path, expected_type, &block)
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>["a", "answer"]})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
 
-          expect {@backend.lookup("key", {}, nil, :array)}.to raise_error(Exception, "Hiera type mismatch for key 'key': expected Array and got Hash")
+          expect {@backend.lookup("key", {}, nil, :array, nil)}.to raise_error(Exception, "Hiera type mismatch for key 'key': expected Array and got Hash")
         end
 
         it "should fail when trying to merge an Array" do
@@ -106,15 +106,15 @@ def read_file(path, expected_type, &block)
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>["a", "wrong"]})
 
-          expect { @backend.lookup("key", {}, nil, :hash) }.to raise_error(Exception, "Hiera type mismatch for key 'key': expected Hash and got Array")
+          expect { @backend.lookup("key", {}, nil, :hash, nil) }.to raise_error(Exception, "Hiera type mismatch for key 'key': expected Hash and got Array")
         end
 
         it "should parse the answer for scope variables" do
           Backend.expects(:datasourcefiles).with(:yaml, {"rspec" => "test"}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>"test_%{rspec}"})
 
-          @backend.lookup("key", {"rspec" => "test"}, nil, :priority).should == "test_test"
+          @backend.lookup("key", {"rspec" => "test"}, nil, :priority, nil).should == "test_test"
         end
 
         it "should retain datatypes found in yaml files" do
@@ -123,9 +123,9 @@ def read_file(path, expected_type, &block)
 
           @cache.value = "---\nstringval: 'string'\nboolval: true\nnumericval: 1"
 
-          @backend.lookup("stringval", {}, nil, :priority).should == "string"
-          @backend.lookup("boolval", {}, nil, :priority).should == true
-          @backend.lookup("numericval", {}, nil, :priority).should == 1
+          @backend.lookup("stringval", {}, nil, :priority, nil).should == "string"
+          @backend.lookup("boolval", {}, nil, :priority, nil).should == true
+          @backend.lookup("numericval", {}, nil, :priority, nil).should == 1
         end
       end
     end