Skip to content

Fix proxying null value result set#745

Merged
markt-asf merged 1 commit intoapache:mainfrom
htdebeer:main
Aug 22, 2024
Merged

Fix proxying null value result set#745
markt-asf merged 1 commit intoapache:mainfrom
htdebeer:main

Conversation

@htdebeer
Copy link
Copy Markdown
Contributor

@htdebeer htdebeer commented Aug 16, 2024

When invoking a method on a proxied statement returns null, that statement's proxy should also return null.

In particular, Statement#getResultSet should not return a ResultSetProxy when the proxied statement returns null. Otherwise, that ResultSet proxy has a null delegate and calling methods on the proxy that lead to calling methods on the delegate could result in a SqlException with message "ResultSet closed."

We found this issue because our Spring + MyBatis application started throwing SqlExceptions when calling stored procedures after upgrading to Tomcat v. 10.1.28, whereas it didn't in version 10.1.26.

Looking in the MyBatis code, it checks if a Statement's ResultSet is null or not. If it isn't—and it isn't when the ResultSet is proxied with a null delegate—, it later calls method ResultSet#getMetaData(). And that method now responds with the SqlException with message "ResultSet closed.".

I think the new behavior was introduced in #742.

When invoking a method on a proxied statement returns `null`, that
statement's proxy should also return `null`.

In particular, `Statement#getResultSet` should not return a
`ResultSetProxy` when the proxied statement returns `null`. Otherwise,
that `ResultSet` proxy has a `null` delegate and calling methods on the
proxy that lead to calling methods on the delegate could result in a
`SqlException` with message "ResultSet closed."

Only prevent creation of `ResultSetProxy` for `null` return values when
invoking a method on `Statement`. In particular, don't do an early
return for `void` methods like `#close` because there's cleanup code for
that `close` method.
@htdebeer
Copy link
Copy Markdown
Contributor Author

Created bug ticket for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants