Fix LDAP connection leak (issue 139)

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)
 

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;		
 	}