Skip to content

Commit 9588a09

Browse files
magnuspmagnusp
authored
fix(jdbc): validate prepared statement kind before execution (#57)
Closes #29. Part of #54. ## What changed - `LibsqlPreparedStatement` now classifies the statement kind once, at construction, and rejects wrong-kind calls before any execution: - `executeQuery()` on a non-result statement throws `Query does not return results` without calling the connection. - `executeUpdate()` on a result-producing statement throws `Query returns results` without calling the connection. - Added `LibsqlPreparedStatementTest` with behavior tests that record connection execution calls: wrong-kind calls assert zero calls, and right-kind calls still execute and return rows/update counts. - Covered `INSERT`, `SELECT`, `INSERT ... RETURNING`, and `WITH ... SELECT`. ## Contract decision Matched xerial 3.53.4.0 `JDBC3PreparedStatement.java:73-78,110-116`: validate the prepared statement kind before executing, using xerial's exact messages. `execute()` and the non-prepared `LibsqlStatement` paths are unchanged because xerial's `Statement` path still executes before reporting that the result kind is wrong. ## Evidence Failing before (production file reverted to `HEAD`, new tests kept): ``` ./mvnw -pl libsql-jdbc -am -Dtest=LibsqlPreparedStatementTest -Dsurefire.failIfNoSpecifiedTests=false test Tests run: 6, Failures: 3, Errors: 0, Skipped: 0 executeQueryOnInsertThrowsWithoutExecuting message was "PreparedStatement did not produce a ResultSet: ..." executeUpdateOnSelectThrowsWithoutExecuting Expecting code to raise a throwable executeUpdateOnWithSelectThrowsWithoutExecuting Expecting code to raise a throwable ``` Passing after: ``` ./mvnw -pl libsql-jdbc -am test Tests run: 9, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` The prepared-statement test class runs 6/6 green. `./mvnw spotless:apply` succeeds. ## Deviations Hrana does not expose SQLite's prepare-time column count without a server round trip, and the steering requires validation before any client call. The driver therefore classifies the kind lexically (comments, quoted literals, parenthesis depth, `RETURNING`, `WITH` bodies, and `PRAGMA ... = ...` assignments are handled). This is the only transport-forced deviation from xerial's `columnCount` approach; exotic SQL outside this classification could be treated as a non-result statement.
1 parent 5202fae commit 9588a09

2 files changed

Lines changed: 268 additions & 1 deletion

File tree

‎libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatement.java‎

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,19 @@
2626
import java.util.ArrayList;
2727
import java.util.Calendar;
2828
import java.util.List;
29+
import java.util.Locale;
2930

3031
public class LibsqlPreparedStatement extends LibsqlStatement implements PreparedStatement {
3132

3233
private final String sql;
34+
private final boolean returnsResultSet;
3335
// Mitigation for Bug 7: Sequential ArrayList instead of Map to eliminate Integer autoboxing overhead
3436
private final List<Value> parameters = new ArrayList<>();
3537

3638
public LibsqlPreparedStatement(LibsqlConnection connection, String sql) {
3739
super(connection);
3840
this.sql = sql;
41+
this.returnsResultSet = returnsResultSet(sql);
3942
}
4043

4144
private void setParam(int parameterIndex, Value value) throws SQLException {
@@ -53,19 +56,163 @@ private Stmt buildCurrentStmt() {
5356

5457
@Override
5558
public ResultSet executeQuery() throws SQLException {
59+
checkClosed();
60+
if (!returnsResultSet) {
61+
throw new SQLException("Query does not return results");
62+
}
5663
execute();
5764
if (currentResultSet == null) {
58-
throw new SQLException("PreparedStatement did not produce a ResultSet: " + sql);
65+
throw new SQLException("Query does not return results");
5966
}
6067
return currentResultSet;
6168
}
6269

6370
@Override
6471
public int executeUpdate() throws SQLException {
72+
checkClosed();
73+
if (returnsResultSet) {
74+
throw new SQLException("Query returns results");
75+
}
6576
execute();
6677
return (int) updateCount;
6778
}
6879

80+
private static boolean returnsResultSet(String sql) {
81+
List<Token> tokens = tokenize(sql);
82+
if (tokens.isEmpty()) {
83+
return false;
84+
}
85+
return switch (tokens.get(0).word()) {
86+
case "SELECT", "VALUES", "EXPLAIN" -> true;
87+
case "PRAGMA" -> pragmaReturnsResultSet(tokens);
88+
case "INSERT", "UPDATE", "DELETE", "REPLACE" -> hasTopLevelReturning(tokens, 1);
89+
case "WITH" -> withReturnsResultSet(tokens);
90+
default -> false;
91+
};
92+
}
93+
94+
private static boolean pragmaReturnsResultSet(List<Token> tokens) {
95+
for (int i = 1; i < tokens.size(); i++) {
96+
Token token = tokens.get(i);
97+
if (token.depth() == 0 && token.word().equals("=")) {
98+
return false;
99+
}
100+
}
101+
return true;
102+
}
103+
104+
private static boolean withReturnsResultSet(List<Token> tokens) {
105+
for (int i = 1; i < tokens.size(); i++) {
106+
Token token = tokens.get(i);
107+
if (token.depth() != 0) {
108+
continue;
109+
}
110+
switch (token.word()) {
111+
case "SELECT", "VALUES" -> {
112+
return true;
113+
}
114+
case "INSERT", "UPDATE", "DELETE", "REPLACE" -> {
115+
return hasTopLevelReturning(tokens, i + 1);
116+
}
117+
default -> {
118+
}
119+
}
120+
}
121+
return false;
122+
}
123+
124+
private static boolean hasTopLevelReturning(List<Token> tokens, int from) {
125+
for (int i = from; i < tokens.size(); i++) {
126+
Token token = tokens.get(i);
127+
if (token.depth() == 0 && token.word().equals("RETURNING")) {
128+
return true;
129+
}
130+
}
131+
return false;
132+
}
133+
134+
private static List<Token> tokenize(String sql) {
135+
List<Token> tokens = new ArrayList<>();
136+
if (sql == null) {
137+
return tokens;
138+
}
139+
int depth = 0;
140+
int i = 0;
141+
int n = sql.length();
142+
while (i < n) {
143+
char c = sql.charAt(i);
144+
if (Character.isWhitespace(c)) {
145+
i++;
146+
} else if (c == '-' && i + 1 < n && sql.charAt(i + 1) == '-') {
147+
i += 2;
148+
while (i < n && sql.charAt(i) != '\n') {
149+
i++;
150+
}
151+
} else if (c == '/' && i + 1 < n && sql.charAt(i + 1) == '*') {
152+
i += 2;
153+
while (i + 1 < n && !(sql.charAt(i) == '*' && sql.charAt(i + 1) == '/')) {
154+
i++;
155+
}
156+
i = Math.min(n, i + 2);
157+
} else if (c == '\'' || c == '"' || c == '`') {
158+
i = skipQuoted(sql, i, c);
159+
} else if (c == '[') {
160+
i++;
161+
while (i < n && sql.charAt(i) != ']') {
162+
i++;
163+
}
164+
if (i < n) {
165+
i++;
166+
}
167+
} else if (c == '(') {
168+
depth++;
169+
i++;
170+
} else if (c == ')') {
171+
depth = Math.max(0, depth - 1);
172+
i++;
173+
} else if (c == '=') {
174+
tokens.add(new Token("=", depth));
175+
i++;
176+
} else if (isWordStart(c)) {
177+
int start = i;
178+
i++;
179+
while (i < n && isWordPart(sql.charAt(i))) {
180+
i++;
181+
}
182+
tokens.add(new Token(sql.substring(start, i).toUpperCase(Locale.ROOT), depth));
183+
} else {
184+
i++;
185+
}
186+
}
187+
return tokens;
188+
}
189+
190+
private static int skipQuoted(String sql, int start, char quote) {
191+
int i = start + 1;
192+
int n = sql.length();
193+
while (i < n) {
194+
if (sql.charAt(i) == quote) {
195+
if (i + 1 < n && sql.charAt(i + 1) == quote) {
196+
i += 2;
197+
continue;
198+
}
199+
return i + 1;
200+
}
201+
i++;
202+
}
203+
return n;
204+
}
205+
206+
private static boolean isWordStart(char c) {
207+
return Character.isLetter(c) || c == '_';
208+
}
209+
210+
private static boolean isWordPart(char c) {
211+
return Character.isLetterOrDigit(c) || c == '_' || c == '$';
212+
}
213+
214+
private record Token(String word, int depth) {}
215+
69216
@Override
70217
public boolean execute() throws SQLException {
71218
checkClosed();

‎libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlPreparedStatementTest.java‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package com.github.magnusp.libsql.jdbc;
2+
3+
import com.github.magnusp.libsql.hrana.codec.Col;
4+
import com.github.magnusp.libsql.hrana.codec.Row;
5+
import com.github.magnusp.libsql.hrana.codec.Stmt;
6+
import com.github.magnusp.libsql.hrana.codec.StmtResult;
7+
import com.github.magnusp.libsql.hrana.codec.Value;
8+
import org.junit.jupiter.api.Test;
9+
10+
import java.sql.ResultSet;
11+
import java.sql.SQLException;
12+
import java.util.List;
13+
14+
import static org.assertj.core.api.Assertions.assertThat;
15+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
16+
17+
class LibsqlPreparedStatementTest {
18+
19+
private static final StmtResult ROWS = new StmtResult(
20+
List.of(new Col("id", "INTEGER")),
21+
List.of(new Row(List.of(Value.of(1L)))),
22+
0,
23+
null);
24+
25+
private static final StmtResult NO_ROWS = new StmtResult(List.of(), List.of(), 1, 1L);
26+
27+
@Test
28+
void executeQueryOnInsertThrowsWithoutExecuting() throws SQLException {
29+
RecordingConnection connection = new RecordingConnection(NO_ROWS);
30+
try (LibsqlPreparedStatement statement =
31+
new LibsqlPreparedStatement(connection, "insert into t values (2)")) {
32+
assertThatThrownBy(statement::executeQuery)
33+
.isInstanceOf(SQLException.class)
34+
.hasMessage("Query does not return results");
35+
}
36+
37+
assertThat(connection.executeCalls).isZero();
38+
}
39+
40+
@Test
41+
void executeUpdateOnSelectThrowsWithoutExecuting() throws SQLException {
42+
RecordingConnection connection = new RecordingConnection(ROWS);
43+
try (LibsqlPreparedStatement statement =
44+
new LibsqlPreparedStatement(connection, "select id from t")) {
45+
assertThatThrownBy(statement::executeUpdate)
46+
.isInstanceOf(SQLException.class)
47+
.hasMessage("Query returns results");
48+
}
49+
50+
assertThat(connection.executeCalls).isZero();
51+
}
52+
53+
@Test
54+
void executeQueryOnSelectStillExecutes() throws SQLException {
55+
RecordingConnection connection = new RecordingConnection(ROWS);
56+
try (LibsqlPreparedStatement statement =
57+
new LibsqlPreparedStatement(connection, "select id from t")) {
58+
ResultSet resultSet = statement.executeQuery();
59+
60+
assertThat(resultSet.next()).isTrue();
61+
assertThat(resultSet.getLong(1)).isEqualTo(1L);
62+
}
63+
64+
assertThat(connection.executeCalls).isEqualTo(1);
65+
}
66+
67+
@Test
68+
void executeUpdateOnInsertStillExecutes() throws SQLException {
69+
RecordingConnection connection = new RecordingConnection(NO_ROWS);
70+
try (LibsqlPreparedStatement statement =
71+
new LibsqlPreparedStatement(connection, "insert into t values (2)")) {
72+
assertThat(statement.executeUpdate()).isEqualTo(1);
73+
}
74+
75+
assertThat(connection.executeCalls).isEqualTo(1);
76+
}
77+
78+
@Test
79+
void executeQueryOnInsertReturningStillExecutes() throws SQLException {
80+
RecordingConnection connection = new RecordingConnection(ROWS);
81+
try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement(
82+
connection, "insert into t values (2) returning id")) {
83+
ResultSet resultSet = statement.executeQuery();
84+
85+
assertThat(resultSet.next()).isTrue();
86+
assertThat(resultSet.getLong(1)).isEqualTo(1L);
87+
}
88+
89+
assertThat(connection.executeCalls).isEqualTo(1);
90+
}
91+
92+
@Test
93+
void executeUpdateOnWithSelectThrowsWithoutExecuting() throws SQLException {
94+
RecordingConnection connection = new RecordingConnection(ROWS);
95+
try (LibsqlPreparedStatement statement = new LibsqlPreparedStatement(
96+
connection, "with ids as (select id from t) select id from ids")) {
97+
assertThatThrownBy(statement::executeUpdate)
98+
.isInstanceOf(SQLException.class)
99+
.hasMessage("Query returns results");
100+
}
101+
102+
assertThat(connection.executeCalls).isZero();
103+
}
104+
105+
private static final class RecordingConnection extends LibsqlConnection {
106+
private final StmtResult result;
107+
private int executeCalls;
108+
109+
private RecordingConnection(StmtResult result) {
110+
super(null, null);
111+
this.result = result;
112+
}
113+
114+
@Override
115+
public StmtResult executeStatement(Stmt stmt) {
116+
executeCalls++;
117+
return result;
118+
}
119+
}
120+
}

0 commit comments

Comments
 (0)