From b4b9939b7d05ded5189ec20564585f3b13a037ff Mon Sep 17 00:00:00 2001
From: Dominik Sander <git@dsander.de>
Date: Wed, 30 Apr 2014 23:47:43 +0200
Subject: [PATCH] LiquidMigrator is checking for complex JSONPaths

If a complex JSONPath like '$.text[*]' is found the migration will
fail with an exception and keep the database intact
---
 app/concerns/liquid_interpolatable.rb |  4 ++--
 lib/liquid_migrator.rb                | 13 ++++++++++---
 spec/lib/liquid_migrator_spec.rb      | 22 +++++++++++++++++++---
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/app/concerns/liquid_interpolatable.rb b/app/concerns/liquid_interpolatable.rb
index 45039b75..6b79c5f4 100644
--- a/app/concerns/liquid_interpolatable.rb
+++ b/app/concerns/liquid_interpolatable.rb
@@ -1,7 +1,7 @@
 module LiquidInterpolatable
   extend ActiveSupport::Concern
 
-  def interpolate_options options, payload
+  def interpolate_options(options, payload)
     duped_options = options.dup.tap do |duped_options|
       duped_options.each_pair do |key, value|
         if value.class == String
@@ -14,7 +14,7 @@ module LiquidInterpolatable
     duped_options
   end
 
-  def interpolate_string string, payload
+  def interpolate_string(string, payload)
     Liquid::Template.parse(string).render(payload)
   end
 
diff --git a/lib/liquid_migrator.rb b/lib/liquid_migrator.rb
index b6675752..9d52c401 100644
--- a/lib/liquid_migrator.rb
+++ b/lib/liquid_migrator.rb
@@ -20,9 +20,9 @@ module LiquidMigrator
           end
           hash[key] = LiquidMigrator.convert_string value, options[:leading_dollarsign_is_jsonpath]
         when 'Hash'
-          # might want to make it recursive?
+          raise "nested Hashes are not supported at the moment"
         when 'Array'
-          # do we need it?
+          raise "nested Arrays are not supported at the moment"
         end
       end
         # remove the unneeded *_path attributes
@@ -51,7 +51,14 @@ module LiquidMigrator
   end
 
   def self.convert_json_path(string, filter = "")
-    "{{#{string[2..-1].gsub(/\.\*\Z/, '')}#{filter}}}"
+    check_path(string)
+    "{{#{string[2..-1]}#{filter}}}"
+  end
+
+  def self.check_path(string)
+    if string !~ /\A(\$\.)?(\w+\.)*(\w+)\Z/
+      raise "JSONPath '#{string}' is too complex, please check your migration."
+    end
   end
 end
 
diff --git a/spec/lib/liquid_migrator_spec.rb b/spec/lib/liquid_migrator_spec.rb
index 6664b7de..c2858e3c 100644
--- a/spec/lib/liquid_migrator_spec.rb
+++ b/spec/lib/liquid_migrator_spec.rb
@@ -5,7 +5,6 @@ describe LiquidMigrator do
     it "should work" do
       LiquidMigrator.convert_string("$.data", true).should == "{{data}}"
       LiquidMigrator.convert_string("$.data.test", true).should == "{{data.test}}"
-      LiquidMigrator.convert_string("$.data.test.*", true).should == "{{data.test}}"
     end
 
     it "should ignore strings which just contain a JSONPath" do
@@ -13,12 +12,14 @@ describe LiquidMigrator do
       LiquidMigrator.convert_string(" $.data", true).should == " $.data"
       LiquidMigrator.convert_string("lorem $.data", true).should == "lorem $.data"
     end
+    it "should raise an exception when encountering complex JSONPaths" do
+      expect { LiquidMigrator.convert_string("$.data.test.*", true) }.
+        to raise_error("JSONPath '$.data.test.*' is too complex, please check your migration.")
+    end
   end
 
   describe "converting escaped JSONPath strings" do
     it "should work" do
-      LiquidMigrator.convert_string("Received <$.content.text.*> from <$.content.name> .").should ==
-                                    "Received {{content.text}} from {{content.name}} ."
       LiquidMigrator.convert_string("Weather looks like <$.conditions> according to the forecast at <$.pretty_date.time>").should ==
                                     "Weather looks like {{conditions}} according to the forecast at {{pretty_date.time}}"
     end
@@ -27,6 +28,11 @@ describe LiquidMigrator do
       LiquidMigrator.convert_string("Escaped: <escape $.content.name>\nNot escaped: <$.content.name>").should ==
                                     "Escaped: {{content.name | uri_escape}}\nNot escaped: {{content.name}}"
     end
+
+    it "should raise an exception when encountering complex JSONPaths" do
+      expect { LiquidMigrator.convert_string("Received <$.content.text.*> from <$.content.name> .") }.
+        to raise_error("JSONPath '$.content.text.*' is too complex, please check your migration.")
+    end
   end
 
   describe "migrating a hash" do
@@ -42,6 +48,10 @@ describe LiquidMigrator do
       LiquidMigrator.convert_hash({'a' => "default", 'a_path' => "$.data"}, {leading_dollarsign_is_jsonpath: true, merge_path_attributes: true}).should ==
                                   {'a' => "{{data}}"}
     end
+    it "should raise an exception when encountering complex JSONPaths" do
+      expect { LiquidMigrator.convert_hash({'b' => "This is <$.complex[2]>"}) }.
+        to raise_error("JSONPath '$.complex[2]' is too complex, please check your migration.")
+    end
   end
 
   describe "migrating an actual agent" do
@@ -69,5 +79,11 @@ describe LiquidMigrator do
       LiquidMigrator.convert_all_agent_options(@agent)
       @agent.reload.options.should == {"auth_token" => 'token', 'color' => 'yellow', 'notify' => false, 'room_name' => 'test', 'username' => '{{username}}', 'message' => '{{message}}'}
     end
+
+    it "should raise an exception when encountering complex JSONPaths" do
+      @agent.options['username_path'] = "$.very.complex[*]"
+      expect { LiquidMigrator.convert_all_agent_options(@agent) }.
+        to raise_error("JSONPath '$.very.complex[*]' is too complex, please check your migration.")
+    end
   end
 end
\ No newline at end of file