Support decimal values in cacheDuration parsing

Since the introduction of cacheDuration parsing in 25cbddd we've been
seeing parsing failures for one of our IdPs, whose cacheDuration value
is set to

    cacheDuration="PT6H0M0.000S"

This is a valid ISO8601 duration, however the regexp being used for
parsing doesn't provide for the possibility of milliseconds.

The ISO8601 spec[1] supports decimal values for at least some of a
duration's components (S4.4.3.2):

  > If necessary for a particular application, the lowest order
  > components may have a decimal fraction.

It's not entirely plain which components this might refer to; it could
potentially be any of them (i.e. whichever component happens to be the
smallest in a given duration, which would mean P0.5Y would be valid).

However, applying decimal logic to the nominal components (Y/M/D)
isn't possible in our current implementation, as the DateTime#next_year,
next_month and next_day methods that we're using to construct the
new timestamp only respect integer values. If we parsed decimals but
then ignored them, someone setting a duration of P0.5Y would silently
receive a duration of zero.

Even if we were to try and adjust our implementation to support
fractions in the nominal components, given that all nominal values can
vary in length (months have different lengths, years can be leap years,
days can have leap seconds), the question of what a fractional nominal
component actually means is ill-defined.

This commit therefore adds fractional support to just the H/M/S
components of the duration parsing. To ensure that we actually respect
these values, I've updated our parsed value coercion to always produce
floats.

The spec permits arbitrary decimal precision, and supports both commas
and dots as the decimal separator, which is reflected in the
implementation.

[1] https://www.loc.gov/standards/datetime/iso-tc154-wg5_n0038_iso_wd_8601-1_2016-02-16.pdf

Author: Simon Coffey

lib/onelogin/ruby-saml/utils.rb

@@ -16,18 +16,18 @@ class Utils
       DSIG      = "http://www.w3.org/2000/09/xmldsig#"
       XENC      = "http://www.w3.org/2001/04/xmlenc#"
       DURATION_FORMAT = %r(^
-        (-?)P                         # 1: Duration sign
+        (-?)P                       # 1: Duration sign
         (?:
-          (?:(\d+)Y)?                 # 2: Years
-          (?:(\d+)M)?                 # 3: Months
-          (?:(\d+)D)?                 # 4: Days
+          (?:(\d+)Y)?               # 2: Years
+          (?:(\d+)M)?               # 3: Months
+          (?:(\d+)D)?               # 4: Days
           (?:T
-            (?:(\d+)H)?               # 5: Hours
-            (?:(\d+)M)?               # 6: Minutes
-            (?:(\d+)S)?   # 7: Seconds and optional milliseconds
+            (?:(\d+(?:[.,]\d+)?)H)? # 5: Hours
+            (?:(\d+(?:[.,]\d+)?)M)? # 6: Minutes
+            (?:(\d+(?:[.,]\d+)?)S)? # 7: Seconds
           )?
           |
-          (\d+)W                      # 8: Weeks
+          (\d+)W                    # 8: Weeks
         )
       $)x
 
@@ -59,13 +59,8 @@ def self.parse_duration(duration, timestamp=Time.now.utc)
           raise Exception.new("Invalid ISO 8601 duration")
         end
 
-        durYears = matches[2].to_i
-        durMonths = matches[3].to_i
-        durDays = matches[4].to_i
-        durHours = matches[5].to_i
-        durMinutes = matches[6].to_i
-        durSeconds = matches[7].to_f
-        durWeeks = matches[8].to_i
+        durYears, durMonths, durDays, durHours, durMinutes, durSeconds, durWeeks =
+          matches[2..8].map { |match| match ? match.tr(',', '.').to_f : 0.0 }
 
         if matches[1] == "-"
           durYears = -durYears

test/utils_test.rb

@@ -12,6 +12,14 @@ class UtilsTest < Minitest::Test
       # Nominal wraparounds
       "P13M" => "1971-02-01T00:00:00.000Z",
       "P31D" => "1970-02-01T00:00:00.000Z",
+
+      # Decimal values
+      "PT0.5H" => "1970-01-01T00:30:00.000Z",
+      "PT0.5M" => "1970-01-01T00:00:30.000Z",
+      "PT0.5S" => "1970-01-01T00:00:00.500Z",
+
+      # Comma decimal separator
+      "PT0,5S" => "1970-01-01T00:00:00.500Z"
     }
 
     def result(duration, reference = 0)