From 215eb3476f9a5593d3fa847452b01b604595febd Mon Sep 17 00:00:00 2001 From: Reindl Date: Tue, 20 Jan 2026 11:01:01 -0600 Subject: [PATCH 1/5] API-11280 Enforce reasonable limit on days and weekdays --- CHANGELOG.md | 2 ++ VERSION | 2 +- lib/sparkql/function_resolver.rb | 25 ++++++++++++++++---- test/unit/function_resolver_test.rb | 36 +++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89a0608..1d96bcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +v1.3.4, 20265-01-20 + * [BUGFIX] Validate limit for days and weekdays v1.3.3, 2025-08-12 ------------------- diff --git a/VERSION b/VERSION index 31e5c84..d0149fe 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.3 +1.3.4 diff --git a/lib/sparkql/function_resolver.rb b/lib/sparkql/function_resolver.rb index 3857c63..325bf3c 100644 --- a/lib/sparkql/function_resolver.rb +++ b/lib/sparkql/function_resolver.rb @@ -21,6 +21,7 @@ class FunctionResolver VALID_REGEX_FLAGS = ['', 'i'].freeze MIN_DATE_TIME = Time.new(1970, 1, 1, 0, 0, 0, '+00:00').iso8601 MAX_DATE_TIME = Time.new(9999, 12, 31, 23, 59, 59, '+00:00').iso8601 + MAX_DAYS = (365 * 1000).freeze # 1000 years ought to cover most cases VALID_CAST_TYPES = %i[field character decimal integer].freeze SUPPORTED_FUNCTIONS = { @@ -43,16 +44,16 @@ class FunctionResolver regex: { args: [:character], opt_args: [{ - type: :character, - default: '' - }], + type: :character, + default: '' + }], return_type: :character }, substring: { args: [%i[field character], :integer], opt_args: [{ - type: :integer - }], + type: :integer + }], resolve_for_type: true, return_type: :character }, @@ -526,6 +527,13 @@ def hours(num) # Offset the current timestamp by a number of days def days(number_of_days) + if number_of_days.abs > MAX_DAYS + @errors << Sparkql::ParserError.new(token: number_of_days, + message: "Function call 'days' max offset #{MAX_DAYS} days", + status: :fatal) + return + end + # date calculated as the offset from midnight tommorrow. Zero will provide values for all times # today. d = current_date + number_of_days @@ -536,6 +544,13 @@ def days(number_of_days) end def weekdays(number_of_days) + if number_of_days.abs > MAX_DAYS + @errors << Sparkql::ParserError.new(token: number_of_days, + message: "Function call 'weekdays' max offset #{MAX_DAYS} days", + status: :fatal) + return + end + today = current_date weekend_start = today.saturday? || today.sunday? direction = number_of_days.positive? ? 1 : -1 diff --git a/test/unit/function_resolver_test.rb b/test/unit/function_resolver_test.rb index c471b5f..192ee5c 100644 --- a/test/unit/function_resolver_test.rb +++ b/test/unit/function_resolver_test.rb @@ -445,6 +445,24 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) end end + test 'days - exceed max' do + f = FunctionResolver.new('days', + [{ type: :integer, value: 365_001 }], + current_timestamp: EXAMPLE_DATE) + f.validate + refute f.errors? + assert_nil f.call + assert f.errors?, "function 'days' limit 365000" + + f = FunctionResolver.new('days', + [{ type: :integer, value: -365_001 }], + current_timestamp: EXAMPLE_DATE) + f.validate + refute f.errors? + assert_nil f.call + assert f.errors?, "function 'days' limit 365000" + end + test 'weekdays()' do friday = Date.new(2012, 10, 19) saturday = Date.new(2012, 10, 20) @@ -514,6 +532,24 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) end end + test 'weekdays - exceed max' do + f = FunctionResolver.new('weekdays', + [{ type: :integer, value: 365_001 }], + current_timestamp: EXAMPLE_DATE) + f.validate + refute f.errors? + assert_nil f.call + assert f.errors?, "function 'weekdays' limit 365000" + + f = FunctionResolver.new('weekdays', + [{ type: :integer, value: -365_001 }], + current_timestamp: EXAMPLE_DATE) + f.validate + refute f.errors? + assert_nil f.call + assert f.errors?, "function 'weekdays' limit 365000" + end + test 'months()' do f = FunctionResolver.new('months', [{ type: :integer, value: 3 }], From fe0f8f07731c61b4a837d4bc50265d3286266064 Mon Sep 17 00:00:00 2001 From: Reindl Date: Tue, 20 Jan 2026 11:47:38 -0600 Subject: [PATCH 2/5] Fixing tests --- test/unit/function_resolver_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/function_resolver_test.rb b/test/unit/function_resolver_test.rb index 192ee5c..ee92b28 100644 --- a/test/unit/function_resolver_test.rb +++ b/test/unit/function_resolver_test.rb @@ -450,7 +450,7 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) [{ type: :integer, value: 365_001 }], current_timestamp: EXAMPLE_DATE) f.validate - refute f.errors? + assert !f.errors? assert_nil f.call assert f.errors?, "function 'days' limit 365000" @@ -458,7 +458,7 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) [{ type: :integer, value: -365_001 }], current_timestamp: EXAMPLE_DATE) f.validate - refute f.errors? + assert !f.errors? assert_nil f.call assert f.errors?, "function 'days' limit 365000" end @@ -537,7 +537,7 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) [{ type: :integer, value: 365_001 }], current_timestamp: EXAMPLE_DATE) f.validate - refute f.errors? + assert !f.errors? assert_nil f.call assert f.errors?, "function 'weekdays' limit 365000" @@ -545,7 +545,7 @@ def assert_times(call_value, expected_call_type = :datetime, overrides = {}) [{ type: :integer, value: -365_001 }], current_timestamp: EXAMPLE_DATE) f.validate - refute f.errors? + assert !f.errors? assert_nil f.call assert f.errors?, "function 'weekdays' limit 365000" end From 0de6aab90479e06ff70407b97c8e7cac3bd42e7d Mon Sep 17 00:00:00 2001 From: Reindl Date: Tue, 20 Jan 2026 13:51:00 -0600 Subject: [PATCH 3/5] Would like the error message to percolate up to the api response. Syntax errors are masked. --- lib/sparkql/function_resolver.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/sparkql/function_resolver.rb b/lib/sparkql/function_resolver.rb index 325bf3c..c66d476 100644 --- a/lib/sparkql/function_resolver.rb +++ b/lib/sparkql/function_resolver.rb @@ -530,7 +530,9 @@ def days(number_of_days) if number_of_days.abs > MAX_DAYS @errors << Sparkql::ParserError.new(token: number_of_days, message: "Function call 'days' max offset #{MAX_DAYS} days", - status: :fatal) + status: :fatal, + syntax: false, + constraint: true) return end @@ -547,7 +549,9 @@ def weekdays(number_of_days) if number_of_days.abs > MAX_DAYS @errors << Sparkql::ParserError.new(token: number_of_days, message: "Function call 'weekdays' max offset #{MAX_DAYS} days", - status: :fatal) + status: :fatal, + syntax: false, + constraint: true) return end From cb94052511764f26799ae70650361fec545f2c67 Mon Sep 17 00:00:00 2001 From: Reindl Date: Tue, 20 Jan 2026 14:15:34 -0600 Subject: [PATCH 4/5] Trying to track down why the error message is getting lost --- lib/sparkql/parser_tools.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/sparkql/parser_tools.rb b/lib/sparkql/parser_tools.rb index b0ec9d4..07190d2 100644 --- a/lib/sparkql/parser_tools.rb +++ b/lib/sparkql/parser_tools.rb @@ -46,6 +46,7 @@ def no_field_error(field, operator) def tokenize_expression(field, op_token, val) operator = get_operator(val, op_token) unless val.nil? + Rails.logger.error("tokenize_expression #{field}, #{op_token}, #{val}, #{operator}") field_manipulations = nil if field.is_a?(Hash) && field[:type] == :function From 275b925b255bed78ebbda7631f1aefb4ed3a7b96 Mon Sep 17 00:00:00 2001 From: Reindl Date: Tue, 20 Jan 2026 14:37:38 -0600 Subject: [PATCH 5/5] Remove the logging --- lib/sparkql/parser_tools.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/sparkql/parser_tools.rb b/lib/sparkql/parser_tools.rb index 07190d2..b0ec9d4 100644 --- a/lib/sparkql/parser_tools.rb +++ b/lib/sparkql/parser_tools.rb @@ -46,7 +46,6 @@ def no_field_error(field, operator) def tokenize_expression(field, op_token, val) operator = get_operator(val, op_token) unless val.nil? - Rails.logger.error("tokenize_expression #{field}, #{op_token}, #{val}, #{operator}") field_manipulations = nil if field.is_a?(Hash) && field[:type] == :function