This is an automated email from the ASF dual-hosted git repository. elecharny pushed a commit to branch 1.2.X in repository https://gitbox.apache.org/repos/asf/mina-ftpserver.git
The following commit(s) were added to refs/heads/1.2.X by this push: new b07c87f2 Used try-with-resources; Removed close methods, now useless; Some code improvement b07c87f2 is described below commit b07c87f2267e3c3667d266ca9b5777909a5e834b Author: emmanuel lecharny <elecha...@apache.org> AuthorDate: Sun Jan 5 08:26:36 2025 +0100 Used try-with-resources; Removed close methods, now useless; Some code improvement --- .../usermanager/impl/AbstractUserManager.java | 1 + .../ftpserver/usermanager/impl/DbUserManager.java | 343 ++++++++------------- 2 files changed, 123 insertions(+), 221 deletions(-) diff --git a/core/src/main/java/org/apache/ftpserver/usermanager/impl/AbstractUserManager.java b/core/src/main/java/org/apache/ftpserver/usermanager/impl/AbstractUserManager.java index 68151b24..2d504396 100644 --- a/core/src/main/java/org/apache/ftpserver/usermanager/impl/AbstractUserManager.java +++ b/core/src/main/java/org/apache/ftpserver/usermanager/impl/AbstractUserManager.java @@ -90,6 +90,7 @@ public abstract class AbstractUserManager implements UserManager { /** * Retrieve the password encryptor used for this user manager + * * @return The password encryptor. Default to {@link Md5PasswordEncryptor} * if no other has been provided */ diff --git a/core/src/main/java/org/apache/ftpserver/usermanager/impl/DbUserManager.java b/core/src/main/java/org/apache/ftpserver/usermanager/impl/DbUserManager.java index 88f3e4ec..ceab1e30 100644 --- a/core/src/main/java/org/apache/ftpserver/usermanager/impl/DbUserManager.java +++ b/core/src/main/java/org/apache/ftpserver/usermanager/impl/DbUserManager.java @@ -103,18 +103,12 @@ public class DbUserManager extends AbstractUserManager { this.authenticateStmt = authenticateStmt; this.isAdminStmt = isAdminStmt; - Connection con = null; - try { - // test the connection - con = createConnection(); - + try (Connection connection = createConnection()) { LOG.info("Database connection opened."); } catch (SQLException ex) { LOG.error("Failed to open connection to user database", ex); throw new FtpServerConfigurationException( "Failed to open connection to user database", ex); - } finally{ - closeQuitely(con); } } @@ -283,32 +277,24 @@ public class DbUserManager extends AbstractUserManager { */ @Override public boolean isAdmin(String login) throws FtpException { - // check input if (login == null) { return false; } - Statement stmt = null; - ResultSet rs = null; - try { - - // create the sql query - HashMap<String, Object> map = new HashMap<>(); - map.put(ATTR_LOGIN, escapeString(login)); - String sql = StringUtils.replaceString(isAdminStmt, map); - LOG.info(sql); + // create the sql query + HashMap<String, Object> map = new HashMap<>(); + map.put(ATTR_LOGIN, escapeString(login)); + String sql = StringUtils.replaceString(isAdminStmt, map); + LOG.info(sql); - // execute query - stmt = createConnection().createStatement(); - rs = stmt.executeQuery(sql); + // execute query + try (Statement stmt = createConnection().createStatement(); + ResultSet rs = stmt.executeQuery(sql)) { return rs.next(); } catch (SQLException ex) { LOG.error("DbUserManager.isAdmin()", ex); throw new FtpException("DbUserManager.isAdmin()", ex); - } finally { - closeQuitely(rs); - closeQuitely(stmt); } } @@ -336,15 +322,11 @@ public class DbUserManager extends AbstractUserManager { LOG.info(sql); // execute query - Statement stmt = null; - try { - stmt = createConnection().createStatement(); + try (Statement stmt = createConnection().createStatement()){ stmt.executeUpdate(sql); } catch (SQLException ex) { LOG.error("DbUserManager.delete()", ex); throw new FtpException("DbUserManager.delete()", ex); - } finally { - closeQuitely(stmt); } } @@ -359,131 +341,84 @@ public class DbUserManager extends AbstractUserManager { throw new NullPointerException("User name is null."); } - Statement stmt = null; - try { + // create sql query + HashMap<String, Object> map = new HashMap<>(); + map.put(ATTR_LOGIN, escapeString(user.getName())); - // create sql query - HashMap<String, Object> map = new HashMap<>(); - map.put(ATTR_LOGIN, escapeString(user.getName())); - - String password = null; - if (user.getPassword() != null) { - // password provided, encrypt it and store the encrypted value - password= getPasswordEncryptor().encrypt(user.getPassword()); - } else { - // password was not provided, either load from the existing user and store that again - // or store as null - ResultSet rs = null; - - try { - User userWithPassword = selectUserByName(user.getName()); - - if (userWithPassword != null) { - // user exists, reuse password - password = userWithPassword.getPassword(); - } - } finally { - closeQuitely(rs); - } - } - map.put(ATTR_PASSWORD, escapeString(password)); + String password = null; + if (user.getPassword() != null) { + // password provided, encrypt it and store the encrypted value + password= getPasswordEncryptor().encrypt(user.getPassword()); + } else { + // password was not provided, either load from the existing user and store that again + // or store as null + try { + User userWithPassword = selectUserByName(user.getName()); - String home = user.getHomeDirectory(); - if (home == null) { - home = "/"; - } - map.put(ATTR_HOME, escapeString(home)); - map.put(ATTR_ENABLE, String.valueOf(user.getEnabled())); - - map.put(ATTR_WRITE_PERM, String.valueOf(user - .authorize(new WriteRequest()) != null)); - map.put(ATTR_MAX_IDLE_TIME, user.getMaxIdleTime()); - - TransferRateRequest transferRateRequest = new TransferRateRequest(); - transferRateRequest = (TransferRateRequest) user - .authorize(transferRateRequest); - - if (transferRateRequest != null) { - map.put(ATTR_MAX_UPLOAD_RATE, transferRateRequest - .getMaxUploadRate()); - map.put(ATTR_MAX_DOWNLOAD_RATE, transferRateRequest - .getMaxDownloadRate()); - } else { - map.put(ATTR_MAX_UPLOAD_RATE, 0); - map.put(ATTR_MAX_DOWNLOAD_RATE, 0); + if (userWithPassword != null) { + // user exists, reuse password + password = userWithPassword.getPassword(); + } + } catch (SQLException ex) { + LOG.error("DbUserManager.save()", ex); + throw new FtpException("DbUserManager.save()", ex); } + } - // request that always will succeed - ConcurrentLoginRequest concurrentLoginRequest = new ConcurrentLoginRequest( - 0, 0); - concurrentLoginRequest = (ConcurrentLoginRequest) user - .authorize(concurrentLoginRequest); - - if (concurrentLoginRequest != null) { - map.put(ATTR_MAX_LOGIN_NUMBER, concurrentLoginRequest - .getMaxConcurrentLogins()); - map.put(ATTR_MAX_LOGIN_PER_IP, concurrentLoginRequest - .getMaxConcurrentLoginsPerIP()); - } else { - map.put(ATTR_MAX_LOGIN_NUMBER, 0); - map.put(ATTR_MAX_LOGIN_PER_IP, 0); - } + map.put(ATTR_PASSWORD, escapeString(password)); - String sql = null; - if (!doesExist(user.getName())) { - sql = StringUtils.replaceString(insertUserStmt, map); - } else { - sql = StringUtils.replaceString(updateUserStmt, map); - } - LOG.info(sql); + String home = user.getHomeDirectory(); - // execute query - stmt = createConnection().createStatement(); - stmt.executeUpdate(sql); - } catch (SQLException ex) { - LOG.error("DbUserManager.save()", ex); - throw new FtpException("DbUserManager.save()", ex); - } finally { - closeQuitely(stmt); + if (home == null) { + home = "/"; } - } - private void closeQuitely(Statement stmt) { - if (stmt != null) { - Connection con = null; - try { - con = stmt.getConnection(); - } catch (Exception e) { - } + map.put(ATTR_HOME, escapeString(home)); + map.put(ATTR_ENABLE, String.valueOf(user.getEnabled())); + map.put(ATTR_WRITE_PERM, String.valueOf(user.authorize(new WriteRequest()) != null)); + map.put(ATTR_MAX_IDLE_TIME, user.getMaxIdleTime()); - try { - stmt.close(); - } catch (SQLException e) { - // ignore - } + TransferRateRequest transferRateRequest = new TransferRateRequest(); + transferRateRequest = (TransferRateRequest) user.authorize(transferRateRequest); - closeQuitely(con); + if (transferRateRequest != null) { + map.put(ATTR_MAX_UPLOAD_RATE, transferRateRequest.getMaxUploadRate()); + map.put(ATTR_MAX_DOWNLOAD_RATE, transferRateRequest.getMaxDownloadRate()); + } else { + map.put(ATTR_MAX_UPLOAD_RATE, 0); + map.put(ATTR_MAX_DOWNLOAD_RATE, 0); } - } - private void closeQuitely(ResultSet rs) { - if (rs != null) { - try { - rs.close(); - } catch (SQLException e) { - // ignore - } + // request that always will succeed + ConcurrentLoginRequest concurrentLoginRequest = new ConcurrentLoginRequest(0, 0); + concurrentLoginRequest = (ConcurrentLoginRequest) user.authorize(concurrentLoginRequest); + + if (concurrentLoginRequest != null) { + map.put(ATTR_MAX_LOGIN_NUMBER, concurrentLoginRequest.getMaxConcurrentLogins()); + map.put(ATTR_MAX_LOGIN_PER_IP, concurrentLoginRequest.getMaxConcurrentLoginsPerIP()); + } else { + map.put(ATTR_MAX_LOGIN_NUMBER, 0); + map.put(ATTR_MAX_LOGIN_PER_IP, 0); } - } - protected void closeQuitely(Connection con) { - if (con != null) { - try { - con.close(); - } catch (SQLException e) { - // ignore - } + String sql = null; + + if (!doesExist(user.getName())) { + sql = StringUtils.replaceString(insertUserStmt, map); + } else { + sql = StringUtils.replaceString(updateUserStmt, map); + } + + LOG.info(sql); + + // execute query + try (Statement stmt = createConnection().createStatement()){ + // execute query + stmt.executeUpdate(sql); + } catch (SQLException ex) { + LOG.error("DbUserManager.save()", ex); + throw new FtpException("DbUserManager.save()", ex); } } @@ -494,15 +429,12 @@ public class DbUserManager extends AbstractUserManager { String sql = StringUtils.replaceString(selectUserStmt, map); LOG.info(sql); - Statement stmt = null; - ResultSet rs = null; - try { - // execute query - stmt = createConnection().createStatement(); - rs = stmt.executeQuery(sql); - + // execute query + try (Statement stmt = createConnection().createStatement(); + ResultSet rs = stmt.executeQuery(sql)) { // populate user object BaseUser thisUser = null; + if (rs.next()) { thisUser = new BaseUser(); thisUser.setName(rs.getString(ATTR_LOGIN)); @@ -512,6 +444,7 @@ public class DbUserManager extends AbstractUserManager { thisUser.setMaxIdleTime(rs.getInt(ATTR_MAX_IDLE_TIME)); List<Authority> authorities = new ArrayList<>(); + if (rs.getBoolean(ATTR_WRITE_PERM)) { authorities.add(new WritePermission()); } @@ -525,11 +458,8 @@ public class DbUserManager extends AbstractUserManager { thisUser.setAuthorities(authorities); } - return thisUser; - } finally { - closeQuitely(rs); - closeQuitely(stmt); + return thisUser; } } @@ -539,8 +469,6 @@ public class DbUserManager extends AbstractUserManager { * {@inheritDoc} */ public User getUserByName(String name) throws FtpException { - Statement stmt = null; - ResultSet rs = null; try { BaseUser user = selectUserByName(name); @@ -551,14 +479,9 @@ public class DbUserManager extends AbstractUserManager { } return user; - - } catch (SQLException ex) { LOG.error("DbUserManager.getUserByName()", ex); throw new FtpException("DbUserManager.getUserByName()", ex); - } finally { - closeQuitely(rs); - closeQuitely(stmt); } } @@ -568,26 +491,19 @@ public class DbUserManager extends AbstractUserManager { * {@inheritDoc} */ public boolean doesExist(String name) throws FtpException { - Statement stmt = null; - ResultSet rs = null; - try { - - // create the sql - HashMap<String, Object> map = new HashMap<>(); - map.put(ATTR_LOGIN, escapeString(name)); - String sql = StringUtils.replaceString(selectUserStmt, map); - LOG.info(sql); + // create the sql + HashMap<String, Object> map = new HashMap<>(); + map.put(ATTR_LOGIN, escapeString(name)); + String sql = StringUtils.replaceString(selectUserStmt, map); + LOG.info(sql); - // execute query - stmt = createConnection().createStatement(); - rs = stmt.executeQuery(sql); + // execute query + try (Statement stmt = createConnection().createStatement(); + ResultSet rs = stmt.executeQuery(sql)) { return rs.next(); } catch (SQLException ex) { LOG.error("DbUserManager.doesExist()", ex); throw new FtpException("DbUserManager.doesExist()", ex); - } finally { - closeQuitely(rs); - closeQuitely(stmt); } } @@ -597,31 +513,24 @@ public class DbUserManager extends AbstractUserManager { * {@inheritDoc} */ public String[] getAllUserNames() throws FtpException { + // create sql query + String sql = selectAllStmt; + LOG.info(sql); - Statement stmt = null; - ResultSet rs = null; - try { - - // create sql query - String sql = selectAllStmt; - LOG.info(sql); - - // execute query - stmt = createConnection().createStatement(); - rs = stmt.executeQuery(sql); - + // execute query + try (Statement stmt = createConnection().createStatement(); + ResultSet rs = stmt.executeQuery(sql)) { // populate list ArrayList<String> names = new ArrayList<>(); + while (rs.next()) { names.add(rs.getString(ATTR_LOGIN)); } + return names.toArray(new String[0]); } catch (SQLException ex) { LOG.error("DbUserManager.getAllUserNames()", ex); throw new FtpException("DbUserManager.getAllUserNames()", ex); - } finally { - closeQuitely(rs); - closeQuitely(stmt); } } @@ -630,8 +539,7 @@ public class DbUserManager extends AbstractUserManager { * * {@inheritDoc} */ - public User authenticate(Authentication authentication) - throws AuthenticationFailedException { + public User authenticate(Authentication authentication) throws AuthenticationFailedException { if (authentication instanceof UsernamePasswordAuthentication) { UsernamePasswordAuthentication upauth = (UsernamePasswordAuthentication) authentication; @@ -646,43 +554,33 @@ public class DbUserManager extends AbstractUserManager { password = ""; } - Statement stmt = null; - ResultSet rs = null; - try { - - // create the sql query - HashMap<String, Object> map = new HashMap<>(); - map.put(ATTR_LOGIN, escapeString(user)); - String sql = StringUtils.replaceString(authenticateStmt, map); - LOG.info(sql); + // create the sql query + HashMap<String, Object> map = new HashMap<>(); + map.put(ATTR_LOGIN, escapeString(user)); + String sql = StringUtils.replaceString(authenticateStmt, map); + LOG.info(sql); - // execute query - stmt = createConnection().createStatement(); - rs = stmt.executeQuery(sql); + // execute query + try (Statement stmt = createConnection().createStatement(); + ResultSet rs = stmt.executeQuery(sql)) { if (rs.next()) { try { String storedPassword = rs.getString(ATTR_PASSWORD); + if (getPasswordEncryptor().matches(password, storedPassword)) { return getUserByName(user); } else { - throw new AuthenticationFailedException( - "Authentication failed"); + throw new AuthenticationFailedException("Authentication failed"); } } catch (FtpException e) { - throw new AuthenticationFailedException( - "Authentication failed", e); + throw new AuthenticationFailedException("Authentication failed", e); } } else { - throw new AuthenticationFailedException( - "Authentication failed"); + throw new AuthenticationFailedException("Authentication failed"); } } catch (SQLException ex) { LOG.error("DbUserManager.authenticate()", ex); - throw new AuthenticationFailedException( - "Authentication failed", ex); - } finally { - closeQuitely(rs); - closeQuitely(stmt); + throw new AuthenticationFailedException("Authentication failed", ex); } } else if (authentication instanceof AnonymousAuthentication) { try { @@ -695,12 +593,10 @@ public class DbUserManager extends AbstractUserManager { } catch (AuthenticationFailedException e) { throw e; } catch (FtpException e) { - throw new AuthenticationFailedException( - "Authentication failed", e); + throw new AuthenticationFailedException("Authentication failed", e); } } else { - throw new IllegalArgumentException( - "Authentication not supported by this user manager"); + throw new IllegalArgumentException("Authentication not supported by this user manager"); } } @@ -713,15 +609,20 @@ public class DbUserManager extends AbstractUserManager { } StringBuilder valBuf = new StringBuilder(input); - for (int i = 0; i < valBuf.length(); i++) { - char ch = valBuf.charAt(i); - if (ch == '\'' || ch == '\\' || ch == '$' || ch == '^' || ch == '[' - || ch == ']' || ch == '{' || ch == '}') { - valBuf.insert(i, '\\'); - i++; + for (int i = 0; i < valBuf.length(); i++) { + switch (valBuf.charAt(i)) { + case '\'': + case '\\': + case '$': + case '[': + case ']': + case '{': + case '}': + valBuf.insert(i++, '\\'); } } + return valBuf.toString(); } }