You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/04/17 15:43:58 UTC

[tomcat] branch 7.0.x updated: Fix potential resource leaks on exception paths

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/7.0.x by this push:
     new ad47568  Fix potential resource leaks on exception paths
ad47568 is described below

commit ad475688da93bff59ae3661481b3bcf7d2d509a7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Apr 17 16:16:53 2019 +0100

    Fix potential resource leaks on exception paths
    
    Identified by Coverity scan
---
 .../org/apache/catalina/realm/DataSourceRealm.java | 78 ++++++----------------
 webapps/docs/changelog.xml                         |  4 ++
 2 files changed, 24 insertions(+), 58 deletions(-)

diff --git a/java/org/apache/catalina/realm/DataSourceRealm.java b/java/org/apache/catalina/realm/DataSourceRealm.java
index dc18275..97ddf07 100644
--- a/java/org/apache/catalina/realm/DataSourceRealm.java
+++ b/java/org/apache/catalina/realm/DataSourceRealm.java
@@ -445,31 +445,32 @@ public class DataSourceRealm
         }
     }
 
+
     /**
      * Return the password associated with the given principal's user name.
+     *
      * @param dbConnection The database connection to be used
      * @param username Username for which password should be retrieved
+     *
+     * @return the password for the specified user
      */
-    protected String getPassword(Connection dbConnection,
-                                 String username) {
+    protected String getPassword(Connection dbConnection, String username) {
 
         ResultSet rs = null;
         PreparedStatement stmt = null;
         String dbCredentials = null;
 
         try {
-            stmt = credentials(dbConnection, username);
+            stmt = dbConnection.prepareStatement(preparedCredentials);
+            stmt.setString(1, username);
             rs = stmt.executeQuery();
             if (rs.next()) {
                 dbCredentials = rs.getString(1);
             }
 
             return (dbCredentials != null) ? dbCredentials.trim() : null;
-
-        } catch(SQLException e) {
-            containerLog.error(
-                    sm.getString("dataSourceRealm.getPassword.exception",
-                                 username), e);
+        } catch (SQLException e) {
+            containerLog.error(sm.getString("dataSourceRealm.getPassword.exception", username), e);
         } finally {
             try {
                 if (rs != null) {
@@ -530,13 +531,16 @@ public class DataSourceRealm
         }
     }
 
+
     /**
-     * Return the roles associated with the given user name
+     * Return the roles associated with the given user name.
+     *
      * @param dbConnection The database connection to be used
-     * @param username Username for which roles should be retrieved
+     * @param username User name for which roles should be retrieved
+     *
+     * @return an array list of the role names
      */
-    protected ArrayList<String> getRoles(Connection dbConnection,
-                                     String username) {
+    protected ArrayList<String> getRoles(Connection dbConnection, String username) {
 
         if (allRolesMode != AllRolesMode.STRICT_MODE && !isRoleStoreDefined()) {
             // Using an authentication only configuration and no role store has
@@ -549,7 +553,8 @@ public class DataSourceRealm
         ArrayList<String> list = null;
 
         try {
-            stmt = roles(dbConnection, username);
+            stmt = dbConnection.prepareStatement(preparedRoles);
+            stmt.setString(1, username);
             rs = stmt.executeQuery();
             list = new ArrayList<String>();
 
@@ -561,8 +566,7 @@ public class DataSourceRealm
             }
             return list;
         } catch(SQLException e) {
-            containerLog.error(
-                sm.getString("dataSourceRealm.getRoles.exception", username), e);
+            containerLog.error(sm.getString("dataSourceRealm.getRoles.exception", username), e);
         }
         finally {
             try {
@@ -574,55 +578,13 @@ public class DataSourceRealm
                 }
             } catch (SQLException e) {
                     containerLog.error(
-                        sm.getString("dataSourceRealm.getRoles.exception",
-                                     username), e);
+                            sm.getString("dataSourceRealm.getRoles.exception", username), e);
             }
         }
 
         return null;
     }
 
-    /**
-     * Return a PreparedStatement configured to perform the SELECT required
-     * to retrieve user credentials for the specified username.
-     *
-     * @param dbConnection The database connection to be used
-     * @param username Username for which credentials should be retrieved
-     *
-     * @exception SQLException if a database error occurs
-     */
-    private PreparedStatement credentials(Connection dbConnection,
-                                            String username)
-        throws SQLException {
-
-        PreparedStatement credentials =
-            dbConnection.prepareStatement(preparedCredentials);
-
-        credentials.setString(1, username);
-        return (credentials);
-
-    }
-
-    /**
-     * Return a PreparedStatement configured to perform the SELECT required
-     * to retrieve user roles for the specified username.
-     *
-     * @param dbConnection The database connection to be used
-     * @param username Username for which roles should be retrieved
-     *
-     * @exception SQLException if a database error occurs
-     */
-    private PreparedStatement roles(Connection dbConnection, String username)
-        throws SQLException {
-
-        PreparedStatement roles =
-            dbConnection.prepareStatement(preparedRoles);
-
-        roles.setString(1, username);
-        return (roles);
-
-    }
-
 
     private boolean isRoleStoreDefined() {
         return userRoleTable != null || roleNameCol != null;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 907a9c9..accad91 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -73,6 +73,10 @@
         Fix a potential concurrency issue in the main Sendfile thread of the APR
         connector. Identified by Coverity scan. (markt)
       </fix>
+      <fix>
+        Fix a potential resource leak on some exception paths in the
+        <code>DataSourceRealm</code>. Identified by Coverity scan. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org