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.

Sign up for GitHub

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

Jump to bottom

h2spec #22

Open
indutny opened this issue Dec 8, 2015 · 7 comments
Open

h2spec #22

indutny opened this issue Dec 8, 2015 · 7 comments
Labels
ready

Comments

@indutny
Copy link
Collaborator

indutny commented Dec 8, 2015

Looks like there are many errors when running h2spec against spdy-transport.

===============================================================================
Failed tests
===============================================================================

  4.3. Header Compression and Decompression
    × Encodes Dynamic Table Size Update (RFC 7541, 6.3) after common header fields
      - The endpoint MUST terminate the connection with a connection error of type COMPRESSION_ERROR.
        Expected: GOAWAY frame (ErrorCode: COMPRESSION_ERROR)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

  5.1. Stream States
    × idle: Sends a DATA frame
      - The endpoint MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: STREAM_CLOSED)
    × idle: Sends a RST_STREAM frame
      - The endpoint MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: Test timeout
    × idle: Sends a WINDOW_UPDATE frame
      - The endpoint MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: STREAM_CLOSED)
    × half closed (remote): Sends a HEADERS frame
      - The endpoint MUST respond with a stream error (Section 5.4.2) of type STREAM_CLOSED.
        Expected: GOAWAY frame (ErrorCode: STREAM_CLOSED)
                  RST_STREAM frame (ErrorCode: STREAM_CLOSED)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

    5.1.1. Stream Identifiers
      × Sends even-numbered stream identifier
        - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
          Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                    Connection close
            Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: PROTOCOL_ERROR)

  5.4. Error Handling
    5.4.1. Connection Error Handling
      × Receives a GOAWAY frame
        - After sending the GOAWAY frame, the endpoint MUST close the TCP connection.
          Expected: Connection close
            Actual: Error: Connection closed, but did not receive a GOAWAY Frame.

  5.5. Extending HTTP/2
    × Sends an unknown extension frame in the middle of a header block
      - The endpoint MUST treat as a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: Test timeout

  6.1. DATA
    × Sends a DATA frame with 0x0 stream identifier
      - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: Error: connection error: PROTOCOL_ERROR

  6.2. HEADERS
    × Sends a HEADERS frame followed by any frame other than CONTINUATION
      - The endpoint MUST treat the receipt of any other type of frame as a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: STREAM_CLOSED)

  6.4. RST_STREAM
    × Sends a RST_STREAM frame on a idle stream
      - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: Test timeout

  6.5. SETTINGS
    6.5.2. Defined SETTINGS Parameters
      × SETTINGS_ENABLE_PUSH (0x2): Sends the value other than 0 or 1
        - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
          Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                    Connection close
            Actual: SETTINGS frame (Length: 0, Flags: 1)
      × SETTINGS_INITIAL_WINDOW_SIZE (0x4): Sends the value above the maximum flow control window size
        - The endpoint MUST respond with a connection error of type FLOW_CONTROL_ERROR.
          Expected: GOAWAY frame (ErrorCode: FLOW_CONTROL_ERROR)
                    Connection close
            Actual: SETTINGS frame (Length: 0, Flags: 1)
      × SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value below the initial value
        - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
          Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                    Connection close
            Actual: SETTINGS frame (Length: 0, Flags: 1)
      × SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value above the maximum allowed frame size
        - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
          Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                    Connection close
            Actual: SETTINGS frame (Length: 0, Flags: 1)

  6.9. WINDOW_UPDATE
    × Sends a WINDOW_UPDATE frame with an flow control window increment of 0
      - The endpoint MUST respond with a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: Test timeout
    × Sends a WINDOW_UPDATE frame with an flow control window increment of 0 on a stream
      - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

    6.9.1. The Flow Control Window
      × Sends multiple WINDOW_UPDATE frames on a connection increasing the flow control window to above 2^31-1
        - The endpoint MUST sends a GOAWAY frame with a FLOW_CONTROL_ERROR code.
          Expected: GOAWAY frame (ErrorCode: FLOW_CONTROL_ERROR)
            Actual: Test timeout
      × Sends multiple WINDOW_UPDATE frames on a stream increasing the flow control window to above 2^31-1
        - The endpoint MUST sends a RST_STREAM with the error code of FLOW_CONTROL_ERROR code.
          Expected: RST_STREAM frame (ErrorCode: FLOW_CONTROL_ERROR)
            Actual: DATA frame (Length: 0, Flags: 1)

    6.9.2. Initial Flow Control Window Size
      × Sends a SETTINGS_INITIAL_WINDOW_SIZE settings with an exceeded maximum window size value
        - The endpoint MUST respond with a connection error of type FLOW_CONTROL_ERROR.
          Expected: GOAWAY frame (ErrorCode: FLOW_CONTROL_ERROR)
                    Connection close
            Actual: SETTINGS frame (Length: 0, Flags: 1)

  6.10. CONTINUATION
    × Sends a CONTINUATION frame followed by any frame other than CONTINUATION
      - The endpoint MUST treat as a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: STREAM_CLOSED)

  8.1. HTTP Request/Response Exchange
    × Sends a second HEADERS frame without the END_STREAM flag
      - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

    8.1.2. HTTP Header Fields
      × Sends a HEADERS frame that contains the header field name in uppercase letters
        - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
          Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                    RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                    Connection close
            Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.1. Pseudo-Header Fields
        × Sends a HEADERS frame that contains the pseudo-header field defined for response
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the invalid pseudo-header field
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.2. Connection-Specific Header Fields
        × 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 (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the TE header field that contain any value other than "trailers"
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.3. Request Pseudo-Header Fields
        × Sends a HEADERS frame that omits mandatory pseudo-header fields
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame containing more than one pseudo-header fields with the same name
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.6. Malformed Requests and Responses
        × Sends a HEADERS frame that contains the "content-length" header field which does not equal the sum of the DATA frame payload lengths
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the "content-length" header field which does not equal the sum of the multiple DATA frame payload lengths
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

  8.2. Server Push
    × Sends a PUSH_PROMISE frame
      - The endpoint MUST treat the receipt of a PUSH_PROMISE frame as a connection error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: RST_STREAM frame (Length: 4, Flags: 0, ErrorCode: STREAM_CLOSED)
@djphoenix
Copy link
Contributor

Any progress?

@indutny
Copy link
Collaborator Author

indutny commented Jan 3, 2016

Not yet. Do you want to help me with this?

@djphoenix
Copy link
Contributor

Maybe I will send some pull requests.

@djphoenix djphoenix mentioned this issue Jan 10, 2016
Closed
@djphoenix
Copy link
Contributor

I think 4.3. Header Compression and Decompression is related to hpack.js (maybe need event for table update), and 5.4.1. Connection Error Handling is raised cause /lib/spdy-transport/protocol/base/scheduler.js#L128 / L166.
We need to fix them before continue...

@indutny
Copy link
Collaborator Author

indutny commented Jan 13, 2016

Yeah 4.3 most likely belongs to hpack.js indeed.

Could you please explain what exactly happening in scheduler and why is it causing 5.4.1? I think the problem in 5.4.1 is that GOAWAY is not sent at all...

@djphoenix
Copy link
Contributor

After some debugging I found that GOAWAY frame is encoded and scheduled, and dequeued from scheduler to write queue. Maybe stream closed before actual write to socket. But I failed to fix it anyway, with "dirty hacks" too.

indutny pushed a commit that referenced this issue Jan 14, 2016
@djphoenix @indutny
http2: fix some of the h2spec issues
a727a0a 
See: #22

Fixed:

5.1 Stream States
    × idle: Sends a DATA frame
    × idle: Sends a RST_STREAM frame
    × idle: Sends a WINDOW_UPDATE frame
5.1.1 Stream Identifiers
    × Sends even-numbered stream identifier
5.5 Extending HTTP/2
    × Sends an unknown extension frame in the middle of a header block
6.1 DATA
    × Sends a DATA frame with 0x0 stream identifier
6.4 RST_STREAM
    × Sends a RST_STREAM frame on a idle stream
6.5.2 Defined SETTINGS Parameters
    × SETTINGS_ENABLE_PUSH (0x2): Sends the value other than 0 or 1
    × SETTINGS_INITIAL_WINDOW_SIZE (0x4): Sends the value above...
    × SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value below the...
    × SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value above the...
8.2. Server Push
    × Sends a PUSH_PROMISE frame
@djphoenix
Copy link
Contributor

Updated to:

===============================================================================
Failed tests
===============================================================================

  4.3. Header Compression and Decompression
    × Encodes Dynamic Table Size Update (RFC 7541, 6.3) after common header fields
      - The endpoint MUST terminate the connection with a connection error of type COMPRESSION_ERROR.
        Expected: GOAWAY frame (ErrorCode: COMPRESSION_ERROR)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

  5.4. Error Handling
    5.4.1. Connection Error Handling
      × Receives a GOAWAY frame
        - After sending the GOAWAY frame, the endpoint MUST close the TCP connection.
          Expected: Connection close
            Actual: Error: Connection closed, but did not receive a GOAWAY Frame.

  8.1. HTTP Request/Response Exchange
    × Sends a second HEADERS frame without the END_STREAM flag
      - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
        Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                  RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                  Connection close
          Actual: DATA frame (Length: 0, Flags: 1)

    8.1.2. HTTP Header Fields
      8.1.2.1. Pseudo-Header Fields
        × Sends a HEADERS frame that contains the pseudo-header field defined for response
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the invalid pseudo-header field
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.2. Connection-Specific Header Fields
        × 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 (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the TE header field that contain any value other than "trailers"
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.3. Request Pseudo-Header Fields
        × Sends a HEADERS frame that omits mandatory pseudo-header fields
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame containing more than one pseudo-header fields with the same name
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

      8.1.2.6. Malformed Requests and Responses
        × Sends a HEADERS frame that contains the "content-length" header field which does not equal the sum of the DATA frame payload lengths
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)
        × Sends a HEADERS frame that contains the "content-length" header field which does not equal the sum of the multiple DATA frame payload lengths
          - The endpoint MUST respond with a stream error of type PROTOCOL_ERROR.
            Expected: GOAWAY frame (ErrorCode: PROTOCOL_ERROR)
                      RST_STREAM frame (ErrorCode: PROTOCOL_ERROR)
                      Connection close
              Actual: DATA frame (Length: 0, Flags: 1)

@mykiimike mykiimike mentioned this issue Nov 2, 2016
Closed
@daviddias daviddias added the ready label Feb 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ready
Milestone
No milestone
Development

No branches or pull requests
3 participants
@indutny @daviddias @djphoenix