From 19e05115f27f4fd22d9c384de2200a4c3132437a Mon Sep 17 00:00:00 2001
From: Jack Weeden <jack@ajack.org>
Date: Mon, 23 Dec 2024 11:09:28 +0000
Subject: [PATCH] Fix date parsing with complex types

The `DateHashParser` class is responsible for taking a hash containing
keys of some combination of `day`, `month` and `year` and trying to
return a full `Date` object.

It is flexible enough to handle hashes where certain keys are missing,
e.g. if the `year` key isn't present, it sets the value to be the
current year.

We've seen quite a few exceptions coming through from this class where
it looks like somebody has been messing about with the query parameters
in the URL, which are combined into a hash and sent through to this
class for parsing.

A (truncated) example request is:

```
/search/all?keywords=the&public_timestamp[from][day][]=17&public_timestamp[from][month]=7&public_timestamp[from][year]=1967
```

Here we can see that `public_timestamp[from][day]` is an array value
rather than a numeric value like `month` and `year`.

This doesn't appear to be a legitimate bug caused by the frontend.
Looking through the Sentry logs it's pretty clear somebody is
manipulating the URL for some reason.

So it makes sense to fix the bug in this class by flagging a date hash
as invalid if any of the values don't respond to `to_i`. We're always
expecting these values to be numeric so this feels like a valid
assumption to make.

The exception is raised in the constructor currently, where we call
`transform_values(&:to_i)`, which blows up if a value is, for example,
an array.

Instead we'll extend the `any_non_positive_values?` method to check that
the values both respond to `to_i` and are positive.
---
 app/parsers/date_hash_parser.rb       | 9 ++++-----
 spec/parsers/date_hash_parser_spec.rb | 6 ++++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/app/parsers/date_hash_parser.rb b/app/parsers/date_hash_parser.rb
index 3cf2c14bb..64a3f9644 100644
--- a/app/parsers/date_hash_parser.rb
+++ b/app/parsers/date_hash_parser.rb
@@ -21,7 +21,6 @@ def initialize(date_hash)
     @date_hash = date_hash
       .slice(:day, :month, :year)
       .compact_blank
-      .transform_values(&:to_i)
   end
 
   def parse
@@ -43,19 +42,19 @@ def day_without_month?
   def any_non_positive_values?
     # If the user enters a zero or negative value, or a string that `#to_i` converts to zero, we
     # consider the whole date invalid
-    !date_hash.values.all?(&:positive?)
+    !date_hash.values.all? { |value| value.respond_to?(:to_i) && value.to_i.positive? }
   end
 
   def day
-    date_hash.fetch(:day, 1)
+    date_hash.fetch(:day, 1).to_i
   end
 
   def month
-    date_hash.fetch(:month, 1)
+    date_hash.fetch(:month, 1).to_i
   end
 
   def year
-    value = date_hash.fetch(:year, Date.current.year)
+    value = date_hash.fetch(:year, Date.current.year).to_i
 
     case value
     when 0..MILLENNIUM_CUTOFF
diff --git a/spec/parsers/date_hash_parser_spec.rb b/spec/parsers/date_hash_parser_spec.rb
index 43526a924..4d98c7697 100644
--- a/spec/parsers/date_hash_parser_spec.rb
+++ b/spec/parsers/date_hash_parser_spec.rb
@@ -105,6 +105,12 @@
         it { is_expected.to be_nil }
       end
 
+      context "with hash with a complex type value" do
+        let(:date_hash) { { day: [1], month: 12, year: 2023 } }
+
+        it { is_expected.to be_nil }
+      end
+
       context "with hash with non-numeric year only" do
         let(:date_hash) { { year: "baz" } }