Prohibit parsing of non-ASCII digits

dkhalanskyjb · dkhalanskyjb · commit 57a66fe27487 · 2024-07-22T15:39:06.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`,
@@ -50,7 +49,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?,
@@ -66,7 +64,7 @@ internal class UnsignedIntConsumer<in Receiver>(
     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()) {
+        else -> when (val result = input.parseAsciiIntOrNull()) {
             null -> NumberConsumptionError.ExpectedInt
             else -> setter.setWithoutReassigning(storage, if (multiplyByMinus1) -result else result)
         }
@@ -84,9 +82,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: String): NumberConsumptionError? {
+        val result = input.parseAsciiInt()
+        return setter.setWithoutReassigning(storage, if (result >= baseMod) {
             baseFloor + result
         } else {
             baseFloor + modulo + result
@@ -108,23 +110,24 @@ internal class ConstantNumberConsumer<in Receiver>(
 }
 
 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))
-        }
+        input.length < minLength -> NumberConsumptionError.TooFewDigits(minLength)
+        input.length > maxLength -> NumberConsumptionError.TooManyDigits(maxLength)
+        else -> setter.setWithoutReassigning(storage, DecimalFraction(input.parseAsciiInt(), input.length))
     }
 }
 
@@ -135,3 +138,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 String.parseAsciiInt(start: Int = 0, end: Int = length): 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 String.parseAsciiIntOrNull(start: Int = 0, end: Int = length): 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)
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'
