Bug 52767 - Potential Bug or Inconsistency in JDBCRealm.java and JDBCAccessLogValve.java
Potential Bug or Inconsistency in JDBCRealm.java and JDBCAccessLogValve.java
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
trunk
All All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2012-02-25 14:32 UTC by msrbugzilla
Modified: 2012-03-07 13:23 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description msrbugzilla 2012-02-25 14:32:33 UTC
This is Ken Cheung, a Computer Science M.Phil. student. I observed some
code clones in Tomcat and found inconsistent code:

/tomcat/trunk/java/org/apache/catalina/realm/JDBCRealm.java

676	        if (driver == null) {
677	            try {
678	                Class<?> clazz = Class.forName(driverName);
679	                driver = (Driver) clazz.newInstance();
680	            } catch (Throwable e) {
681	                ExceptionUtils.handleThrowable(e);
682	                throw new SQLException(e.getMessage(), e);
683	            }
684	        }
685	
686	        // Open a new connection
687	        Properties props = new Properties();
688	        if (connectionName != null)
689	            props.put("user", connectionName);
690	        if (connectionPassword != null)
691	            props.put("password", connectionPassword);
692	        dbConnection = driver.connect(connectionURL, props);

/tomcat/trunk/java/org/apache/catalina/valves/JDBCAccessLogValve.java

566	        if (driver == null) {
567	            try {
568	                Class<?> clazz = Class.forName(driverName);
569	                driver = (Driver) clazz.newInstance();
570	            } catch (Throwable e) {
571	                ExceptionUtils.handleThrowable(e);
572	                throw new SQLException(e.getMessage(), e);
573	            }
574	        }
575	
576	        // Open a new connection
577	        Properties props = new Properties();
578	        props.put("autoReconnect", "true");
579	        if (connectionName != null) {
580	            props.put("user", connectionName);
581	        }
582	        if (connectionPassword != null) {
583	            props.put("password", connectionPassword);
584	        }
585	        conn = driver.connect(connectionURL, props);

Quick description of the inconsistency
Two code snippets are very similar code, but as you see, in JDBCRealm.java does not use "props.put("autoReconnect", "true")" while JDBCAccessLogValve.java has.

We thought it could be a potential bug or inconsistency. Hope this helps.
Comment 1 Mark Thomas 2012-02-25 23:59:51 UTC
Yep, there are different and the difference is documented.

I don't like a mysql specific setting being in the code - especially that one - but removing it for 7.0.x is not an option since there is a small chance it could break something.

Removing it for 8.0.x (trunk seems reasonable). Users that want it can always add it back in via the URL.
Comment 2 msrbugzilla 2012-02-27 15:31:37 UTC
Could I know where the difference is documented? Thank you.
Comment 3 Mark Thomas 2012-03-07 13:23:58 UTC
(In reply to comment #2)
> Could I know where the difference is documented? Thank you.

In the Javadoc.


This has been fixed in trunk (it will not be back-ported to 7.0.x) and will be included in 8.0.0 onwards.