Regression: Clientside length validation in ColumnType breaks otherwi…

…se harmless where clause #1204 select query throws "can't be stored to database column because exceeds length" #1222
JetBrains · May 9, 2021 · 9ad54b4 · 9ad54b4
1 parent b373121
commit 9ad54b4
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 14 deletions.
diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt
@@ -8,6 +8,7 @@ import org.jetbrains.exposed.sql.statements.api.ExposedBlob
 import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi
 import org.jetbrains.exposed.sql.vendors.currentDialect
 import java.io.InputStream
+import java.lang.IllegalArgumentException
 import java.math.BigDecimal
 import java.math.RoundingMode
 import java.nio.ByteBuffer
@@ -66,6 +67,13 @@ interface IColumnType {
         else
             stmt[index] = value
     }
+
+    /**
+     * Function checks that provided value suites the column type and throws [IllegalArgumentException] otherwise.
+     * [value] can be of any type (including [Expression])
+     * */
+    @Throws(IllegalArgumentException::class)
+    fun validateValueBeforeUpdate(value: Any?) {}
 }
 
 /**
@@ -512,11 +520,12 @@ open class CharColumnType(
         }
     }
 
-    override fun notNullValueToDB(value: Any): Any {
-        require(value is String && value.codePointCount(0, value.length) <= colLength) {
-            "Value '$value' can't be stored to database column because exceeds length ($colLength)"
+    override fun validateValueBeforeUpdate(value: Any?) {
+        if (value is String) {
+            require(value.codePointCount(0, value.length) <= colLength) {
+                "Value '$value' can't be stored to database column because exceeds length ($colLength)"
+            }
         }
-        return value
     }
 
     override fun equals(other: Any?): Boolean {
@@ -554,11 +563,12 @@ open class VarCharColumnType(
         }
     }
 
-    override fun notNullValueToDB(value: Any): Any {
-        require(value is String && value.codePointCount(0, value.length) <= colLength) {
-            "Value '$value' can't be stored to database column because exceeds length ($colLength)"
+    override fun validateValueBeforeUpdate(value: Any?) {
+        if (value is String) {
+            require(value.codePointCount(0, value.length) <= colLength) {
+                "Value '$value' can't be stored to database column because exceeds length ($colLength)"
+            }
         }
-        return value
     }
 
     override fun equals(other: Any?): Boolean {
@@ -632,11 +642,12 @@ class BinaryColumnType(
 ) : BasicBinaryColumnType() {
     override fun sqlType(): String = currentDialect.dataTypeProvider.binaryType(length)
 
-    override fun notNullValueToDB(value: Any): Any {
-        require(value is ByteArray && value.size <= length) {
-            "Value '$value' can't be stored to database column because exceeds length ($length)"
+    override fun validateValueBeforeUpdate(value: Any?) {
+        if (value is ByteArray) {
+            require(value.size <= length) {
+                "Value '$value' can't be stored to database column because exceeds length ($length)"
+            }
         }
-        return value
     }
 
     override fun equals(other: Any?): Boolean {

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateBuilder.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateBuilder.kt
@@ -16,19 +16,24 @@ abstract class UpdateBuilder<out T>(type: StatementType, targets: List<Table>) :
         when {
             values.containsKey(column) -> error("$column is already initialized")
             !column.columnType.nullable && value == null -> error("Trying to set null to not nullable column $column")
-            else -> values[column] = value
+            else -> {
+                column.columnType.validateValueBeforeUpdate(value)
+                values[column] = value
+            }
         }
     }
 
     @JvmName("setWithEntityIdExpression")
     operator fun <S, ID : EntityID<S>, E : Expression<S>> set(column: Column<ID>, value: E) {
         require(!values.containsKey(column)) { "$column is already initialized" }
+        column.columnType.validateValueBeforeUpdate(value)
         values[column] = value
     }
 
     @JvmName("setWithEntityIdValue")
     operator fun <S : Comparable<S>, ID : EntityID<S>, E : S?> set(column: Column<ID>, value: E) {
         require(!values.containsKey(column)) { "$column is already initialized" }
+        column.columnType.validateValueBeforeUpdate(value)
         values[column] = value
     }
 
@@ -40,11 +45,14 @@ abstract class UpdateBuilder<out T>(type: StatementType, targets: List<Table>) :
 
     open fun <T, S : T?> update(column: Column<T>, value: Expression<S>) {
         require(!values.containsKey(column)) { "$column is already initialized" }
+        column.columnType.validateValueBeforeUpdate(value)
         values[column] = value
     }
 
     open fun <T, S : T?> update(column: Column<T>, value: SqlExpressionBuilder.() -> Expression<S>) {
         require(!values.containsKey(column)) { "$column is already initialized" }
-        values[column] = SqlExpressionBuilder.value()
+        val expression = SqlExpressionBuilder.value()
+        column.columnType.validateValueBeforeUpdate(expression)
+        values[column] = expression
     }
 }
diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt
@@ -351,6 +351,20 @@ class InsertTests : DatabaseTestsBase() {
         }
     }
 
+    @Test(expected = java.lang.IllegalArgumentException::class)
+    fun `test that column length checked on insert`() {
+        val stringTable = object : IntIdTable("StringTable") {
+            val name = varchar("name", 10)
+        }
+
+        withTables(stringTable) {
+            val veryLongString = "1".repeat(255)
+            stringTable.insert {
+                it[name] = veryLongString
+            }
+        }
+    }
+
     /*
     @Test fun testGeneratedKey04() {
         val CharIdTable = object : IdTable<String>("charId") {

diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/SelectTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/SelectTests.kt
@@ -238,4 +238,21 @@ class SelectTests : DatabaseTestsBase() {
             assertEquals(1, secondEntries.size)
         }
     }
+
+    @Test
+    fun `test that column length check is not affects select queries`() {
+        val stringTable = object : IntIdTable("StringTable") {
+            val name = varchar("name", 10)
+        }
+
+        withTables(stringTable) {
+            stringTable.insert {
+                it[name] = "TestName"
+            }
+            assertEquals(1, stringTable.select { stringTable.name eq "TestName" }.count())
+
+            val veryLongString = "1".repeat(255)
+            assertEquals(0, stringTable.select { stringTable.name eq veryLongString }.count())
+        }
+    }
 }
diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt
@@ -1,5 +1,6 @@
 package org.jetbrains.exposed.sql.tests.shared.dml
 
+import org.jetbrains.exposed.dao.id.IntIdTable
 import org.jetbrains.exposed.exceptions.UnsupportedByDialectException
 import org.jetbrains.exposed.sql.*
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
@@ -8,6 +9,7 @@ import org.jetbrains.exposed.sql.tests.shared.assertEquals
 import org.jetbrains.exposed.sql.tests.shared.expectException
 import org.jetbrains.exposed.sql.vendors.SQLiteDialect
 import org.junit.Test
+import java.lang.IllegalArgumentException
 
 class UpdateTests : DatabaseTestsBase() {
     private val notSupportLimit by lazy {
@@ -80,4 +82,22 @@ class UpdateTests : DatabaseTestsBase() {
             }
         }
     }
+
+    @Test(expected = IllegalArgumentException::class)
+    fun `test that column length checked in update `() {
+        val stringTable = object : IntIdTable("StringTable") {
+            val name = varchar("name", 10)
+        }
+
+        withTables(stringTable) {
+            stringTable.insert {
+                it[name] = "TestName"
+            }
+
+            val veryLongString = "1".repeat(255)
+            stringTable.update({ stringTable.name eq "TestName" }) {
+                it[name] = veryLongString
+            }
+        }
+    }
 }