HttpComponents HttpCore
  1. HttpComponents HttpCore
  2. HTTPCORE-236

Failed to receive last chunk of the large https response through httpcore-nio

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 4.1-beta2
    • Fix Version/s: 4.1
    • Component/s: HttpCore NIO
    • Labels:
      None
    • Environment:
      java 1.6_06 windows xp, linux redhat.

      Description

      I'm a wso2 esb (synapse) user/developer.
      I got a problem with https nio transport.
      I tried versions 4.1 alpha1 and 4.1 beta2.

      The problem:
      transport failed to receive large (50k) response through nio https.
      The connection is keep-alive and chunked. Every time I received only response divisible
      by 8192 and last chunk is never returned.

      After digging the logs and code I found that last chunk not returned because
      of the code in this class org.apache.http.impl.nio.reactor.SSLIOSession.java

      I changed one line in the code and now it's working.
      Now I want to ask somebody who knows the code if this change is ok,
      and maybe it should be applied to httpcore (see attached file)

      1. SSLIOSession.diff
        0.7 kB
        Dmitry
      2. SSLClientIOEventDispatch.diff
        2 kB
        Dmitry

        Activity

        Hide
        Dmitry added a comment -

        org.apache.http.impl.nio.reactor.SSLIOSession.java

        public synchronized boolean isAppInputReady() throws IOException {
        int bytesRead = receiveEncryptedData();
        if (bytesRead == -1)

        { this.endOfStream = true; }

        doHandshake();
        decryptData();
        // Some decrypted data is available or at the end of stream
        return (this.appEventMask & SelectionKey.OP_READ) > 0
        && (this.inPlain.position() > 0 || (this.endOfStream && this.status == ACTIVE)

        this.appBufferStatus.hasBufferedInput() );
        }
        Show
        Dmitry added a comment - org.apache.http.impl.nio.reactor.SSLIOSession.java public synchronized boolean isAppInputReady() throws IOException { int bytesRead = receiveEncryptedData(); if (bytesRead == -1) { this.endOfStream = true; } doHandshake(); decryptData(); // Some decrypted data is available or at the end of stream return (this.appEventMask & SelectionKey.OP_READ) > 0 && (this.inPlain.position() > 0 || (this.endOfStream && this.status == ACTIVE) this.appBufferStatus.hasBufferedInput() ); }
        Hide
        Oleg Kalnichevski added a comment -

        Patch checked in with some minor tweaks. Many thanks, Dmitry, for contributing it.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Patch checked in with some minor tweaks. Many thanks, Dmitry, for contributing it. Oleg
        Hide
        Dmitry added a comment - - edited

        I was too fast to tell that this patch solves the problem.
        I found another case when buffered input "eats" last chunk.

        The method inputReady in the class SSLClientIOEventDispatch.java

        This method calls conn.consumeInput() only when
        sslSession.isAppInputReady() returns true.

        But actually conn (NHttpClientIOTarget) could be buffered. In my case it's a DefaultNHttpClientConnection.

        So, I suggest additional change (probably it could be simplified):

        public void inputReady(final IOSession session) {
        NHttpClientIOTarget conn =
        (NHttpClientIOTarget) session.getAttribute(ExecutionContext.HTTP_CONNECTION);
        SSLIOSession sslSession =
        (SSLIOSession) session.getAttribute(SSL_SESSION);

        try {
        if(conn instanceof SessionBufferStatus){
        //we have buffered NHttpClientIOTarget, let,s check buffer also...
        boolean hasBufferedInput = hasBufferedInput=((SessionBufferStatus)conn).hasBufferedInput();
        //DLukyanov: i'm not sure that the next "if" operator is necessary...
        // better will be if NHttpClientIOTarget itself will decide if consume will be effective or not.
        if (sslSession.isAppInputReady() || hasBufferedInput)

        { conn.consumeInput(this.handler); }

        }else

        { //just try to consume. we can't guess if it's buffered or not conn.consumeInput(this.handler); }

        //ORIGINAL CODE:
        //if (sslSession.isAppInputReady())

        { // conn.consumeInput(this.handler); //}

        sslSession.inboundTransport();
        } catch (IOException ex)

        { this.handler.exception(conn, ex); sslSession.shutdown(); }

        }

        Show
        Dmitry added a comment - - edited I was too fast to tell that this patch solves the problem. I found another case when buffered input "eats" last chunk. The method inputReady in the class SSLClientIOEventDispatch.java This method calls conn.consumeInput() only when sslSession.isAppInputReady() returns true. But actually conn (NHttpClientIOTarget) could be buffered. In my case it's a DefaultNHttpClientConnection. So, I suggest additional change (probably it could be simplified): public void inputReady(final IOSession session) { NHttpClientIOTarget conn = (NHttpClientIOTarget) session.getAttribute(ExecutionContext.HTTP_CONNECTION); SSLIOSession sslSession = (SSLIOSession) session.getAttribute(SSL_SESSION); try { if(conn instanceof SessionBufferStatus){ //we have buffered NHttpClientIOTarget, let,s check buffer also... boolean hasBufferedInput = hasBufferedInput=((SessionBufferStatus)conn).hasBufferedInput(); //DLukyanov: i'm not sure that the next "if" operator is necessary... // better will be if NHttpClientIOTarget itself will decide if consume will be effective or not. if (sslSession.isAppInputReady() || hasBufferedInput) { conn.consumeInput(this.handler); } }else { //just try to consume. we can't guess if it's buffered or not conn.consumeInput(this.handler); } //ORIGINAL CODE: //if (sslSession.isAppInputReady()) { // conn.consumeInput(this.handler); //} sslSession.inboundTransport(); } catch (IOException ex) { this.handler.exception(conn, ex); sslSession.shutdown(); } }
        Hide
        Oleg Kalnichevski added a comment -

        This just does not look right. The DefaultNHttpClientConnection passes an instance of SessionBufferStatus it implements to the I/O session object at the construction time. So, the I/O session should always be aware of the data in the connection's buffers. If there is some buffered input the SSLIOSession#isAppInputReady() method should always return true.

        Please try to reproduce the problem with a test case. Otherwise, I will have to put the issue back to RESLOVED.

        Oleg

        Show
        Oleg Kalnichevski added a comment - This just does not look right. The DefaultNHttpClientConnection passes an instance of SessionBufferStatus it implements to the I/O session object at the construction time. So, the I/O session should always be aware of the data in the connection's buffers. If there is some buffered input the SSLIOSession#isAppInputReady() method should always return true. Please try to reproduce the problem with a test case. Otherwise, I will have to put the issue back to RESLOVED. Oleg
        Hide
        Dmitry added a comment -

        give me two days

        Show
        Dmitry added a comment - give me two days
        Hide
        Dmitry added a comment -

        Oleg, You are right. There was a problem on my side. Let's mark it as RESOLVED.
        Regards/

        Show
        Dmitry added a comment - Oleg, You are right. There was a problem on my side. Let's mark it as RESOLVED. Regards/

          People

          • Assignee:
            Unassigned
            Reporter:
            Dmitry
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 120h
              120h
              Remaining:
              Remaining Estimate - 120h
              120h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development