Fixed h2spec 5.1-11 test when sending a data frame to the server on a closed stream

[bryancall]

on a closed stream.

[bryancall]

Before the fix:

16:27:23 homer:/usr/local/var/log/trafficserver$ h2spec  -t -k -p 4443 http2/5.1/11 -v
Hypertext Transfer Protocol Version 2 (HTTP/2)
  5. Streams and Multiplexing
    5.1. Stream States
           [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0)
           [recv] SETTINGS Frame (length:12, flags:0x00, stream_id:0)
           [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
           [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)
           [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
           [send] HEADERS Frame (length:15, flags:0x05, stream_id:1)
           [recv] HEADERS Frame (length:109, flags:0x04, stream_id:1)
           [recv] DATA Frame (length:531, flags:0x01, stream_id:1)
           [send] DATA Frame (length:4, flags:0x01, stream_id:1)
           [recv] RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
           [recv] Timeout
      × 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)

Failures:

Hypertext Transfer Protocol Version 2 (HTTP/2)
  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)

Finished in 2.0229 seconds
1 tests, 0 passed, 0 skipped, 1 failed

After the fix:

16:27:33 homer:/usr/local/var/log/trafficserver$ h2spec  -t -k -p 4443 http2/5.1/11 -v
Hypertext Transfer Protocol Version 2 (HTTP/2)
  5. Streams and Multiplexing
    5.1. Stream States
           [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0)
           [recv] SETTINGS Frame (length:12, flags:0x00, stream_id:0)
           [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
           [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)
           [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
           [send] HEADERS Frame (length:15, flags:0x05, stream_id:1)
           [recv] HEADERS Frame (length:109, flags:0x04, stream_id:1)
           [recv] DATA Frame (length:531, flags:0x01, stream_id:1)
           [send] DATA Frame (length:4, flags:0x01, stream_id:1)
           [recv] GOAWAY Frame (length:8, flags:0x00, stream_id:0)
           [recv] Connection closed
      ✔ 11: closed: Sends a DATA frame

Finished in 0.0354 seconds
1 tests, 1 passed, 0 skipped, 0 failed

[maskit]

I haven't tested with the change yet, but it may break other cases. If I remember correctly, the block is not only for stream count error.
You may want to read #1753 .

[maskit]

@zizhong Are you working on the issue?

[bryancall]

@maskit Originally the change came from #1262. I was keeping the case where there might be a bunch of POSTs and it is going over the max streams set by the server.

[bryancall]

Another option is to only send stream errors to id that are over our last stream id. We have a pretty good idea that stream ids less than or equal to the last stream id that we can't find in the stream list it is closed.

[bryancall]

I went back to read the RFC again. After rereading section 6.1 (https://tools.ietf.org/html/rfc7540#section-6.1), I believe we are doing the correct behavior and h2spec is incorrect:

If a DATA frame is received
whose stream is not in "open" or "half-closed (local)" state, the
recipient MUST respond with a stream error (Section 5.4.2) of type
STREAM_CLOSED.

[maskit]

I think the test is for closed state on section 5.1 (https://tools.ietf.org/html/rfc7540#section-5.1).

      An endpoint that receives any frame other than PRIORITY
      after receiving a RST_STREAM MUST treat that as a stream error
      (Section 5.4.2) of type STREAM_CLOSED.  Similarly, an endpoint
      that receives any frames after receiving a frame with the
      END_STREAM flag set MUST treat that as a connection error
      (Section 5.4.1) of type STREAM_CLOSED, unless the frame is
      permitted as described below.

We are treating the both cases above as a stream error, which is incorrect according to this paragraph. The h2spec test case is for the latter case and #1753 tried to fix it, but the change was treating the both case as a connection error, which breaks the former case.

[bryancall]

@maskit The server isn't receiving a frame with END_STREAM, it is sending a frame with END_STREAM. I still think that h2spec is wrong.

[zwoop]

This sounds like potentially a 7.1.x back port too ?

[bryancall]

@zwoop I think the ATS behavior is already correct and h2spec is incorrect.

[maskit]

@bryancall h2spec is sending a HEADERS frame with END_STREAM flag.

[send] HEADERS Frame (length:15, flags:0x05, stream_id:1)

[mlibbey]

Thanks Bryan for filing summerwind/h2spec#88.