Issue Details (XML | Word | Printable)

Key: LDAP-113
Type: Bug Bug
Status: Closed Closed
Resolution: Invalid
Priority: Major Major
Assignee: Unassigned
Reporter: Jakub Jozwicki
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Spring LDAP

Incorrect handling of PartialResultException in ldap.core.LdapTemplate

Created: 10/Jun/08 02:14 AM   Updated: 11/Oct/08 10:48 AM   Resolved: 11/Oct/08 10:48 AM
Component/s: None
Affects Version/s: 1.2.1
Fix Version/s: 1.3.0-RC1

Time Tracking:
Not Specified

Environment: MS AD @ W2K3
Issue Links:
Related
 


 Description  « Hide

Code in ldap.core.LdapTemplate is wrong, because while(results.hasMore())
is inside try/catch(PartialResultException), and exception breaks while loop,
but it shouldn't. The try/catch(PartialResultException) should be located inside the while loop.
Like this:

public void search(SearchExecutor se, NameClassPairCallbackHandler handler,
DirContextProcessor processor) {
DirContext ctx = contextSource.getReadOnlyContext();

NamingEnumeration results = null;
RuntimeException ex = null;
try {
processor.preProcess(ctx);
results = se.executeSearch(ctx);
log.debug("results are="+results);

while(true) {
try { if (!results.hasMore()) break; NameClassPair result = (NameClassPair) results.next(); log.debug("result="+result); log.debug("result="+result.getName()+", "+result.getClassName()); handler.handleNameClassPair(result); }
catch (PartialResultException e) {
if (ignorePartialResultException) { log.debug("PartialResultException encountered and ignored: ["+e.getResolvedName()+"] "+ e.getRemainingName()); } else { ex = LdapUtils.convertLdapException(e); }
continue;
}
catch (NullPointerException npe) { break; }
}
}
catch (NameNotFoundException e) { // The base context was not found, which basically means // that the search did not return any results. Just clean up and // exit. // Note that this may present problems if a DirContextProcessor was // supplied - there's no guarantee that the postProcess() operation // will go well after a NamingException has been thrown. It is // however quite possible that information will be available for // retrieval either way. e.printStackTrace(); }
catch (javax.naming.NamingException e) { ex = LdapUtils.convertLdapException(e); }
finally {
try { processor.postProcess(ctx); } catch (javax.naming.NamingException e) {
if (ex == null) { ex = LdapUtils.convertLdapException(e); } else { // We already had an exception from above and should ignore // this one. log.debug("Ignoring Exception from postProcess, " + "main exception thrown instead", e); }
}
closeContextAndNamingEnumeration(ctx, results);
// If we got an exception it should be thrown.
if (ex != null) { throw ex; }
}
}

see also: http://jira.springframework.org/browse/SEC-832



Mattias Arthursson added a comment - 10/Jun/08 02:33 AM

Are you saying that we may get more results after a PartialResultException has been thrown? If that is true our code should reflect that.

The javadocs for PartialResultException seems to indicate otherwise (though, admittedly, it's far from clear):
"This exception is thrown to indicate that the result being returned or returned so far is partial, and that the operation cannot be completed."


Jakub Jozwicki added a comment - 10/Jun/08 06:41 AM

Yes. I have seen this situation with MS AD 2003.


Mattias Arthursson added a comment - 10/Jun/08 12:14 PM

Interesting. Could you please provide some plain JNDI code that shows what happens (i.e. when the exceptions are thrown, how the results are affected, and corresponding output)?


Luke Taylor added a comment - 18/Aug/08 04:35 PM

I'm just testing this against an AD installation in relation to SEC-832 (the original bug). I particularly don't like the

catch (NullPointerException npe) { break; }

clause, which seems like a total hack, ignoring some NPE that is thrown internally by the Sun LDAP provider (I don't know why the NPE is thrown exactly). Furthermore, in my tests, when results.hasMore() is called after ignoring the PartialResultException, I always get the NPE on the next call to hasMore(), which means the loop breaks immediately anyway. Can you explain in what circumstances you see the loop continuing, and how you can guarantee that you won't get a NPE prematurely with this code, causing you to miss further search results?


Jakub Jozwicki added a comment - 21/Aug/08 05:36 AM

Maybe we should close this bug. On clean install of W2K3 R2 + Windows Update, after PartialResultException no more data is returned.


Ulrik Sandberg added a comment - 11/Oct/08 10:48 AM

Closed on the request of the issue reporter.