Day-of-week validation input checking (#554)

* Update documentation links in gemspec

* Add rule_type validation specs

* Validate occurrence passed to day_of_week

* Update CHANGELOG

* linting

* Undo linting

* Move rule specs to new folder

* Move i18n specs to new folder

* Check the actual values, not just the count

* Update argument validations

Author: pacso

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - When using a rule with hour_of_day validations, and asking for occurrences on the day that DST skips forward, valid occurrences would be missed. ([#464](https://github.com/seejohnrun/ice_cube/pull/464)) by [@jakebrady5](https://github.com/jakebrady5)
 - Include `exrules` when exporting a schedule to YAML, JSON or a Hash. ([#519](https://github.com/ice-cube-ruby/ice_cube/pull/519)) by [@pacso](https://github.com/pacso)
 - Documentation links updated to point to the new repository location. ([#553](https://github.com/ice-cube-ruby/ice_cube/pull/553)) by [@pacso](https://github.com/pacso)
+- Input validation added for day_of_week validator. Fixes infinite loops when invalid day_of_week occurrences are provided. ([#554](https://github.com/ice-cube-ruby/ice_cube/pull/554)) by [@pacso](https://github.com/pacso)
 
 ## [0.16.4] - 2021-10-21
 ### Added

lib/ice_cube/rule.rb

@@ -73,10 +73,7 @@ def from_hash(original_hash)
 
         rule = IceCube::Rule.send(interval_type, hash[:interval] || 1)
 
-        if match[1] == "Weekly"
-          rule.interval(hash[:interval] || 1, TimeUtil.wday_to_sym(hash[:week_start] || 0))
-        end
-
+        rule.interval(hash[:interval] || 1, TimeUtil.wday_to_sym(hash[:week_start] || 0)) if rule.is_a? WeeklyRule
         rule.until(TimeUtil.deserialize_time(hash[:until])) if hash[:until]
         rule.count(hash[:count]) if hash[:count]
 

lib/ice_cube/validations/day_of_week.rb

@@ -15,6 +15,9 @@ class Validation
       attr_reader :day, :occ
 
       def initialize(day, occ)
+        raise ArgumentError, "Integer occurrence value required" unless occ.is_a?(Integer)
+        raise ArgumentError, "Invalid day_of_week occurrence: #{occ.inspect}" if occ.zero? || occ.abs > 5
+
         @day = day
         @occ = occ
       end

spec/examples/from_hash_spec.rb

@@ -0,0 +1,129 @@
+require File.dirname(__FILE__) + "/../spec_helper"
+
+module IceCube
+  describe Rule, "from_hash" do
+    describe "rule_type validations" do
+      it "should raise an ArgumentError when the hash is empty" do
+        expect { Rule.from_hash({}) }
+          .to raise_error ArgumentError, "Invalid rule type"
+      end
+
+      it "should raise an ArgumentError when the hash[:rule_type] is invalid" do
+        expect { Rule.from_hash({rule_type: "IceCube::MadeUpIntervalRule"}) }
+          .to raise_error ArgumentError, "Invalid rule frequency type: MadeUpInterval"
+      end
+
+      it "returns a SecondlyRule when the hash[:rule_type] is secondly" do
+        expect(Rule.from_hash({rule_type: "IceCube::SecondlyRule"})).to be_a SecondlyRule
+      end
+
+      it "returns a MinutelyRule when the hash[:rule_type] is minutely" do
+        expect(Rule.from_hash({rule_type: "IceCube::MinutelyRule"})).to be_a MinutelyRule
+      end
+
+      it "returns a HourlyRule when the hash[:rule_type] is hourly" do
+        expect(Rule.from_hash({rule_type: "IceCube::HourlyRule"})).to be_a HourlyRule
+      end
+
+      it "returns a DailyRule when the hash[:rule_type] is daily" do
+        expect(Rule.from_hash({rule_type: "IceCube::DailyRule"})).to be_a DailyRule
+      end
+
+      it "returns a WeeklyRule when the hash[:rule_type] is weekly" do
+        expect(Rule.from_hash({rule_type: "IceCube::WeeklyRule"})).to be_a WeeklyRule
+      end
+
+      it "returns a MonthlyRule when the hash[:rule_type] is monthly" do
+        expect(Rule.from_hash({rule_type: "IceCube::MonthlyRule"})).to be_a MonthlyRule
+      end
+
+      it "returns a YearlyRule when the hash[:rule_type] is yearly" do
+        expect(Rule.from_hash({rule_type: "IceCube::YearlyRule"})).to be_a YearlyRule
+      end
+    end
+
+    describe "creating monthly rule" do
+      context "with valid day_of_week validations" do
+        let(:input_hash) {
+          {
+            rule_type: "IceCube::MonthlyRule",
+            interval: 1,
+            validations: {
+              day_of_week: {
+                "0": [2],
+                "1": [2],
+                "2": [2],
+                "3": [2],
+                "4": [2],
+                "5": [2],
+                "6": [1, 2]
+              },
+              hour_of_day: 7,
+              minute_of_hour: 19
+            }
+          }
+        }
+
+        it "can provide the first occurrence" do
+          rule = Rule.from_hash(input_hash)
+          schedule = Schedule.new(Time.utc(2010, 1, 1, 0, 0, 0))
+          schedule.add_recurrence_rule rule
+          expect(schedule.first(10).map(&:to_time)).to eq([
+            Time.utc(2010, 1, 2, 7, 19, 0),
+            Time.utc(2010, 1, 8, 7, 19, 0),
+            Time.utc(2010, 1, 9, 7, 19, 0),
+            Time.utc(2010, 1, 10, 7, 19, 0),
+            Time.utc(2010, 1, 11, 7, 19, 0),
+            Time.utc(2010, 1, 12, 7, 19, 0),
+            Time.utc(2010, 1, 13, 7, 19, 0),
+            Time.utc(2010, 1, 14, 7, 19, 0),
+            Time.utc(2010, 2, 6, 7, 19, 0),
+            Time.utc(2010, 2, 8, 7, 19, 0)
+          ])
+        end
+      end
+
+      context "with invalid day_of_week validations" do
+        let(:input_hash_with_zeroeth_occurrence) {
+          {
+            rule_type: "IceCube::MonthlyRule",
+            interval: 1,
+            validations: {
+              day_of_week: {
+                "1": [],
+                "2": [0],
+                "3": [],
+                "4": []
+              },
+              hour_of_day: 7,
+              minute_of_hour: 19
+            }
+          }
+        }
+        let(:input_hash_with_sixth_occurrence) {
+          {
+            rule_type: "IceCube::MonthlyRule",
+            interval: 1,
+            validations: {
+              day_of_week: {
+                "1": [],
+                "2": [6],
+                "3": [],
+                "4": []
+              },
+              hour_of_day: 7,
+              minute_of_hour: 19
+            }
+          }
+        }
+
+        it "should raise an ArgumentError" do
+          expect { Rule.from_hash(input_hash_with_zeroeth_occurrence) }
+            .to raise_error ArgumentError, "Invalid day_of_week occurrence: 0"
+          expect { Rule.from_hash(input_hash_with_sixth_occurrence) }
+            .to raise_error ArgumentError, "Invalid day_of_week occurrence: 6"
+        end
+      end
+    end
+  end
+end

spec/examples/time_util_spec.rb

@@ -42,32 +42,38 @@ module IceCube
     end
 
     describe :wday_to_sym do
-      it "converts 0..6 to weekday symbols" do
-        expect(TimeUtil.wday_to_sym(1)).to eq(:monday)
-      end
+      [:sunday, :monday, :tuesday, :wednesday, :thursday, :friday, :saturday].each_with_index do |sym, i|
+        it "converts #{i} to weekday symbol :#{sym}" do
+          expect(TimeUtil.wday_to_sym(i)).to eq(sym)
+        end
 
-      it "returns weekday symbols as is" do
-        expect(TimeUtil.wday_to_sym(:monday)).to eq(:monday)
+        it "returns :#{sym} when passed :#{sym}" do
+          expect(TimeUtil.wday_to_sym(sym)).to eq(sym)
+        end
       end
 
       it "raises an error for bad input" do
         expect { TimeUtil.wday_to_sym(:anyday) }.to raise_error(ArgumentError)
-        expect { TimeUtil.wday_to_sym(17) }.to raise_error(ArgumentError)
+        expect { TimeUtil.wday_to_sym(8) }.to raise_error(ArgumentError)
+        expect { TimeUtil.wday_to_sym(-1) }.to raise_error(ArgumentError)
       end
     end
 
     describe :sym_to_wday do
-      it "converts weekday symbols to 0..6 wday numbers" do
-        expect(TimeUtil.sym_to_wday(:monday)).to eq(1)
-      end
+      [:sunday, :monday, :tuesday, :wednesday, :thursday, :friday, :saturday].each_with_index do |sym, i|
+        it "converts :#{sym} to integer #{i}" do
+          expect(TimeUtil.sym_to_wday(sym)).to eq(i)
+        end
 
-      it "returns wday numbers as is" do
-        expect(TimeUtil.sym_to_wday(1)).to eq(1)
+        it "returns #{i} when passed #{i}" do
+          expect(TimeUtil.sym_to_wday(i)).to eq(i)
+        end
       end
 
       it "raises an error for bad input" do
         expect { TimeUtil.sym_to_wday(:anyday) }.to raise_error(ArgumentError)
-        expect { TimeUtil.sym_to_wday(17) }.to raise_error(ArgumentError)
+        expect { TimeUtil.sym_to_wday(-1) }.to raise_error(ArgumentError)
+        expect { TimeUtil.sym_to_wday(8) }.to raise_error(ArgumentError)
       end
     end