From 301adb550d1d45bbbc6344252dd9cf1c628122d3 Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 26 Sep 2012 17:10:31 -0400 Subject: [PATCH] Fix LDAP connection leak (issue 139) --- docs/04_releases.mkd | 1 + src/com/gitblit/LdapUserService.java | 71 +++++++++++++++------------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/docs/04_releases.mkd b/docs/04_releases.mkd index 7f0feece..0ad5879d 100644 --- a/docs/04_releases.mkd +++ b/docs/04_releases.mkd @@ -11,6 +11,7 @@ If you are updating from an earlier release AND you have indexed branches with t #### fixes +- Fixed connection leak in LDAPUserService (issue 139) - Fixed bug in commit page where changes to a submodule threw a null pointer exception (issue 132) - Fixed bug in the diff view for filenames that have non-ASCII characters (issue 128) diff --git a/src/com/gitblit/LdapUserService.java b/src/com/gitblit/LdapUserService.java index 54a55752..9ce18f6d 100644 --- a/src/com/gitblit/LdapUserService.java +++ b/src/com/gitblit/LdapUserService.java @@ -160,48 +160,51 @@ public class LdapUserService extends GitblitUserService { public UserModel authenticate(String username, char[] password) { String simpleUsername = getSimpleUsername(username); - LDAPConnection ldapConnection = getLdapConnection(); + LDAPConnection ldapConnection = getLdapConnection(); if (ldapConnection != null) { - // Find the logging in user's DN - String accountBase = settings.getString(Keys.realm.ldap.accountBase, ""); - String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))"); - accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(simpleUsername)); + try { + // Find the logging in user's DN + String accountBase = settings.getString(Keys.realm.ldap.accountBase, ""); + String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))"); + accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(simpleUsername)); - SearchResult result = doSearch(ldapConnection, accountBase, accountPattern); - if (result != null && result.getEntryCount() == 1) { - SearchResultEntry loggingInUser = result.getSearchEntries().get(0); - String loggingInUserDN = loggingInUser.getDN(); - - if (isAuthenticated(ldapConnection, loggingInUserDN, new String(password))) { - logger.debug("LDAP authenticated: " + username); - - UserModel user = getUserModel(simpleUsername); - if (user == null) // create user object for new authenticated user - user = new UserModel(simpleUsername); + SearchResult result = doSearch(ldapConnection, accountBase, accountPattern); + if (result != null && result.getEntryCount() == 1) { + SearchResultEntry loggingInUser = result.getSearchEntries().get(0); + String loggingInUserDN = loggingInUser.getDN(); - // create a user cookie - if (StringUtils.isEmpty(user.cookie) && !ArrayUtils.isEmpty(password)) { - user.cookie = StringUtils.getSHA1(user.username + new String(password)); - } - - if (!supportsTeamMembershipChanges()) - getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user); - - // Get User Attributes - setUserAttributes(user, loggingInUser); + if (isAuthenticated(ldapConnection, loggingInUserDN, new String(password))) { + logger.debug("LDAP authenticated: " + username); + + UserModel user = getUserModel(simpleUsername); + if (user == null) // create user object for new authenticated user + user = new UserModel(simpleUsername); - // Push the ldap looked up values to backing file - super.updateUserModel(user); - if (!supportsTeamMembershipChanges()) { - for (TeamModel userTeam : user.teams) - updateTeamModel(userTeam); + // create a user cookie + if (StringUtils.isEmpty(user.cookie) && !ArrayUtils.isEmpty(password)) { + user.cookie = StringUtils.getSHA1(user.username + new String(password)); + } + + if (!supportsTeamMembershipChanges()) + getTeamsFromLdap(ldapConnection, simpleUsername, loggingInUser, user); + + // Get User Attributes + setUserAttributes(user, loggingInUser); + + // Push the ldap looked up values to backing file + super.updateUserModel(user); + if (!supportsTeamMembershipChanges()) { + for (TeamModel userTeam : user.teams) + updateTeamModel(userTeam); + } + + return user; } - - return user; } + } finally { + ldapConnection.close(); } } - return null; } -- 2.39.5