In going through the tomcat-dev archives I found a bug report (#605)
involving the JDBCRealm class that may be related to my last bug
report(#602). In my original patch to fix that problem I missed what seems
to be a similar problem with re-establishing the connection if it has failed
in each method that hits the database in Tomcat 3.2.x. Here are a few
snippets of code from the part I think may be in error ( please forgive the
re-formatting of the code, I am trying to avoid the nastiness that comes
with text being wrapped by my email client ):
Tomcat 3.2:
---
from authenticate:
---
// Establish the database connection if necessary
if ((dbConnection == null) || dbConnection.isClosed()) {
log(sm.getString("jdbcRealm.authDBClosed"));
dbConnection = DriverManager.getConnection(connectionURL);
if( (dbConnection == null) || dbConnection.isClosed() ) {
log(sm.getString("jdbcRealm.authDBReOpenFail"));
return false;
}
dbConnection.setReadOnly(true);
}
---
from getUserRoles:
---
if( (dbConnection == null) || dbConnection.isClosed() ) {
log(sm.getString("jdbcRealm.getUserRolesDBClosed"));
dbConnection = DriverManager.getConnection(connectionURL);
if( dbConnection == null || dbConnection.isClosed() ) {
log(sm.getString("jdbcRealm.getUserRolesDBReOpenFail"));
return null;
}
---
from contextInit:
---
Class.forName(driverName);
if ((connectionName == null || connectionName.equals("")) &&
(connectionPassword == null || connectionPassword.equals(""))) {
dbConnection = DriverManager.getConnection(connectionURL);
} else {
dbConnection = DriverManager.getConnection(connectionURL,
connectionName,
connectionPassword);
*****
Tomcat-Catalina-4.0m5:
---
from authorize:
---
if ((dbConnection == null) || dbConnection.isClosed()) {
log(sm.getString("jdbcRealm.authDBClosed"));
if ((connectionName == null || connectionName.equals("")) &&
(connectionPassword == null || connectionPassword.equals(""))) {
dbConnection = DriverManager.getConnection(connectionURL);
} else {
dbConnection = DriverManager.getConnection(connectionURL,
connectionName,
connectionPassword);
}
if( (dbConnection == null) || dbConnection.isClosed() ) {
log(sm.getString("jdbcRealm.authDBReOpenFail"));
return null;
}
...
}
---
from start:
---
if ((connectionName == null || connectionName.equals("")) &&
(connectionPassword == null || connectionPassword.equals(""))) {
dbConnection = DriverManager.getConnection(connectionURL);
} else {
dbConnection = DriverManager.getConnection(connectionURL,
connectionName,
connectionPassword);
}
---
There are a few questions I have about these parts of the code:
* Would it be acceptable for me to submit a patch for this code that
currently gets repeated in 2-3 different places ( in a few different ways
for Tomcat 3.2 ), moving the act of reconnecting to the database to it's own
seperate method that either returns void or returns a connection.
* Would this adversely affect the amount of logging that is done in the
current code?
* If I am interpreting the logic that checks for the presence of a
connection name and connection password correctly, in the case that only
*one* of them is empty or null, the three parameter getConnection method is
used. Is it possible that this could cause problems as well?
I apologize for not catching this with my original patch, and for not
submitting a full patch with this message. I am fairly new at this and want
to be sure 1) My guesswork is correct that these parts of JDBC can/will
cause problems and B) I am doing things the right/acceptable/logical way
before submitting a patch. ( And nacho I promise to include the patch as an
attached text file next time ;) Thanks again and I hope everyone has a great
holiday season!
David Weinrich
