Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 5.1
    • 5.1.3, 5.2-beta1
    • HttpCore
    • None

    Description

      current implemention in version 5.1, when connection gracefully shutdown and receives a single HEADERS frame with END_HEADERS flag set, it will throws "Stream refused" exception, not sure about this. 

      As RFC7540 section 4.3 defines, looks like single HEADERS frame should be consumed.

       

      This code in org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:1073

       

      Attachments

        Activity

          Commit 7ae878d018f630cd635662c8954924200614b830 in httpcomponents-core's branch refs/heads/github_ci_on_win from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=7ae878d01 ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit 7ae878d018f630cd635662c8954924200614b830 in httpcomponents-core's branch refs/heads/github_ci_on_win from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=7ae878d01 ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.
          olegk Oleg Kalnichevski made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Resolved [ 5 ]

          Commit 8629781127259e1c337ac9079f825eaaa6ba6601 in httpcomponents-core's branch refs/heads/master from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=8629781 ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit 8629781127259e1c337ac9079f825eaaa6ba6601 in httpcomponents-core's branch refs/heads/master from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=8629781 ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          Commit 7ae878d018f630cd635662c8954924200614b830 in httpcomponents-core's branch refs/heads/5.1.x from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=7ae878d ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit 7ae878d018f630cd635662c8954924200614b830 in httpcomponents-core's branch refs/heads/5.1.x from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=7ae878d ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.
          yinwoods yinwoods added a comment -

          I think it's OK, Really appreciate it.Oleg Kalnichevski

          yinwoods yinwoods added a comment - I think it's OK, Really appreciate it. Oleg Kalnichevski

          > I have only one question, after receiving a GOAWAY frame, could receive other frame like HEADRES?

          yinwoods In my opintion the sender of GOAWAY may not send HEADRES or PUSH_PROMISE anymore, but it can receive and discard HEADRES for new streams or PUSH_PROMISE from the oppositve endpoint after minimal processing. The receiver of GOAWAY should not receive HEADRES for new streams or PUSH_PROMISE from the opposite endpoint.

          Please review the updated patch:

          https://github.com/apache/httpcomponents-core/commit/8593d4b6a227389fd12ae2a11138d3cb811eb9be

          Oleg

          olegk Oleg Kalnichevski added a comment - > I have only one question, after receiving a GOAWAY frame, could receive other frame like HEADRES? yinwoods In my opintion the sender of GOAWAY may not send HEADRES or PUSH_PROMISE anymore, but it can receive and discard HEADRES for new streams or PUSH_PROMISE from the oppositve endpoint after minimal processing. The receiver of GOAWAY should not receive HEADRES for new streams or PUSH_PROMISE from the opposite endpoint. Please review the updated patch: https://github.com/apache/httpcomponents-core/commit/8593d4b6a227389fd12ae2a11138d3cb811eb9be Oleg

          Commit 8593d4b6a227389fd12ae2a11138d3cb811eb9be in httpcomponents-core's branch refs/heads/HTTPCORE-696 from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=8593d4b ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit 8593d4b6a227389fd12ae2a11138d3cb811eb9be in httpcomponents-core's branch refs/heads/ HTTPCORE-696 from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=8593d4b ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.
          yinwoods yinwoods added a comment -

          Oleg Kalnichevski Thanks for your proposed fix, I have only one question, after receiving a GOAWAY frame, could receive other frame like HEADRES? I made a comment in https://github.com/apache/httpcomponents-core/commit/e8a44a4286bd4f52df6c0877b51badb55bd3596d, Please check.

          yinwoods yinwoods added a comment - Oleg Kalnichevski Thanks for your proposed fix, I have only one question, after receiving a GOAWAY frame, could receive other frame like HEADRES? I made a comment in https://github.com/apache/httpcomponents-core/commit/e8a44a4286bd4f52df6c0877b51badb55bd3596d, Please check.

          Commit 3360fc3adcb79a02dcede64ce8b06f6965873b35 in httpcomponents-core's branch refs/heads/HTTPCORE-696 from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=3360fc3 ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit 3360fc3adcb79a02dcede64ce8b06f6965873b35 in httpcomponents-core's branch refs/heads/ HTTPCORE-696 from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=3360fc3 ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.
          olegk Oleg Kalnichevski added a comment - yinwoods Please review / test the proposed fix: https://github.com/apache/httpcomponents-core/commit/e8a44a4286bd4f52df6c0877b51badb55bd3596d https://github.com/apache/httpcomponents-core/commits/HTTPCORE-696 Oleg

          Commit e8a44a4286bd4f52df6c0877b51badb55bd3596d in httpcomponents-core's branch refs/heads/HTTPCORE-696 from Oleg Kalnichevski
          [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=e8a44a4 ]

          HTTPCORE-696: H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.

          jira-bot ASF subversion and git services added a comment - Commit e8a44a4286bd4f52df6c0877b51badb55bd3596d in httpcomponents-core's branch refs/heads/ HTTPCORE-696 from Oleg Kalnichevski [ https://gitbox.apache.org/repos/asf?p=httpcomponents-core.git;h=e8a44a4 ] HTTPCORE-696 : H2 protocol handler to ensure minimal processing of incoming frames during a graceful shutdown.
          olegk Oleg Kalnichevski made changes -
          Fix Version/s 5.1.3 [ 12350658 ]
          Fix Version/s 5.2-alpha3 [ 12350708 ]
          olegk Oleg Kalnichevski made changes -
          Assignee Oleg Kalnichevski [ olegk ]
          Resolution Invalid [ 6 ]
          Status Closed [ 6 ] Reopened [ 4 ]

          yinwoods My apologoies. I overlooked this line that does look wrong. Due to this check HEADERS and PUSH_PROMISE frames do not get processed. I am looking into the issue.

          https://github.com/apache/httpcomponents-core/blob/5.1.x/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java#L725

          Oleg

          olegk Oleg Kalnichevski added a comment - yinwoods My apologoies. I overlooked this line that does look wrong. Due to this check HEADERS and PUSH_PROMISE frames do not get processed. I am looking into the issue. https://github.com/apache/httpcomponents-core/blob/5.1.x/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java#L725 Oleg
          yinwoods yinwoods made changes -
          Resolution Invalid [ 6 ]
          Status Open [ 1 ] Closed [ 6 ]
          yinwoods yinwoods added a comment -

          Never mind, I think it's my problem, I will close this issuse. Thanks anyway.

          yinwoods yinwoods added a comment - Never mind, I think it's my problem, I will close this issuse. Thanks anyway.

          yinwoods I am not sure I understand the problem. What protocol requirement does HttpCore fail to comply with?

          Oleg

          olegk Oleg Kalnichevski added a comment - yinwoods I am not sure I understand the problem. What protocol requirement does HttpCore fail to comply with? Oleg
          yinwoods yinwoods added a comment - - edited

          In Section 6.8 https://datatracker.ietf.org/doc/html/rfc7540#section-6.8, 

           

          After sending a GOAWAY frame, the sender can discard frames for
          streams initiated by the receiver with identifiers higher than the
          identified last stream. However, any frames that alter connection
          state cannot be completely ignored. For instance, HEADERS,
          PUSH_PROMISE, and CONTINUATION frames MUST be minimally processed to
          ensure the state maintained for header compression is consistent (see Section 4.3
          );
          similarly, DATA frames MUST be counted toward the
          connection flow-control window. Failure to process these frames can
          cause flow control or header compression state to become
          unsynchronized.

          and in Section 4.3,

          A receiving endpoint reassembles the header block by concatenating
          its fragments and then decompresses the block to reconstruct the
          header list.

          A complete header block consists of either:

          o a single HEADERS or PUSH_PROMISE frame, with the END_HEADERS flag
          set, or

          o a HEADERS or PUSH_PROMISE frame with the END_HEADERS flag cleared
          and one or more CONTINUATION frames, where the last CONTINUATION
          frame has the END_HEADERS flag set.

          Header compression is stateful. One compression context and one
          decompression context are used for the entire connection. A decoding
          error in a header block MUST be treated as a connection error
          (Section 5.4.1
          ) of type COMPRESSION_ERROR.

          I'm not sure about how to deal with a single HEADERS with the END_HEADERS flag set. Thanks Oleg Kalnichevski

          yinwoods yinwoods added a comment - - edited In Section 6.8 https://datatracker.ietf.org/doc/html/rfc7540#section-6.8,     After sending a GOAWAY frame, the sender can discard frames for streams initiated by the receiver with identifiers higher than the identified last stream. However, any frames that alter connection state cannot be completely ignored. For instance, HEADERS, PUSH_PROMISE, and CONTINUATION frames MUST be minimally processed to ensure the state maintained for header compression is consistent (see Section 4.3 ); similarly, DATA frames MUST be counted toward the connection flow-control window. Failure to process these frames can cause flow control or header compression state to become unsynchronized. and in Section 4.3, A receiving endpoint reassembles the header block by concatenating its fragments and then decompresses the block to reconstruct the header list. A complete header block consists of either: o a single HEADERS or PUSH_PROMISE frame, with the END_HEADERS flag set, or o a HEADERS or PUSH_PROMISE frame with the END_HEADERS flag cleared and one or more CONTINUATION frames, where the last CONTINUATION frame has the END_HEADERS flag set. Header compression is stateful. One compression context and one decompression context are used for the entire connection. A decoding error in a header block MUST be treated as a connection error ( Section 5.4.1 ) of type COMPRESSION_ERROR. I'm not sure about how to deal with a single HEADERS with the END_HEADERS flag set. Thanks Oleg Kalnichevski

          > As RFC7540 section 4.3 defines, looks like single HEADERS frame should be consumed.

          yinwoods I could not find anything in the section 4.3 to that effect. Could you please quote a statement from the specification you are referring to?

          Oleg

          olegk Oleg Kalnichevski added a comment - > As RFC7540 section 4.3 defines, looks like single HEADERS frame should be consumed. yinwoods I could not find anything in the section 4.3 to that effect. Could you please quote a statement from the specification you are referring to? Oleg
          yinwoods yinwoods made changes -
          Description current implemention in version 5.1, when connection gracefully shutdown and receives a single HEADERS frame with END_HEADERS flag set, it will throws "Stream refused" exception, not sure about this. 

          As RFC7540 section 4.3 defines, looks like single HEADERS frame should be consumed.
          current implemention in version 5.1, when connection gracefully shutdown and receives a single HEADERS frame with END_HEADERS flag set, it will throws "Stream refused" exception, not sure about this. 

          As RFC7540 section 4.3 defines, looks like single HEADERS frame should be consumed.

           

          This code in org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java:1073

           

          !image-2021-11-08-20-07-02-610.png!
          yinwoods yinwoods made changes -
          Attachment image-2021-11-08-20-07-02-610.png [ 13035834 ]
          yinwoods yinwoods made changes -
          Field Original Value New Value
          Attachment image-2021-11-08-20-06-43-301.png [ 13035833 ]
          yinwoods yinwoods created issue -

          People

            olegk Oleg Kalnichevski
            yinwoods yinwoods
            Votes:
            0 Vote for this issue
            Watchers:
            Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

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

                Slack