[Tickets #12837] Re: Imap_Client: Leaking IMAP socket on error

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12837
------------------------------------------------------------------------------
  Ticket             | 12837
  Updated By         | Michael Slusarz <slusarz at horde.org>
  Summary            | Imap_Client: Leaking IMAP socket on error
  Queue              | Horde Framework Packages
  Version            | Git master
  Type               | Bug
-State              | Unconfirmed
+State              | Feedback
  Priority           | 1. Low
  Milestone          |
  Patch              |
  Owners             |
------------------------------------------------------------------------------


Michael Slusarz <slusarz at horde.org> (2013-11-14 13:21) wrote:

> When an IMAP auth error is thrown, the socket to the IMAP server is  
> not closed.

That is correct.  An authorization error shouldn't close the  
connection.  It just indicates that the transition between  
non-authenticated -> authenticated state was not successful.  But at  
the protocol level you are not disconnected from the IMAP server and,  
in fact, the IMAP server remains willing/able to process further  
commands.

For example, in an interactive environment - such as a CLI script -  
there is no need to destroy the IMAP connection.  If, after an  
unsuccessful authentication attempt, the CLI can ask for another  
password and we should be reusing the same connection for  
authentication the next time we try to authenticate.

> I've created the attached example script to demonstrate the problem.

I'm confused by this script.  First, it's going to continually loop  
whether or not authentication is correct or not.  So I'm not sure what  
the outer while() loop is attempting to show.

Second, you are not distinguishing between what the error is from  
listMailboxes().  listMailboxes() may fail because of an  
authentication error.  But it also may fail because of invalid input.   
Or because of a temporary error on the IMAP server.  Or because you  
don't have the correct ACL permissions.  Simply checking for an  
Exception in this case is insufficient.

It sounds like this is more of a logic error in the Kolab code.  For  
starters, the Horde_Imap_Client instantiation code should only occur  
once and should be disconnected from any of the actions on the object  
itself.  And if an IMAP action throws an exception, you surely need to  
break any loop containing that IMAP action.  But this doesn't have  
anything to do with authentication errors per se.  That just seems to  
be deficient exception handling in general.

This could be a failure of the documentation to explain the different  
types of Exceptions.  But it doesn't seem to be a design error in  
Imap_Client.

> Could we close the socket on error?

You can do it manually by calling logout().  But see above: there  
shouldn't be any reason to.  See, e.g., IMP: we never call logout().