[jira] Created: (HTTPCLIENT-886) Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler

Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
------------------------------------------------------------------------------

                 Key: HTTPCLIENT-886
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
             Project: HttpComponents HttpClient
          Issue Type: Bug
    Affects Versions: 3.1 Final
            Reporter: Sylvain Laurent
            Priority: Critical
             Fix For: 3.1.1


We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.

Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.

I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :

        if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
            // Retry if the server dropped connection on us AND the request was not sent
            return true;
        }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773475#action_12773475 ]

Ortwin Glück commented on HTTPCLIENT-886:
-----------------------------------------

> In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.

If a repeated request is "very dangerous" you'd better implementing detection for duplicate requests in your application! Use challenge/reponse tokens, sequence numbers, timestamps or other means to detect repeated requests. The behaviour of HttpClient is not the only source for repeated requests. Messaging over any unreliable medium (like a network) ALWAYS has to make the choice between either loosing some messages, or accepting and dealing with duplicates. This is a fundamental theorem. It's the reason for why there is UDP and TCP and their different guarantees.

If the client doesn't get a response it can never know whether the request didn't reach the server or the response just didn't reach the client. By not retrying the request, the client risks that the request never reached the destination. It HAS TO retry the request until it gets a response. This algorithm is called Idle-RQ and is used for TCP for instance.

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>            Priority: Critical
>             Fix For: 3.1.1
>
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ortwin Glück resolved HTTPCLIENT-886.
-------------------------------------

    Resolution: Invalid

isRequestSent() doesn't mean that the request has actually reached the server.

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>            Priority: Critical
>             Fix For: 3.1.1
>
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ortwin Glück updated HTTPCLIENT-886:
------------------------------------

    Fix Version/s:     (was: 3.1.1)

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>            Priority: Critical
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773482#action_12773482 ]

Sylvain Laurent commented on HTTPCLIENT-886:
--------------------------------------------

I agree with you about implementing detection for duplicate, but I don't agree that the behavior in case of NoHttpResponseException should be to retry. As my case shows, there is a possibility that the request was actually sent to the back-end, so no retry should be attempted (better safe than sorry).
In the case of a java.net.ConnectException it' safe to retry because the client did not send the request, and we are sure of it.

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>            Priority: Critical
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Laurent updated HTTPCLIENT-886:
---------------------------------------

    Priority: Major  (was: Critical)

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Laurent reopened HTTPCLIENT-886:
----------------------------------------

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773493#action_12773493 ]

Ortwin Glück commented on HTTPCLIENT-886:
-----------------------------------------

NoHttpResponseException is only ever thrown (from readStatusLine()) after the response has been sent. So the conditon would never be true, actually. Can we think of ANY scenario where retrying is the right thing to do when we get no response from the server?

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCLIENT-886.
------------------------------------------

    Resolution: Invalid

(1) There will be no fixes to HttpClient 3.x other than for severe security related issues. Please do not re-open the issue unless the problem can be reproduced with HttpClient 4.0

(2) It is not a problem of HttpClient if your application abuses HTTP protocol for something it has never been intended for, such as transaction processing.

Oleg

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773518#action_12773518 ]

Sylvain Laurent commented on HTTPCLIENT-886:
--------------------------------------------

I understand the fact that there are no longer any bug-fix for 3.x.

I cannot test with 4.x at the moment, but I definitely think that http client should NOT retry in such a case, regardless of the application and its intended usage of http protocol...

Sylvain

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCLIENT-886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773520#action_12773520 ]

Ortwin Glück commented on HTTPCLIENT-886:
-----------------------------------------

Feel free to implement your own retry handler. That's why it's customizable.

> Incorrect handling of NoHttpResponseException in DefaultHttpMethodRetryHandler
> ------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-886
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-886
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 3.1 Final
>            Reporter: Sylvain Laurent
>
> We have a buggy load-balancer that sometimes forwards the request to the back-end server, and then close the connections without returning any data at all.
> This provokes a NoHttpResponseException inside commons http client 3.1, which is handled by DefaultHttpMethodRetryHandler.
> Unfortunately DefaultHttpMethodRetryHandler handles this NoHttpResponseException by asking for a retry. In our case, this is very dangerous because the request has already been sent and is being processed by the back-end server.
> I think that in the case of a NoHttpResponseException the test !method.isRequestSent() shoud be done :
>         if (exception instanceof NoHttpResponseException && !method.isRequestSent()) {
>             // Retry if the server dropped connection on us AND the request was not sent
>             return true;
>         }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...