Add rawFractionalSeconds property to GeneralizedTime (#53)

Motivation:

There is a possible overflow when computing a `Double` value for `fractionalSeconds` from an `ArraySlice` of bytes. Hence, access to this original `ArraySlice` is necessary.

Modifications:

- Add `rawFractionalSeconds` property to `GeneralizedTime`.
- Add a new `internal init` to `GeneralizedTime` that accepts `rawFractionalSeconds` instead of `fractionalSeconds` and generates the later from the former.
- Adjust the algorithm for computing `fractionalSeconds`, from a mathematical to a `String` computation. This is due to the precision mismatch between the mathematical computation and the native `Double` computation. This issue, which previously did not surface, now does, due to the newly included round-trip conversion from `fractionalSeconds` to `rawFractionalSeconds`.

Result:

- The `rawFractionalSeconds` property now provides the original `ArraySlice` from which `fractionalSeconds` was computed.
- `GeneralizedTime` can now compute `fractionalSeconds` from the provided `rawFractionalSeconds`.

Sources/SwiftASN1/Basic ASN1 Types/GeneralizedTime.swift

@@ -105,6 +105,24 @@ public struct GeneralizedTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Has
         }
         set {
             self._fractionalSeconds = newValue
+            self._rawFractionalSeconds = ArraySlice<UInt8>()
+            try! self._rawFractionalSeconds.append(fractionalSeconds: self._fractionalSeconds)
+
+            try! self._validate()
+        }
+    }
+
+    /// The ArraySlice of bytes from which the fractional seconds will be computed. (Preserved due to a possible overflow
+    /// when computing a Double from this ArraySlice.)
+    @inlinable
+    public var rawFractionalSeconds: ArraySlice<UInt8> {
+        get {
+            return self._rawFractionalSeconds
+        }
+        set {
+            self._rawFractionalSeconds = newValue
+            self._fractionalSeconds = try! Double(fromRawFractionalSeconds: self._rawFractionalSeconds)
+
             try! self._validate()
         }
     }
@@ -115,7 +133,10 @@ public struct GeneralizedTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Has
     @usableFromInline var _hours: Int
     @usableFromInline var _minutes: Int
     @usableFromInline var _seconds: Int
+    /// `_fractionalSeconds` is a cached value and `_rawFractionalSeconds` is the source of truth for the numerical
+    /// fractonal seconds. (No information is lost in the conversion from `_fractionalSeconds` to `_rawFractionalSeconds`.)
     @usableFromInline var _fractionalSeconds: Double
+    @usableFromInline var _rawFractionalSeconds: ArraySlice<UInt8>
 
     /// Construct a new ``GeneralizedTime`` from individual components.
     ///
@@ -144,6 +165,41 @@ public struct GeneralizedTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Has
         self._minutes = minutes
         self._seconds = seconds
         self._fractionalSeconds = fractionalSeconds
+        self._rawFractionalSeconds = ArraySlice<UInt8>()
+        try self._rawFractionalSeconds.append(fractionalSeconds: self._fractionalSeconds)
+
+        try self._validate()
+    }
+
+    /// Construct a new ``GeneralizedTime`` from individual components.
+    ///
+    /// - parameters:
+    ///     - year: The numerical year
+    ///     - month: The numerical month
+    ///     - day: The numerical day
+    ///     - hours: The numerical hours
+    ///     - minutes: The numerical minutes
+    ///     - seconds: The numerical seconds
+    ///     - rawFractionalSeconds: The ArraySlice of bytes from which the fractional seconds will be computed.
+    ///     (Preserved due to a possible overflow when computing a Double from this ArraySlice.)
+    @inlinable
+    public init(
+        year: Int,
+        month: Int,
+        day: Int,
+        hours: Int,
+        minutes: Int,
+        seconds: Int,
+        rawFractionalSeconds: ArraySlice<UInt8>
+    ) throws {
+        self._year = year
+        self._month = month
+        self._day = day
+        self._hours = hours
+        self._minutes = minutes
+        self._seconds = seconds
+        self._rawFractionalSeconds = rawFractionalSeconds
+        self._fractionalSeconds = try Double(fromRawFractionalSeconds: self._rawFractionalSeconds)
 
         try self._validate()
     }
@@ -216,6 +272,11 @@ public struct GeneralizedTime: DERImplicitlyTaggable, BERImplicitlyTaggable, Has
                 reason: "Invalid fractional seconds for GeneralizedTime \(self._fractionalSeconds)"
             )
         }
+
+        // When `rawFractionalSeconds` is converted to a `Double`, it must be equal to `fractionalSeconds`.
+        assert(
+            (try? Double(fromRawFractionalSeconds: self._rawFractionalSeconds)) == self._fractionalSeconds
+        )
     }
 }
 
@@ -228,6 +289,20 @@ extension GeneralizedTime: Comparable {
         if lhs.hours < rhs.hours { return true } else if lhs.hours > rhs.hours { return false }
         if lhs.minutes < rhs.minutes { return true } else if lhs.minutes > rhs.minutes { return false }
         if lhs.seconds < rhs.seconds { return true } else if lhs.seconds > rhs.seconds { return false }
-        return lhs.fractionalSeconds < rhs.fractionalSeconds
+        if lhs.fractionalSeconds < rhs.fractionalSeconds {
+            return true
+        } else if lhs.fractionalSeconds > rhs.fractionalSeconds {
+            return false
+        }
+
+        for (lhsByte, rhsByte) in zip(lhs.rawFractionalSeconds, rhs.rawFractionalSeconds) {
+            if lhsByte != rhsByte {
+                return lhsByte < rhsByte
+            }
+        }
+
+        // Since the above `zip` iteration stops at the length of the shorter `Sequence`, finally,
+        // compare the length of the two `Sequence`s.
+        return lhs.rawFractionalSeconds.count < rhs.rawFractionalSeconds.count
     }
 }

Sources/SwiftASN1/Basic ASN1 Types/TimeUtilities.swift

@@ -38,9 +38,10 @@ enum TimeUtilities {
         }
 
         // There may be some fractional seconds.
-        var fractionalSeconds: Double = 0
+        var rawFractionalSeconds = ArraySlice<UInt8>()
         if bytes.first == UInt8(ascii: ".") {
-            fractionalSeconds = try bytes._readFractionalSeconds()
+            bytes.removeFirst()
+            rawFractionalSeconds = try bytes._readRawFractionalSeconds()
         }
 
         // The next character _must_ be Z, or the encoding is invalid.
@@ -60,7 +61,7 @@ enum TimeUtilities {
             hours: rawHour,
             minutes: rawMinutes,
             seconds: rawSeconds,
-            fractionalSeconds: fractionalSeconds
+            rawFractionalSeconds: rawFractionalSeconds
         )
     }
 
@@ -178,41 +179,44 @@ extension ArraySlice where Element == UInt8 {
         return (first &* 10) &+ (second)
     }
 
-    /// This may only be called if there's a leading period: we precondition on this fact.
     @inlinable
-    mutating func _readFractionalSeconds() throws -> Double {
-        precondition(self.popFirst() == UInt8(ascii: "."))
-
-        var numerator = 0
-        var denominator = 1
+    mutating func _readRawFractionalSeconds() throws -> ArraySlice<UInt8> {
+        guard let nonDecimalASCIIIndex = self.firstIndex(where: { Int(fromDecimalASCII: $0) == nil }) else {
+            throw ASN1Error.invalidASN1Object(
+                reason: "Invalid fractional seconds"
+            )
+        }
 
-        while let nextASCII = self.first, let next = Int(fromDecimalASCII: nextASCII) {
-            self = self.dropFirst()
+        // If `nonDecimalASCIIIndex == self.startIndex`, then it means that there is a decimal point
+        // but there are no fractional seconds
+        if nonDecimalASCIIIndex == self.startIndex {
+            throw ASN1Error.invalidASN1Object(
+                reason: "Invalid fractional seconds"
+            )
+        }
 
-            let (newNumerator, multiplyOverflow) = numerator.multipliedReportingOverflow(by: 10)
-            let (newDenominator, secondMultiplyOverflow) = denominator.multipliedReportingOverflow(by: 10)
-            let (newNumeratorWithAdded, addingOverflow) = newNumerator.addingReportingOverflow(next)
+        let rawFractionalSeconds = self[..<nonDecimalASCIIIndex]
+        self = self[nonDecimalASCIIIndex...]
+        return rawFractionalSeconds
+    }
 
-            // If the new denominator overflows, we just cap to the old value.
-            if !secondMultiplyOverflow {
-                denominator = newDenominator
-            }
+    @inlinable
+    mutating func append(fractionalSeconds: Double) throws {
+        // Fractional seconds may not be negative and may not be 1 or more.
+        guard fractionalSeconds >= 0 && fractionalSeconds < 1 else {
+            throw ASN1Error.invalidASN1Object(
+                reason: "Invalid fractional seconds: \(fractionalSeconds)"
+            )
+        }
 
-            // If the numerator overflows, we don't support the result.
-            if multiplyOverflow || addingOverflow {
-                throw ASN1Error.invalidASN1Object(reason: "Numerator overflow when calculating fractional seconds")
-            }
+        if fractionalSeconds != 0 {
+            let fractionalSecondsAsString = String(fractionalSeconds)
 
-            numerator = newNumeratorWithAdded
-        }
+            assert(fractionalSecondsAsString.starts(with: "0."), "Invalid fractional seconds")
+            assert(fractionalSecondsAsString.last != "0", "Trailing zeros in fractional seconds")
 
-        // Ok, we're either at the end or the next character is a Z. One final check: there may not have
-        // been any trailing zeros here. This means the number may not be 0 mod 10.
-        if numerator % 10 == 0 {
-            throw ASN1Error.invalidASN1Object(reason: "Trailing zeros in fractional seconds")
+            self.append(contentsOf: fractionalSecondsAsString.utf8.dropFirst(2))
         }
-
-        return Double(numerator) / Double(denominator)
     }
 }
 
@@ -226,16 +230,9 @@ extension Array where Element == UInt8 {
         self._appendTwoDigitDecimal(generalizedTime.minutes)
         self._appendTwoDigitDecimal(generalizedTime.seconds)
 
-        // Ok, tricky moment here. Is the fractional part non-zero? If it is, we need to write it out as well.
-        if generalizedTime.fractionalSeconds != 0 {
-            let stringified = String(generalizedTime.fractionalSeconds)
-            assert(stringified.starts(with: "0."))
-
-            self.append(contentsOf: stringified.utf8.dropFirst(1))
-            // Remove any trailing zeros from self, they are forbidden.
-            while self.last == 0 {
-                self = self.dropLast()
-            }
+        if generalizedTime.rawFractionalSeconds.count > 0 {
+            self.append(UInt8(ascii: "."))
+            self.append(contentsOf: generalizedTime.rawFractionalSeconds)
         }
 
         self.append(UInt8(ascii: "Z"))
@@ -300,3 +297,28 @@ extension Int {
         self = converted
     }
 }
+
+extension Double {
+    @inlinable
+    init(fromRawFractionalSeconds rawFractionalSeconds: ArraySlice<UInt8>) throws {
+        if rawFractionalSeconds.count == 0 {
+            self = 0
+            return
+        }
+
+        if rawFractionalSeconds.last == UInt8(ascii: "0") {
+            throw ASN1Error.invalidASN1Object(reason: "Trailing zeros in raw fractional seconds")
+        }
+
+        let rawFractionalSecondsAsString = String(decoding: rawFractionalSeconds, as: UTF8.self)
+        let fractionalSecondsAsString = "0.\(rawFractionalSecondsAsString)"
+
+        guard let fractionalSeconds = Double(fractionalSecondsAsString) else {
+            throw ASN1Error.invalidASN1Object(
+                reason: "Invalid raw fractional seconds"
+            )
+        }
+
+        self = fractionalSeconds
+    }
+}