From f53b704dba60c72d912400d95769b0a676037024 Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:17:38 +0200 Subject: [PATCH] fix(jdbc): validate prepared statement kind before execution --- .../libsql/jdbc/LibsqlPreparedStatement.java | 149 +++++++++++++++++- .../jdbc/LibsqlPreparedStatementTest.java | 120 ++++++++++++++ 2 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java index 15b2522..7c27397 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java @@ -26,16 +26,19 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.List; +import java.util.Locale; public class LibsqlPreparedStatement extends LibsqlStatement implements PreparedStatement { private final String sql; + private final boolean returnsResultSet; // Mitigation for Bug 7: Sequential ArrayList instead of Map to eliminate Integer autoboxing overhead private final List parameters = new ArrayList<>(); public LibsqlPreparedStatement(LibsqlConnection connection, String sql) { super(connection); this.sql = sql; + this.returnsResultSet = returnsResultSet(sql); } private void setParam(int parameterIndex, Value value) throws SQLException { @@ -53,19 +56,163 @@ private Stmt buildCurrentStmt() { @Override public ResultSet executeQuery() throws SQLException { + checkClosed(); + if (!returnsResultSet) { + throw new SQLException("Query does not return results"); + } execute(); if (currentResultSet == null) { - throw new SQLException("PreparedStatement did not produce a ResultSet: " + sql); + throw new SQLException("Query does not return results"); } return currentResultSet; } @Override public int executeUpdate() throws SQLException { + checkClosed(); + if (returnsResultSet) { + throw new SQLException("Query returns results"); + } execute(); return (int) updateCount; } + private static boolean returnsResultSet(String sql) { + List tokens = tokenize(sql); + if (tokens.isEmpty()) { + return false; + } + return switch (tokens.get(0).word()) { + case "SELECT", "VALUES", "EXPLAIN" -> true; + case "PRAGMA" -> pragmaReturnsResultSet(tokens); + case "INSERT", "UPDATE", "DELETE", "REPLACE" -> hasTopLevelReturning(tokens, 1); + case "WITH" -> withReturnsResultSet(tokens); + default -> false; + }; + } + + private static boolean pragmaReturnsResultSet(List tokens) { + for (int i = 1; i < tokens.size(); i++) { + Token token = tokens.get(i); + if (token.depth() == 0 && token.word().equals("=")) { + return false; + } + } + return true; + } + + private static boolean withReturnsResultSet(List tokens) { + for (int i = 1; i < tokens.size(); i++) { + Token token = tokens.get(i); + if (token.depth() != 0) { + continue; + } + switch (token.word()) { + case "SELECT", "VALUES" -> { + return true; + } + case "INSERT", "UPDATE", "DELETE", "REPLACE" -> { + return hasTopLevelReturning(tokens, i + 1); + } + default -> { + } + } + } + return false; + } + + private static boolean hasTopLevelReturning(List tokens, int from) { + for (int i = from; i < tokens.size(); i++) { + Token token = tokens.get(i); + if (token.depth() == 0 && token.word().equals("RETURNING")) { + return true; + } + } + return false; + } + + private static List tokenize(String sql) { + List tokens = new ArrayList<>(); + if (sql == null) { + return tokens; + } + int depth = 0; + int i = 0; + int n = sql.length(); + while (i < n) { + char c = sql.charAt(i); + if (Character.isWhitespace(c)) { + i++; + } else if (c == '-' && i + 1 < n && sql.charAt(i + 1) == '-') { + i += 2; + while (i < n && sql.charAt(i) != '\n') { + i++; + } + } else if (c == '/' && i + 1 < n && sql.charAt(i + 1) == '*') { + i += 2; + while (i + 1 < n && !(sql.charAt(i) == '*' && sql.charAt(i + 1) == '/')) { + i++; + } + i = Math.min(n, i + 2); + } else if (c == '\'' || c == '"' || c == '`') { + i = skipQuoted(sql, i, c); + } else if (c == '[') { + i++; + while (i < n && sql.charAt(i) != ']') { + i++; + } + if (i < n) { + i++; + } + } else if (c == '(') { + depth++; + i++; + } else if (c == ')') { + depth = Math.max(0, depth - 1); + i++; + } else if (c == '=') { + tokens.add(new Token("=", depth)); + i++; + } else if (isWordStart(c)) { + int start = i; + i++; + while (i < n && isWordPart(sql.charAt(i))) { + i++; + } + tokens.add(new Token(sql.substring(start, i).toUpperCase(Locale.ROOT), depth)); + } else { + i++; + } + } + return tokens; + } + + private static int skipQuoted(String sql, int start, char quote) { + int i = start + 1; + int n = sql.length(); + while (i < n) { + if (sql.charAt(i) == quote) { + if (i + 1 < n && sql.charAt(i + 1) == quote) { + i += 2; + continue; + } + return i + 1; + } + i++; + } + return n; + } + + private static boolean isWordStart(char c) { + return Character.isLetter(c) || c == '_'; + } + + private static boolean isWordPart(char c) { + return Character.isLetterOrDigit(c) || c == '_' || c == '$'; + } + + private record Token(String word, int depth) {} + @Override public boolean execute() throws SQLException { checkClosed(); diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java new file mode 100644 index 0000000..2c880bd --- /dev/null +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java @@ -0,0 +1,120 @@ +package com.github.magnusp.libsql.jdbc; + +import com.github.magnusp.libsql.hrana.codec.Col; +import com.github.magnusp.libsql.hrana.codec.Row; +import com.github.magnusp.libsql.hrana.codec.Stmt; +import com.github.magnusp.libsql.hrana.codec.StmtResult; +import com.github.magnusp.libsql.hrana.codec.Value; +import org.junit.jupiter.api.Test; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class LibsqlPreparedStatementTest { + + private static final StmtResult ROWS = new StmtResult( + List.of(new Col("id", "INTEGER")), + List.of(new Row(List.of(Value.of(1L)))), + 0, + null); + + private static final StmtResult NO_ROWS = new StmtResult(List.of(), List.of(), 1, 1L); + + @Test + void executeQueryOnInsertThrowsWithoutExecuting() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "insert into t values (2)")) { + assertThatThrownBy(statement::executeQuery) + .isInstanceOf(SQLException.class) + .hasMessage("Query does not return results"); + } + + assertThat(connection.executeCalls).isZero(); + } + + @Test + void executeUpdateOnSelectThrowsWithoutExecuting() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "select id from t")) { + assertThatThrownBy(statement::executeUpdate) + .isInstanceOf(SQLException.class) + .hasMessage("Query returns results"); + } + + assertThat(connection.executeCalls).isZero(); + } + + @Test + void executeQueryOnSelectStillExecutes() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "select id from t")) { + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + assertThat(resultSet.getLong(1)).isEqualTo(1L); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void executeUpdateOnInsertStillExecutes() throws SQLException { + RecordingConnection connection = new RecordingConnection(NO_ROWS); + try (LibsqlPreparedStatement statement = + new LibsqlPreparedStatement(connection, "insert into t values (2)")) { + assertThat(statement.executeUpdate()).isEqualTo(1); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void executeQueryOnInsertReturningStillExecutes() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement( + connection, "insert into t values (2) returning id")) { + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + assertThat(resultSet.getLong(1)).isEqualTo(1L); + } + + assertThat(connection.executeCalls).isEqualTo(1); + } + + @Test + void executeUpdateOnWithSelectThrowsWithoutExecuting() throws SQLException { + RecordingConnection connection = new RecordingConnection(ROWS); + try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement( + connection, "with ids as (select id from t) select id from ids")) { + assertThatThrownBy(statement::executeUpdate) + .isInstanceOf(SQLException.class) + .hasMessage("Query returns results"); + } + + assertThat(connection.executeCalls).isZero(); + } + + private static final class RecordingConnection extends LibsqlConnection { + private final StmtResult result; + private int executeCalls; + + private RecordingConnection(StmtResult result) { + super(null, null); + this.result = result; + } + + @Override + public StmtResult executeStatement(Stmt stmt) { + executeCalls++; + return result; + } + } +}