Merge pull request IBMPrivateCloud/bluecf#1 from Jason-Frey/remove_eval

Fix eval of user-input inside of a numeric_set expression

(cherry picked from commit 75a00c6)
(cherry picked from commit e90df11)

lib/miq_expression.rb

@@ -662,16 +662,7 @@ def self.quote(val, typ)
       val.to_s.to_f_with_method
     when "numeric_set"
       val = val.split(",") if val.kind_of?(String)
-      v_arr = val.to_miq_a.flat_map do |v|
-        if v.kind_of?(String)
-          v = begin
-                eval(v)
-              rescue
-                nil
-              end
-        end
-        v.kind_of?(Range) ? v.to_a : v
-      end.compact.uniq.sort
+      v_arr = Array.wrap(val).flat_map { |v| quote_numeric_set_atom(v) }.compact.uniq.sort
       "[#{v_arr.join(",")}]"
     when "string_set"
       val = val.split(",") if val.kind_of?(String)
@@ -682,6 +673,32 @@ def self.quote(val, typ)
     end
   end
 
+  private_class_method def self.quote_numeric_set_atom(val)
+    val = val.to_s unless val.kind_of?(Numeric) || val.kind_of?(Range)
+
+    if val.kind_of?(String)
+      val = val.strip
+      val =
+        if val.include?("..") # Parse Ranges
+          b, e = val.split("..", 2).map do |i|
+            if integer?(i)
+              i.to_i_with_method
+            elsif numeric?(i)
+              i.to_f_with_method
+            end
+          end
+
+          Range.new(b, e) if b && e
+        elsif integer?(val) # Parse Integers
+          val.to_i_with_method
+        elsif numeric?(val) # Parse Floats
+          val.to_f_with_method
+        end
+    end
+
+    val.kind_of?(Range) ? val.to_a : val
+  end
+
   def self.quote_human(val, typ)
     case typ.to_s
     when "integer", "decimal", "fixnum", "float"

spec/lib/miq_expression_spec.rb

@@ -1319,9 +1319,34 @@
       expect { exp.to_ruby }.to raise_error(/operator 'RUBY' is not supported/)
     end
 
-    it "tests numeric set expressions" do
-      exp = MiqExpression.new("=" => {"field" => "Host-enabled_inbound_ports", "value" => "22,427,5988,5989"})
-      expect(exp.to_ruby).to eq('<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> == [22,427,5988,5989]')
+    it "ignores invalid values for a numeric_set in an = expression" do
+      actual = described_class.new("=" => {"field" => "Host-enabled_inbound_ports", "value" => "22, 427, 5988, 5989, foo, `echo 1000`.to_i, abc..123, 1..4"}).to_ruby
+      expected = "<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> == [1,2,3,4,22,427,5988,5989]"
+      expect(actual).to eq(expected)
+    end
+
+    it "ignores invalid values for a numeric_set in an INCLUDES ALL expression" do
+      actual = described_class.new("INCLUDES ALL" => {"field" => "Host-enabled_inbound_ports", "value" => "22, 427, 5988, 5989, foo, `echo 1000`.to_i, abc..123, 1..4"}).to_ruby
+      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> & [1,2,3,4,22,427,5988,5989]) == [1,2,3,4,22,427,5988,5989]"
+      expect(actual).to eq(expected)
+    end
+
+    it "ignores invalid values for a numeric_set in an INCLUDES ANY expression" do
+      actual = described_class.new("INCLUDES ANY" => {"field" => "Host-enabled_inbound_ports", "value" => "22, 427, 5988, 5989, foo, `echo 1000`.to_i, abc..123, 1..4"}).to_ruby
+      expected = "([1,2,3,4,22,427,5988,5989] - <value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value>) != [1,2,3,4,22,427,5988,5989]"
+      expect(actual).to eq(expected)
+    end
+
+    it "ignores invalid values for a numeric_set in an INCLUDES ONLY expression" do
+      actual = described_class.new("INCLUDES ONLY" => {"field" => "Host-enabled_inbound_ports", "value" => "22, 427, 5988, 5989, foo, `echo 1000`.to_i, abc..123, 1..4"}).to_ruby
+      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [1,2,3,4,22,427,5988,5989]) == []"
+      expect(actual).to eq(expected)
+    end
+
+    it "ignores invalid values for a numeric_set in an LIMITED TO expression" do
+      actual = described_class.new("LIMITED TO" => {"field" => "Host-enabled_inbound_ports", "value" => "22, 427, 5988, 5989, foo, `echo 1000`.to_i, abc..123, 1..4"}).to_ruby
+      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [1,2,3,4,22,427,5988,5989]) == []"
+      expect(actual).to eq(expected)
     end
 
     it "escapes forward slashes for values in REGULAR EXPRESSION MATCHES expressions" do
@@ -1429,10 +1454,10 @@
       exp:
         INCLUDES ANY:
           field: Host-enabled_inbound_ports
-          value: 22, 427, 5988, 5989, 1..3
+          value: 22, 427, 5988, 5989, 1..4
       '
 
-      expected = "([1,2,3,22,427,5988,5989] - <value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value>) != [1,2,3,22,427,5988,5989]"
+      expected = "([1,2,3,4,22,427,5988,5989] - <value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value>) != [1,2,3,4,22,427,5988,5989]"
       expect(filter.to_ruby).to eq(expected)
     end
 
@@ -1441,10 +1466,10 @@
       exp:
         INCLUDES ONLY:
           field: Host-enabled_inbound_ports
-          value: 22
+          value: 22, 427, 5988, 5989, 1..4
       '
 
-      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [22]) == []"
+      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [1,2,3,4,22,427,5988,5989]) == []"
       expect(filter.to_ruby).to eq(expected)
     end
 
@@ -1453,10 +1478,10 @@
       exp:
         LIMITED TO:
           field: Host-enabled_inbound_ports
-          value: 22
+          value: 22, 427, 5988, 5989, 1..4
       '
 
-      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [22]) == []"
+      expected = "(<value ref=host, type=numeric_set>/virtual/enabled_inbound_ports</value> - [1,2,3,4,22,427,5988,5989]) == []"
       expect(filter.to_ruby).to eq(expected)
     end