Fix expression normalization in advanced cases

Author: RTLcoil

lib/cloudinary/utils.rb

@@ -32,19 +32,34 @@ class Cloudinary::Utils
 
   PREDEFINED_VARS = {
     "aspect_ratio"         => "ar",
+    "aspectRatio"          => "ar",
     "current_page"         => "cp",
+    "currentPage"          => "cp",
     "face_count"           => "fc",
+    "faceCount"            => "fc",
     "height"               => "h",
     "initial_aspect_ratio" => "iar",
+    "initialAspectRatio"   => "iar",
+    "trimmed_aspect_ratio" => "tar",
+    "trimmedAspectRatio"   => "tar",
     "initial_height"       => "ih",
+    "initialHeight"        => "ih",
     "initial_width"        => "iw",
+    "initialWidth"         => "iw",
     "page_count"           => "pc",
+    "pageCount"            => "pc",
     "page_x"               => "px",
+    "pageX"                => "px",
     "page_y"               => "py",
+    "pageY"                => "py",
     "tags"                 => "tags",
     "initial_duration"     => "idu",
+    "initialDuration"      => "idu",
     "duration"             => "du",
-    "width"                => "w"
+    "width"                => "w",
+    "illustration_score"   => "ils",
+    "illustrationScore"    => "ils",
+    "context"              => "ctx"
   }
 
   SIMPLE_TRANSFORMATION_PARAMS = {
@@ -317,16 +332,16 @@ def self.process_if(if_value)
     "if_" + normalize_expression(if_value) unless if_value.to_s.empty?
   end
 
-  EXP_REGEXP = Regexp.new('(?<!\$)('+PREDEFINED_VARS.keys.join("|")+')'+'|('+CONDITIONAL_OPERATORS.keys.reverse.map { |k| Regexp.escape(k) }.join('|')+')(?=[ _])')
+  EXP_REGEXP = Regexp.new('(\$_*[^_ ]+)|(?<!\$)('+PREDEFINED_VARS.keys.join("|")+')'+'|('+CONDITIONAL_OPERATORS.keys.reverse.map { |k| Regexp.escape(k) }.join('|')+')(?=[ _])')
   EXP_REPLACEMENT = PREDEFINED_VARS.merge(CONDITIONAL_OPERATORS)
 
   def self.normalize_expression(expression)
     if expression.nil?
-      ''
+      nil
     elsif expression.is_a?( String) && expression =~ /^!.+!$/ # quoted string
       expression
     else
-      expression.to_s.gsub(EXP_REGEXP,EXP_REPLACEMENT).gsub(/[ _]+/, "_")
+      expression.to_s.gsub(EXP_REGEXP) { |match| EXP_REPLACEMENT[match] || match }.gsub(/[ _]+/, "_")
     end
   end
 

spec/expression_normalization_spec.rb

@@ -0,0 +1,44 @@
+require "spec_helper"
+
+describe "Expression normalization" do
+  {
+    'nil is not affected' => [nil, nil],
+    'number replaced with a string value' => [10, '10'],
+    'empty string is not affected' => ['', ''],
+    'single space is replaced with a single underscore' => [' ', '_'],
+    'blank string is replaced with a single underscore' => ['   ', '_'],
+    'underscore is not affected' => ['_', '_'],
+    'sequence of underscores and spaces is replaced with a single underscore' => [' _ __  _', '_'],
+    'arbitrary text is not affected' => ['foobar', 'foobar'],
+    'double ampersand replaced with and operator' => ['foo && bar', 'foo_and_bar'],
+    'double ampersand with no space at the end is not affected' => ['foo&&bar', 'foo&&bar'],
+    'width recognized as variable and replaced with w' => ['width', 'w'],
+    'initial aspect ratio recognized as variable and replaced with iar' => ['initial_aspect_ratio', 'iar'],
+    '$width recognized as user variable and not affected' => ['$width', '$width'],
+    '$initial_aspect_ratio recognized as user variable followed by aspect_ratio variable' => [
+      '$initial_aspect_ratio',
+      '$initial_ar',
+    ],
+    '$mywidth recognized as user variable and not affected' => ['$mywidth', '$mywidth'],
+    '$widthwidth recognized as user variable and not affected' => ['$widthwidth', '$widthwidth'],
+    '$_width recognized as user variable and not affected' => ['$_width', '$_width'],
+    '$__width recognized as user variable and not affected' => ['$__width', '$_width'],
+    '$$width recognized as user variable and not affected' => ['$$width', '$$width'],
+    '$height recognized as user variable and not affected' => ['$height_100', '$height_100'],
+    '$heightt_100 recognized as user variable and not affected' => ['$heightt_100', '$heightt_100'],
+    '$$height_100 recognized as user variable and not affected' => ['$$height_100', '$$height_100'],
+    '$heightmy_100 recognized as user variable and not affected' => ['$heightmy_100', '$heightmy_100'],
+    '$myheight_100 recognized as user variable and not affected' => ['$myheight_100', '$myheight_100'],
+    '$heightheight_100 recognized as user variable and not affected' => [
+      '$heightheight_100',
+      '$heightheight_100',
+    ],
+    '$theheight_100 recognized as user variable and not affected' => ['$theheight_100', '$theheight_100'],
+    '$__height_100 recognized as user variable and not affected' => ['$__height_100', '$_height_100']
+  }.each do |test_description, (input, expected)|
+    specify test_description do
+      actual = Cloudinary::Utils.normalize_expression(input)
+      expect(actual).to eq(expected)
+    end
+  end
+end

spec/utils_spec.rb

@@ -1077,6 +1077,18 @@
       t = Cloudinary::Utils.generate_transformation_string options, true
       expect(t).to eq("$width_10/w_$width_add_10_add_w")
     end
+
+    it "should not affect user variable names containing predefined names" do
+      options = {
+        :transformation => [
+          { :variables => [ ["$aheight", 300], ["$mywidth", "100"] ] },
+          { :width => "3 + $mywidth * 3 + 4 / 2 * initialWidth * $mywidth", :height => "3 * initialHeight + $aheight" },
+        ]
+      }
+
+      t = Cloudinary::Utils.generate_transformation_string options, true
+      expect(t).to eq("$aheight_300,$mywidth_100/h_3_mul_ih_add_$aheight,w_3_add_$mywidth_mul_3_add_4_div_2_mul_iw_mul_$mywidth")
+    end
   end
 
     describe "context" do