From 6b999d7b0c60c06ee1b5437780a0e0dec9c5b5c9 Mon Sep 17 00:00:00 2001 From: Jon Pascoe Date: Thu, 18 Jul 2024 22:40:18 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + lib/ice_cube/rule.rb | 5 +- lib/ice_cube/validations/day_of_week.rb | 3 + spec/examples/from_hash_spec.rb | 129 ++++++++++++++++++ spec/examples/time_util_spec.rb | 30 ++-- spec/{examples => i18n}/to_s_de_spec.rb | 0 spec/{examples => i18n}/to_s_en_spec.rb | 0 spec/{examples => i18n}/to_s_es_spec.rb | 0 spec/{examples => i18n}/to_s_id_spec.rb | 0 spec/{examples => i18n}/to_s_ja_spec.rb | 0 spec/{examples => rules}/daily_rule_spec.rb | 0 spec/{examples => rules}/hourly_rule_spec.rb | 0 .../{examples => rules}/minutely_rule_spec.rb | 0 spec/{examples => rules}/monthly_rule_spec.rb | 0 .../{examples => rules}/secondly_rule_spec.rb | 0 spec/{examples => rules}/weekly_rule_spec.rb | 2 +- spec/{examples => rules}/yearly_rule_spec.rb | 2 +- 17 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 spec/examples/from_hash_spec.rb rename spec/{examples => i18n}/to_s_de_spec.rb (100%) rename spec/{examples => i18n}/to_s_en_spec.rb (100%) rename spec/{examples => i18n}/to_s_es_spec.rb (100%) rename spec/{examples => i18n}/to_s_id_spec.rb (100%) rename spec/{examples => i18n}/to_s_ja_spec.rb (100%) rename spec/{examples => rules}/daily_rule_spec.rb (100%) rename spec/{examples => rules}/hourly_rule_spec.rb (100%) rename spec/{examples => rules}/minutely_rule_spec.rb (100%) rename spec/{examples => rules}/monthly_rule_spec.rb (100%) rename spec/{examples => rules}/secondly_rule_spec.rb (100%) rename spec/{examples => rules}/weekly_rule_spec.rb (99%) rename spec/{examples => rules}/yearly_rule_spec.rb (96%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c869887..3080c208 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/ice_cube/rule.rb b/lib/ice_cube/rule.rb index 6652c3db..30689267 100644 --- a/lib/ice_cube/rule.rb +++ b/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] diff --git a/lib/ice_cube/validations/day_of_week.rb b/lib/ice_cube/validations/day_of_week.rb index c04695bc..9fb9f1cd 100644 --- a/lib/ice_cube/validations/day_of_week.rb +++ b/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 diff --git a/spec/examples/from_hash_spec.rb b/spec/examples/from_hash_spec.rb new file mode 100644 index 00000000..7ef09632 --- /dev/null +++ b/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 diff --git a/spec/examples/time_util_spec.rb b/spec/examples/time_util_spec.rb index 43491a8b..c6a434d6 100644 --- a/spec/examples/time_util_spec.rb +++ b/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 diff --git a/spec/examples/to_s_de_spec.rb b/spec/i18n/to_s_de_spec.rb similarity index 100% rename from spec/examples/to_s_de_spec.rb rename to spec/i18n/to_s_de_spec.rb diff --git a/spec/examples/to_s_en_spec.rb b/spec/i18n/to_s_en_spec.rb similarity index 100% rename from spec/examples/to_s_en_spec.rb rename to spec/i18n/to_s_en_spec.rb diff --git a/spec/examples/to_s_es_spec.rb b/spec/i18n/to_s_es_spec.rb similarity index 100% rename from spec/examples/to_s_es_spec.rb rename to spec/i18n/to_s_es_spec.rb diff --git a/spec/examples/to_s_id_spec.rb b/spec/i18n/to_s_id_spec.rb similarity index 100% rename from spec/examples/to_s_id_spec.rb rename to spec/i18n/to_s_id_spec.rb diff --git a/spec/examples/to_s_ja_spec.rb b/spec/i18n/to_s_ja_spec.rb similarity index 100% rename from spec/examples/to_s_ja_spec.rb rename to spec/i18n/to_s_ja_spec.rb diff --git a/spec/examples/daily_rule_spec.rb b/spec/rules/daily_rule_spec.rb similarity index 100% rename from spec/examples/daily_rule_spec.rb rename to spec/rules/daily_rule_spec.rb diff --git a/spec/examples/hourly_rule_spec.rb b/spec/rules/hourly_rule_spec.rb similarity index 100% rename from spec/examples/hourly_rule_spec.rb rename to spec/rules/hourly_rule_spec.rb diff --git a/spec/examples/minutely_rule_spec.rb b/spec/rules/minutely_rule_spec.rb similarity index 100% rename from spec/examples/minutely_rule_spec.rb rename to spec/rules/minutely_rule_spec.rb diff --git a/spec/examples/monthly_rule_spec.rb b/spec/rules/monthly_rule_spec.rb similarity index 100% rename from spec/examples/monthly_rule_spec.rb rename to spec/rules/monthly_rule_spec.rb diff --git a/spec/examples/secondly_rule_spec.rb b/spec/rules/secondly_rule_spec.rb similarity index 100% rename from spec/examples/secondly_rule_spec.rb rename to spec/rules/secondly_rule_spec.rb diff --git a/spec/examples/weekly_rule_spec.rb b/spec/rules/weekly_rule_spec.rb similarity index 99% rename from spec/examples/weekly_rule_spec.rb rename to spec/rules/weekly_rule_spec.rb index 3e8e5719..c59de2d2 100644 --- a/spec/examples/weekly_rule_spec.rb +++ b/spec/rules/weekly_rule_spec.rb @@ -96,7 +96,7 @@ module IceCube it "should raise an error on invalid input" do schedule = Schedule.new(WEDNESDAY) - expect { schedule.add_recurrence_rule Rule.weekly.day(["1", "3"]) }.to raise_error(ArgumentError) + expect { schedule.add_recurrence_rule Rule.weekly.day(["1", "3"]) }.to raise_error(ArgumentError, "expecting Integer or Symbol value for day, got \"1\"") end it "should ignore weekday validation when no days are specified" do diff --git a/spec/examples/yearly_rule_spec.rb b/spec/rules/yearly_rule_spec.rb similarity index 96% rename from spec/examples/yearly_rule_spec.rb rename to spec/rules/yearly_rule_spec.rb index 1e3462ef..c7869917 100644 --- a/spec/examples/yearly_rule_spec.rb +++ b/spec/rules/yearly_rule_spec.rb @@ -38,7 +38,7 @@ schedule.add_recurrence_rule IceCube::Rule.yearly.month_of_year(:april).day_of_week(monday: [1, -1]) one_year = 365 * IceCube::ONE_DAY - expect(schedule.occurrences(start_time + one_year).size).to eq(2) + expect(schedule.occurrences(start_time + one_year)).to eq [Time.local(2011, 4, 4, 5, 0, 0), Time.local(2011, 4, 25, 5, 0, 0)] end it "should produce the correct number of days for @interval = 1" do