[jira] [Comment Edited] (CAMEL-8718) Connection leak with ftp consumer and invalid credentials

["Karsten Vileid (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-8718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14528600#comment-14528600 ] 

Karsten Vileid edited comment on CAMEL-8718 at 5/5/15 3:21 PM:
---------------------------------------------------------------

I am working in a project that has been hit by this problem with leaking sockets, and great to see that problem is attacked.
However I have some comments to the chosen solution.

1. In FtpBadLoginConnectionLeakTest.java using maximumReconnectAttempts=3 (which is camel default), causes leaking sockets, and hence test to fail.

2. In RemoteFileConsumer.java. Some FTP servers (at least one of ours), responds OK to getOperations().sendNoop(), even though user has not been authorized.
This again causes leaking sockets where;
            isConnected = getOperations().sendNoop();  // isConnected is true

and loggedIn is always false (since credentials are wrong in this case), causing new connect, without disconnecting previous connection, for every poll.

        if (!loggedIn || !isConnected) {
            loggedIn = getOperations().connect((RemoteFileConfiguration) endpoint.getConfiguration());

3. Problem with leaking connections is also seen for Ftp producers, suggesting that solution could be done in common part for consumers and producers.

----------

Possible solutions:
1. In FtpOperations.java could disconnect if login failed

{code}
if (!login) {
    throw new GenericFileOperationFailedException(client.getReplyCode(), client.getReplyString());
}
{code}

Changed to:

{code}
if (!login) {
    client.disconnect();
    throw new GenericFileOperationFailedException(client.getReplyCode(), client.getReplyString());
}
{code}

2. Disconnect regardless of what goes wrong, maybe a more robust solution. In FtpOperations.java

{code}
public boolean connect(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {
{code}

Changed to:

{code}
public boolean connect(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {
    try {
        return connectInternal(configuration);
    } catch (GenericFileOperationFailedException e) {
        if (isConnected()) {
            disconnect();
        }
        throw e;
    }
}
   
private boolean connectInternal(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {
{code}



was (Author: kvileid@yahoo.com):
I am working in a project that has been hit by this problem with leaking sockets, and great to see that problem is attacked.
However I have some comments to the chosen solution.

1. In FtpBadLoginConnectionLeakTest.java using maximumReconnectAttempts=3 (which is camel default), causes leaking sockets, and hence test to fail.

2. In RemoteFileConsumer.java. Some FTP servers (at least one of ours), responds OK to getOperations().sendNoop(), even though user has not been authorized.
This again causes leaking sockets where;
            isConnected = getOperations().sendNoop();  // isConnected is true

and loggedIn is always false (since credentials are wrong in this case), causing new connect, without disconnecting previous connection, for every poll.

        if (!loggedIn || !isConnected) {
            loggedIn = getOperations().connect((RemoteFileConfiguration) endpoint.getConfiguration());

3. Problem with leaking connections is also seen for Ftp producers, suggesting that solution could be done in common part for consumers and producers.

----------

Possible solutions:
1. In FtpOperations.java could disconnect if login failed
            if (!login) {
                throw new GenericFileOperationFailedException(client.getReplyCode(), client.getReplyString());
            }

Changed to:
            if (!login) {
                client.disconnect();
                throw new GenericFileOperationFailedException(client.getReplyCode(), client.getReplyString());
            }


2. Disconnect regardless of what goes wrong, maybe a more robust solution. In FtpOperations.java
    public boolean connect(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {

Changed to:
    public boolean connect(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {
        try {
            return connectInternal(configuration);
        } catch (GenericFileOperationFailedException e) {
            if (isConnected()) {
                disconnect();
            }
            throw e;
        }
    }
   
    private boolean connectInternal(RemoteFileConfiguration configuration) throws GenericFileOperationFailedException {


> Connection leak with ftp consumer and invalid credentials
> ---------------------------------------------------------
>
>                 Key: CAMEL-8718
>                 URL: https://issues.apache.org/jira/browse/CAMEL-8718
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-ftp
>    Affects Versions: 2.12.5, 2.13.4, 2.14.2, 2.15.1
>            Reporter: Grzegorz Grzybek
>            Assignee: Grzegorz Grzybek
>             Fix For: 2.14.3, 2.16.0, 2.15.3
>
>
> In each iteration new socket connection is created, but never closed. This leads to growth of {{ESTABLISHED}} connections to FTP server.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)