Prohibit parsing of non-ASCII digits (#413)

dkhalanskyjb · web-flow · commit da6f678d1d47 · 2024-07-24T11:48:34.000+02:00
Fixes #405
diff --git a/core/common/src/internal/format/FormatStructure.kt b/core/common/src/internal/format/FormatStructure.kt
@@ -7,6 +7,7 @@ package kotlinx.datetime.internal.format
 
 import kotlinx.datetime.internal.format.formatter.*
 import kotlinx.datetime.internal.format.parser.*
+import kotlinx.datetime.internal.isAsciiDigit
 
 internal sealed interface FormatStructure<in T> {
     fun parser(): ParserStructure<T>
@@ -37,16 +38,20 @@ internal class ConstantFormatStructure<in T>(
         operations = when {
             string.isEmpty() -> emptyList()
             else -> buildList {
-                val suffix = if (string[0].isDigit()) {
-                    add(NumberSpanParserOperation(listOf(ConstantNumberConsumer(string.takeWhile { it.isDigit() }))))
-                    string.dropWhile { it.isDigit() }
+                val suffix = if (string[0].isAsciiDigit()) {
+                    add(NumberSpanParserOperation(listOf(ConstantNumberConsumer(string.takeWhile {
+                        it.isAsciiDigit()
+                    }))))
+                    string.dropWhile { it.isAsciiDigit() }
                 } else {
                     string
                 }
                 if (suffix.isNotEmpty()) {
-                    if (suffix[suffix.length - 1].isDigit()) {
-                        add(PlainStringParserOperation(suffix.dropLastWhile { it.isDigit() }))
-                        add(NumberSpanParserOperation(listOf(ConstantNumberConsumer(suffix.takeLastWhile { it.isDigit() }))))
+                    if (suffix[suffix.length - 1].isAsciiDigit()) {
+                        add(PlainStringParserOperation(suffix.dropLastWhile { it.isAsciiDigit() }))
+                        add(NumberSpanParserOperation(listOf(ConstantNumberConsumer(suffix.takeLastWhile {
+                            it.isAsciiDigit()
+                        }))))
                     } else {
                         add(PlainStringParserOperation(suffix))
                     }
diff --git a/core/common/src/internal/format/parser/NumberConsumer.kt b/core/common/src/internal/format/parser/NumberConsumer.kt
@@ -5,8 +5,7 @@
 
 package kotlinx.datetime.internal.format.parser
 
-import kotlinx.datetime.internal.POWERS_OF_TEN
-import kotlinx.datetime.internal.DecimalFraction
+import kotlinx.datetime.internal.*
 
 /**
  * A parser that expects to receive a string consisting of [length] digits, or, if [length] is `null`,
@@ -19,13 +18,16 @@ internal sealed class NumberConsumer<in Receiver>(
     val whatThisExpects: String
 ) {
     /**
-     * Wholly consumes the given [input]. Should be called with a string consisting of [length] digits, or,
-     * if [length] is `null`, with a string consisting of any number of digits. [consume] itself does not
-     * necessarily check the length of the input string, instead expecting to be passed a valid one.
+     * Wholly consumes the substring of [input] between indices [start] (inclusive) and [end] (exclusive).
+     *
+     * If [length] is non-null, [end] must be equal to [start] + [length].
+     * In any case, the substring between [start] and [end] must consist of ASCII digits only.
+     * [consume] itself does not necessarily check the length of the input string,
+     * instead expecting to be given a valid one.
      *
      * Returns `null` on success and a `NumberConsumptionError` on failure.
      */
-    abstract fun consume(storage: Receiver, input: String): NumberConsumptionError?
+    abstract fun consume(storage: Receiver, input: CharSequence, start: Int, end: Int): NumberConsumptionError?
 }
 
 internal interface NumberConsumptionError {
@@ -50,7 +52,6 @@ internal interface NumberConsumptionError {
 /**
  * A parser that accepts an [Int] value in range from `0` to [Int.MAX_VALUE].
  */
-// TODO: should the parser reject excessive padding?
 internal class UnsignedIntConsumer<in Receiver>(
     private val minLength: Int?,
     private val maxLength: Int?,
@@ -63,10 +64,10 @@ internal class UnsignedIntConsumer<in Receiver>(
         require(length == null || length in 1..9) { "Invalid length for field $whatThisExpects: $length" }
     }
 
-    override fun consume(storage: Receiver, input: String): NumberConsumptionError? = when {
-        maxLength != null && input.length > maxLength -> NumberConsumptionError.TooManyDigits(maxLength)
-        minLength != null && input.length < minLength -> NumberConsumptionError.TooFewDigits(minLength)
-        else -> when (val result = input.toIntOrNull()) {
+    override fun consume(storage: Receiver, input: CharSequence, start: Int, end: Int): NumberConsumptionError? = when {
+        maxLength != null && end - start > maxLength -> NumberConsumptionError.TooManyDigits(maxLength)
+        minLength != null && end - start < minLength -> NumberConsumptionError.TooFewDigits(minLength)
+        else -> when (val result = input.parseAsciiIntOrNull(start = start, end = end)) {
             null -> NumberConsumptionError.ExpectedInt
             else -> setter.setWithoutReassigning(storage, if (multiplyByMinus1) -result else result)
         }
@@ -84,9 +85,13 @@ internal class ReducedIntConsumer<in Receiver>(
     private val baseMod = base % modulo
     private val baseFloor = base - baseMod
 
-    override fun consume(storage: Receiver, input: String): NumberConsumptionError? = when (val result = input.toIntOrNull()) {
-        null -> NumberConsumptionError.ExpectedInt
-        else -> setter.setWithoutReassigning(storage, if (result >= baseMod) {
+    init {
+        require(length in 1..9) { "Invalid length for field $whatThisExpects: $length" }
+    }
+
+    override fun consume(storage: Receiver, input: CharSequence, start: Int, end: Int): NumberConsumptionError? {
+        val result = input.parseAsciiInt(start = start, end = end)
+        return setter.setWithoutReassigning(storage, if (result >= baseMod) {
             baseFloor + result
         } else {
             baseFloor + modulo + result
@@ -100,31 +105,35 @@ internal class ReducedIntConsumer<in Receiver>(
 internal class ConstantNumberConsumer<in Receiver>(
     private val expected: String
 ) : NumberConsumer<Receiver>(expected.length, "the predefined string $expected") {
-    override fun consume(storage: Receiver, input: String): NumberConsumptionError? = if (input == expected) {
-        null
-    } else {
-        NumberConsumptionError.WrongConstant(expected)
-    }
+    override fun consume(storage: Receiver, input: CharSequence, start: Int, end: Int): NumberConsumptionError? =
+        if (input.substring(startIndex = start, endIndex = end) == expected) {
+            null
+        } else {
+            NumberConsumptionError.WrongConstant(expected)
+        }
 }
 
 internal class FractionPartConsumer<in Receiver>(
-    private val minLength: Int?,
-    private val maxLength: Int?,
+    private val minLength: Int,
+    private val maxLength: Int,
     private val setter: AssignableField<Receiver, DecimalFraction>,
     name: String,
 ) : NumberConsumer<Receiver>(if (minLength == maxLength) minLength else null, name) {
     init {
-        require(minLength == null || minLength in 1..9) { "Invalid length for field $whatThisExpects: $length" }
-        // TODO: bounds on maxLength
+        require(minLength in 1..9) {
+            "Invalid minimum length $minLength for field $whatThisExpects: expected 1..9"
+        }
+        require(maxLength in minLength..9) {
+            "Invalid maximum length $maxLength for field $whatThisExpects: expected $minLength..9"
+        }
     }
 
-    override fun consume(storage: Receiver, input: String): NumberConsumptionError? = when {
-        minLength != null && input.length < minLength -> NumberConsumptionError.TooFewDigits(minLength)
-        maxLength != null && input.length > maxLength -> NumberConsumptionError.TooManyDigits(maxLength)
-        else -> when (val numerator = input.toIntOrNull()) {
-            null -> NumberConsumptionError.TooManyDigits(9)
-            else -> setter.setWithoutReassigning(storage, DecimalFraction(numerator, input.length))
-        }
+    override fun consume(storage: Receiver, input: CharSequence, start: Int, end: Int): NumberConsumptionError? = when {
+        end - start < minLength -> NumberConsumptionError.TooFewDigits(minLength)
+        end - start > maxLength -> NumberConsumptionError.TooManyDigits(maxLength)
+        else -> setter.setWithoutReassigning(
+            storage, DecimalFraction(input.parseAsciiInt(start = start, end = end), end - start)
+        )
     }
 }
 
@@ -135,3 +144,36 @@ private fun <Object, Type> AssignableField<Object, Type>.setWithoutReassigning(
     val conflictingValue = trySetWithoutReassigning(receiver, value) ?: return null
     return NumberConsumptionError.Conflicting(conflictingValue)
 }
+
+/**
+ * Parses a substring of the receiver string as a positive ASCII integer.
+ *
+ * All characters between [start] (inclusive) and [end] (exclusive) must be ASCII digits,
+ * and the size of the substring must be at most 9, but the function does not check it.
+ */
+private fun CharSequence.parseAsciiInt(start: Int, end: Int): Int {
+    var result = 0
+    for (i in start until end) {
+        val digit = this[i]
+        result = result * 10 + digit.asciiDigitToInt()
+    }
+    return result
+}
+
+/**
+ * Parses a substring of the receiver string as a positive ASCII integer.
+ *
+ * All characters between [start] (inclusive) and [end] (exclusive) must be ASCII digits,
+ * but the function does not check it.
+ *
+ * Returns `null` if the result does not fit into a positive [Int].
+ */
+private fun CharSequence.parseAsciiIntOrNull(start: Int, end: Int): Int? {
+    var result = 0
+    for (i in start until end) {
+        val digit = this[i]
+        result = result * 10 + digit.asciiDigitToInt()
+        if (result < 0) return null
+    }
+    return result
+}
diff --git a/core/common/src/internal/format/parser/ParserOperation.kt b/core/common/src/internal/format/parser/ParserOperation.kt
@@ -5,6 +5,8 @@
 
 package kotlinx.datetime.internal.format.parser
 
+import kotlinx.datetime.internal.isAsciiDigit
+
 internal interface ParserOperation<in Output> {
     fun consume(storage: Output, input: CharSequence, startIndex: Int): ParseResult
 }
@@ -15,8 +17,8 @@ internal interface ParserOperation<in Output> {
 internal class PlainStringParserOperation<Output>(val string: String) : ParserOperation<Output> {
     init {
         require(string.isNotEmpty()) { "Empty string is not allowed" }
-        require(!string[0].isDigit()) { "String '$string' starts with a digit" }
-        require(!string[string.length - 1].isDigit()) { "String '$string' ends with a digit" }
+        require(!string[0].isAsciiDigit()) { "String '$string' starts with a digit" }
+        require(!string[string.length - 1].isAsciiDigit()) { "String '$string' ends with a digit" }
     }
 
     override fun consume(storage: Output, input: CharSequence, startIndex: Int): ParseResult {
@@ -77,7 +79,7 @@ internal class NumberSpanParserOperation<Output>(
         if (startIndex + minLength > input.length)
             return ParseResult.Error(startIndex) { "Unexpected end of input: yet to parse $whatThisExpects" }
         var digitsInRow = 0
-        while (startIndex + digitsInRow < input.length && input[startIndex + digitsInRow].isDigit()) {
+        while (startIndex + digitsInRow < input.length && input[startIndex + digitsInRow].isAsciiDigit()) {
             ++digitsInRow
         }
         if (digitsInRow < minLength)
@@ -87,9 +89,9 @@ internal class NumberSpanParserOperation<Output>(
         var index = startIndex
         for (i in consumers.indices) {
             val length = consumers[i].length ?: (digitsInRow - minLength + 1)
-            val numberString = input.substring(index, index + length)
-            val error = consumers[i].consume(storage, numberString)
+            val error = consumers[i].consume(storage, input, index, index + length)
             if (error != null) {
+                val numberString = input.substring(index, index + length)
                 return ParseResult.Error(index) {
                     "Can not interpret the string '$numberString' as ${consumers[i].whatThisExpects}: ${error.errorMessage()}"
                 }
diff --git a/core/common/src/internal/util.kt b/core/common/src/internal/util.kt
@@ -0,0 +1,10 @@
+/*
+ * Copyright 2019-2024 JetBrains s.r.o. and contributors.
+ * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
+ */
+
+package kotlinx.datetime.internal
+
+internal fun Char.isAsciiDigit(): Boolean = this in '0'..'9'
+
+internal fun Char.asciiDigitToInt(): Int = this - '0'
diff --git a/core/common/test/format/DateTimeFormatTest.kt b/core/common/test/format/DateTimeFormatTest.kt
@@ -149,6 +149,37 @@ class DateTimeFormatTest {
         }
         assertEquals(UtcOffset(-7, -30), format.parse("-730"))
     }
+
+    @Test
+    fun testNotParsingNonAsciiNumbers() {
+        val formatWithFraction = DateTimeComponents.Format {
+            secondFraction(3)
+        }
+        formatWithFraction.parse("999")
+        assertFailsWith<IllegalArgumentException> {
+            formatWithFraction.parse("٩٩٩")
+        }
+        val formatWithArbitraryWidthNumber = DateTimeComponents.Format {
+            year()
+        }
+        formatWithArbitraryWidthNumber.parse("+99999")
+        assertFailsWith<IllegalArgumentException> {
+            formatWithArbitraryWidthNumber.parse("+٩٩٩٩٩")
+        }
+        val formatWithFixedWidthNumber = DateTimeComponents.Format {
+            monthNumber()
+        }
+        formatWithFixedWidthNumber.parse("99")
+        assertFailsWith<IllegalArgumentException> {
+            formatWithFixedWidthNumber.parse("٩٩")
+        }
+        val formatWithNonAsciiNumberInString = DateTimeComponents.Format {
+            chars("99")
+            chars("٩٩")
+            chars("99")
+        }
+        formatWithNonAsciiNumberInString.parse("99٩٩99")
+    }
 }
 
 fun <T> DateTimeFormat<T>.assertCanNotParse(input: String) {
