Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP/2 conformance - h2spec failures #2132

Closed
tunetheweb opened this issue Apr 23, 2018 · 4 comments
Closed

HTTP/2 conformance - h2spec failures #2132

tunetheweb opened this issue Apr 23, 2018 · 4 comments
Labels
upstream ⬆️ Relates to some dependency of this project

Comments

@tunetheweb
Copy link

I'm using the h2spec program (https://github.com/summerwind/h2spec) and running the following command:

./h2spec -S -t -h caddyserver.com -p 443

This shows the following results:

146 tests, 137 passed, 0 skipped, 9 failed

The actual failures are given below. Now to be fair most of these tests are about how to handle bad client requests and give the correct error response. So not super critical TBH. In an ideal world clients shouldn't send these types of requests and so there is no issue. Of course we don't live in an idle world...

Anyway thought it worth raising in case you wanted to fix.

Failures: 

Hypertext Transfer Protocol Version 2 (HTTP/2)
  4. HTTP Frames
    4.2. Frame Size
      × 3: Sends a large size HEADERS frame that exceeds the SETTINGS_MAX_FRAME_SIZE
        -> The endpoint MUST respond with a connection error of type FRAME_SIZE_ERROR.
           Expected: GOAWAY Frame (Error Code: FRAME_SIZE_ERROR)
                     Connection closed
             Actual: DATA Frame (length:0, flags:0x01, stream_id:1)

  5. Streams and Multiplexing
    5.1. Stream States
      × 11: closed: Sends a DATA frame
        -> The endpoint MUST treat this as a connection error of type STREAM_CLOSED.
           Expected: GOAWAY Frame (Error Code: STREAM_CLOSED)
                     Connection closed
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

  6. Frame Definitions
    6.9. WINDOW_UPDATE
      6.9.1. The Flow-Control Window
        × 3: Sends multiple WINDOW_UPDATE frames increasing the flow control window to above 2^31-1 on a stream
          -> The endpoint MUST sends a RST_STREAM frame with a FLOW_CONTROL_ERROR code.
             Expected: RST_STREAM Frame (Error Code: FLOW_CONTROL_ERROR)
               Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

      6.9.2. Initial Flow-Control Window Size
        × 2: Sends a SETTINGS frame for window size to be negative
          -> The endpoint MUST track the negative flow-control window.
             Expected: DATA Frame (length:1, flags:0x00, stream_id:1)
               Actual: Timeout

  8. HTTP Message Exchanges
    8.1. HTTP Request/Response Exchange
      8.1.2. HTTP Header Fields
        8.1.2.2. Connection-Specific Header Fields
          × 1: Sends a HEADERS frame that contains the connection-specific header field
            -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
               Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                         RST_STREAM Frame (Error Code: PROTOCOL_ERROR)
                         Connection closed
                 Actual: DATA Frame (length:51, flags:0x01, stream_id:1)
          × 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers"
            -> The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
               Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                         RST_STREAM Frame (Error Code: PROTOCOL_ERROR)
                         Connection closed
                 Actual: DATA Frame (length:53, flags:0x01, stream_id:1)

        8.1.2.6. Malformed Requests and Responses
          × 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length
            -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR.
               Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                         RST_STREAM Frame (Error Code: PROTOCOL_ERROR)
                         Connection closed
                 Actual: Timeout
          × 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length
            -> The endpoint MUST treat this as a stream error of type PROTOCOL_ERROR.
               Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                         RST_STREAM Frame (Error Code: PROTOCOL_ERROR)
                         Connection closed
                 Actual: WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)

HPACK: Header Compression for HTTP/2
  4. Dynamic Table Management
    4.2. Maximum Table Size
      × 1: Sends a dynamic table size update at the end of header block
        -> The endpoint MUST treat this as a decoding error.
           Expected: GOAWAY Frame (Error Code: COMPRESSION_ERROR)
                     Connection closed
             Actual: DATA Frame (length:0, flags:0x01, stream_id:1)

@mholt mholt added help wanted 🆘 Extra attention is needed upstream ⬆️ Relates to some dependency of this project labels Apr 23, 2018
@mholt
Copy link
Member

mholt commented Apr 23, 2018

Thanks! I think most of these will need to be fixed upstream in the Go standard library. This is a TODO. (If anyone wants to tackle these, go for it!)

@mholt
Copy link
Member

mholt commented May 4, 2018

Going to close this since there's not much we can do about it in Caddy.

@mholt mholt closed this as completed May 4, 2018
@mholt mholt removed the help wanted 🆘 Extra attention is needed label May 4, 2018
@Kriechi
Copy link

Kriechi commented Aug 2, 2018

FYI: I just got bitten by this.
For some weird reason using https://github.com/GoogleChromeLabs/simplehttp2server as development server seems to have worked for me.

@mholt
Copy link
Member

mholt commented Aug 2, 2018

It's been mostly fixed upstream, and will be available in Go 1.11 later this month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream ⬆️ Relates to some dependency of this project
Projects
None yet
Development

No branches or pull requests

3 participants