Giter VIP home page Giter VIP logo

h2spec's People

Contributors

anmonteiro avatar bryce-anderson avatar chhsiao90 avatar detailyang avatar deweerdt avatar essen avatar i110 avatar jackloughran avatar jstourac avatar karm avatar kazuho avatar lorban avatar lukasa avatar madgnome avatar masaori335 avatar maskit avatar shane-kearns avatar stepancheg avatar summerwind avatar tatsuhiro-t avatar vankoven avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

h2spec's Issues

Doesn't server receiving GOAWAY frame have to close the connection ?

The following two lines expect that the server will close the connection, but in my understanding http2 specification doesn't state that, rather I think it expects the sender of GOAWAY frame will close the connection, because GOAWAY frame is used to notify the peer that the sender will initiate closing the connection. What do you think?
https://github.com/summerwind/h2spec/blob/master/generic/3_8_goaway.go#L30
https://github.com/summerwind/h2spec/blob/master/http2/7_error_codes.go#L27

h2spec does not send WINDOW_UPDATE frames during connection close tests

This is a minor issue I'm opening mostly for documentation purposes.
A simple workaround is to use a smaller default document when testing.

The test cases which use TestStreamClose() will fail incorrectly if the web server's default document is larger than the flow control window (64k).
After sending 64k of data, the server is required to stop sending until it has received a WINDOW_UPDATE. As h2spec never sends this frame, the test will timeout and fail.

authority includes default ports

When using v0.0.8, the :authority pseudo-header field always appends the port, even when the default port of 443 is used for https. While not strictly incorrect, perhaps a good new test would be to see if both (with and without port) are allowed by a server when a default port (80 or 443) is used.

i.e.:
:authority: 127.0.0.1:443
vs.
:authority: 127.0.0.1

Minor wording issue with test 8.1.2.3

The wording for a test in section 8.1.2.3 is:
"8.1.2.3. Request Pseudo-Header Fields: Sends a HEADERS frame that is omitted mandatory pseudo-header fields"
This should be:
"8.1.2.3. Request Pseudo-Header Fields: Sends a HEADERS frame that omits mandatory pseudo-header fields"

Test case 6.4 must allow response

Test case 6.4 sends a headers frame with end_headers and end_stream set. That means the server is allowed to produce and response and queue the response for sending. The server is not obliged to check for a reset stream arriving before queuing the response for sending.
The test should either:

  • Not send end_stream on the header frame it sends
  • Consume the response, then assert the goaway

Two tests for the same thing?

The test case from 5.1: "half closed (remote): Sends a DATA frame" appears to be testing the same path as the test case from 6.1: "Sends a DATA frame on the stream that is not opened". Perhaps one should be deleted or the test cases disambiguated more clearly? The 6.1 test in particular is somewhat confusing since "not opened" could mean a few different states.

Allow RST_STREAM with code=0 for last 5.1 test

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

My server's endpoint sends RST_STREAM with code=0 (as it doesn't want to read the body) before the incorrect/unexpected CONTINUATION frame comes through.

debug logs:

WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=0
HANDLING	http.h2_connection{type="server"}	SETTING	Flags=0	StreamID=0
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=1
HANDLING	http.h2_connection{type="server"}	SETTING	Flags=1	StreamID=0
HANDLING	http.h2_connection{type="server"}	HEADERS	Flags=1	StreamID=1
HANDLING	http.h2_connection{type="server"}	CONTINUATION	Flags=4	StreamID=1
[16/Dec/2016:19:23:22 +1100] "GET / HTTP/2"  "-" "-"
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="open";parent=0;dependees={}}	HEADERS	Flags=4
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="open";parent=0;dependees={}}	DATA	FLAGS=1
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="half closed (local)";parent=0;dependees={}}	RST_STREAM	Flags=0
HANDLING	http.h2_connection{type="server"}	CONTINUATION	Flags=4	StreamID=1
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={1}}	GOAWAY	Flags=0
get_next_incoming_stream on http.h2_connection{type="server"} failed: PROTOCOL_ERROR(0x1): Protocol error detected: 'CONTINUATION' frames MUST be preceded by a 'HEADERS', 'PUSH_PROMISE' or 'CONTINUATION' frame without the 'END_HEADERS' flag set

stack traceback:
	./http/h2_stream.lua:936: in local 'handler'
	./http/h2_connection.lua:214: in upvalue 'handle_frame'
	./http/h2_connection.lua:254: in method 'step'
	./http/h2_connection.lua:367: in method 'get_next_incoming_stream'
	./http/server.lua:147: in function <./http/server.lua:127>

I think h2spec should skip through the RST_STREAM?

frame size test should be more permissive

4.2. Frame Size
× Sends large size frame that exceeds the SETTINGS_MAX_FRAME_SIZE
- The endpoint MUST send a FRAME_SIZE_ERROR error.

25 313.167882115 10.0.0.1 10.0.0.2 HTTP2 1982 DATA
Stream: DATA, Stream ID: 1, Length 16385
27 313.168127324 10.0.0.2 10.0.0.1 HTTP2 81
RST_STREAM Error: FRAME_SIZE_ERROR (6)

"A frame size error in a frame that could alter the state of the entire
connection MUST be treated as a connection error"

However DATA frames do not affect the whole connection so either a stream error (RST_STREAM) or connection error (GOAWAY) are valid responses.
It's even allowed to close a connection abruptly without sending a GOAWAY "An endpoint might choose to close a connection without sending GOAWAY for misbehaving peers.", but I'm not sure how a test framework would usefully report that. Perhaps a ? (inconclusive) result where an error was expected but the connection was closed.

I do plan to use GOAWAY because a bad sized frame could be very big and probably indicates corruption of the connection.

h2spec 5.1-11 requires GOAWAY frame

When testing with ATS 8.0.0/master and h2spec 2.1.0 it is failing 5.1-11 and requires a GOAWAY frame. However, the HTTP/2 RFC states that the server should send a stream error when there is a data frame on a close stream:

https://tools.ietf.org/html/rfc7540#section-6.1:

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.

My test:

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

Bare CONTINUATION frames in section 5.1 tests

Section 5.1 includes tests that send CONTINUATION frames on a closed or halfclosed stream.
However these CONTINUATION frames are not associated with any HEADERS or PUSH_PROMISE frame, because the preceding frame had the END_HEADERS flag set.

RFC7540 section 6.10 has this to say:
A CONTINUATION frame MUST be preceded by a HEADERS, PUSH_PROMISE or CONTINUATION frame without the END_HEADERS flag set. A recipient that observes violation of this rule MUST respond with a connection error (Section 5.4.1) of type PROTOCOL_ERROR.

Therefore I think a connection error with PROTOCOL error is the expected response for these test cases, rather than STREAM_CLOSED.

Also worth noting this text in section 5.1:
Note that this diagram shows stream state transitions and the frames and flags that affect those transitions only. In this regard, CONTINUATION frames do not result in state transitions; they are effectively part of the HEADERS or PUSH_PROMISE that they follow.

The test case "half closed (remote): Sends a CONTINUATION frame" also causes a COMPRESSION_ERROR with my implementation, because the compressed headers are truncated in the HEADERS frame with END_HEADERS + END_STREAM flags.

should not expect PROTOCOL_ERROR in response to an empty `:path` pseudo header

In 8.1.2.3 there is a test that "sends a HEADERS frame with empty ":path" pseudo-header field" expecting that the server responds with a PROTOCOL_ERROR.

I do not think that RFC 7540 requires such behavior.

Section 8.1.2.3 states:

      This pseudo-header field MUST NOT be empty for "http" or "https"
      URIs; "http" or "https" URIs that do not contain a path component
      MUST include a value of '/'.  The exception to this rule is an
      OPTIONS request for an "http" or "https" URI that does not include
      a path component; these MUST include a ":path" pseudo-header field
      with a value of '*' (see [RFC7230], Section 5.3.4).

   All HTTP/2 requests MUST include exactly one valid value for the
   ":method", ":scheme", and ":path" pseudo-header fields, unless it is
   a CONNECT request (Section 8.3).  An HTTP request that omits
   mandatory pseudo-header fields is malformed (Section 8.1.2.6).

As you can see from the last paragraph, a server is required to respond with a malformed error (i.e. PROTOCOL_ERROR) if the pseudo header field is omitted. There is no definition regarding how a server responds to a request with an empty :path pseudo header field, even though such field is prohibited in the previous paragraph.

Therefore I think that a server is permitted to respond in other ways than sending a PROTOCOL_ERROR (e.g. respond with a 400 Bad Request).

Add way to get listing of tests

It'd be nice to have a way to get a list of available tests
I propose a -l argument to the command line binary that lists them.

HPACK test case 5.2.3 is wrong

Please correct me if I'm wrong, but I believe that test is not sending an EOS anywhere.

It sends a HEADERS frame with the following header representation in it:

"\x00\x85\xf2\xb2\x4a\x87\xff\xff\xff\xfd\x25\x42\x7f"

That decodes as:

  • 0x00 - Literal Header Field Representation without Indexing - New Name
  • 0x85 - Header name has 5 octets and is Huffman encoded
  • (0xf2, 0xb2, 0x4a, 0x87, 0xff) decodes as "x-tes9". More specifically:
x      -      t     e    s     9      *
11110010 10110010 01001010 10000111 11111111

Where * marks the beginning of padding.

So nothing wrong in the header name (except that it decodes as "x-tes9" instead of "x-test" as it says in the comment.

Moving on to the header value, it is encoded as a Huffman-encoded 622462-octet sequence, which is clearly incomplete.

I think the sequence "\xff\xff\xff\xfd" in the input data was meant to encode the EOS symbol, but that's not what's happening in that test.

scheme is not https

When using v0.0.8 with the -t option, the :scheme pseudo-header field has a value of "http", not "https"

Test 6.9: assumes reply has content

  6.9. WINDOW_UPDATE
    × Sends a WINDOW_UPDATE frame
      - The endpoint is expected to send the DATA frame based on the window size.
        Expected: DATA frame
          Actual: Error: The length of DATA frame is invalid.
    ✓ Sends a WINDOW_UPDATE frame with an flow control window increment of 0
    ✓ Sends a WINDOW_UPDATE frame with an flow control window increment of 0 on a stream
    ✓ Sends a WINDOW_UPDATE frame with a length other than a multiple of 4 octets

The endpoint I tested had a 0 length document.

Infinite loop in VerifyConnectionError

We use h2spec in our automated tests for ASP.NET Core's Kestrel HTTP/2 server. The client occasionally gets stuck in an infinite loop in VerifyConnectionError. The timeout setting does not work in this scenario, we have to apply our own that kills the client process after 30 seconds.

Here's a log that shows the client and server behavior. The test behaves as expected and the server closes the connection. The client then gets an error and loops endlessly.

2019-02-09T02:34:39.8786527Z   [xUnit.net 00:00:42.57]     RunIndividualTestCase(testCase: http2/4.3/3, HTTPS:False, Sends a HEADERS frame to another stream while sending the header blocks) [FAIL]
2019-02-09T02:35:14.2263772Z   Failed   RunIndividualTestCase(testCase: http2/4.3/3, HTTPS:False, Sends a HEADERS frame to another stream while sending the header blocks)
2019-02-09T02:35:14.2265139Z   Error Message:
2019-02-09T02:35:14.2266481Z    System.TimeoutException : h2spec didn't exit within 30 seconds.
2019-02-09T02:35:14.2266877Z   Stack Trace:
2019-02-09T02:35:14.2267479Z      at Interop.FunctionalTests.H2SpecCommands.RunTest(String testId, Int32 port, Boolean https, ILogger logger) in /_/src/Servers/Kestrel/test/Interop.FunctionalTests/H2SpecCommands.cs:line 225
2019-02-09T02:35:14.2268245Z      at Interop.FunctionalTests.H2SpecTests.RunIndividualTestCase(H2SpecTestCase testCase) in /_/src/Servers/Kestrel/test/Interop.FunctionalTests/H2SpecTests.cs:line 52
2019-02-09T02:35:14.2268915Z   --- End of stack trace from previous location where exception was thrown ---
2019-02-09T02:35:14.7012619Z   Standard Output Messages:
2019-02-09T02:35:14.9978804Z    | [0.001s] TestLifetime Information: Starting test RunIndividualTestCase-http2/4.3/3, HTTPS:False, Sends a HEADERS frame to another stream while sending the header blocks at 2019-02-09T02:34:09
2019-02-09T02:35:15.0018691Z    | [0.002s] Microsoft.AspNetCore.Hosting.Internal.WebHost Debug: Hosting starting
2019-02-09T02:35:15.0034466Z    | [0.003s] Microsoft.AspNetCore.Hosting.Internal.WebHost Debug: Hosting started
2019-02-09T02:35:15.0065632Z    | [0.003s] Microsoft.AspNetCore.Hosting.Internal.WebHost Debug: Loaded hosting startup assembly Interop.FunctionalTests
2019-02-09T02:35:15.0078218Z    | [0.058s] Interop.FunctionalTests.H2SpecTests Debug: Hypertext Transfer Protocol Version 2 (HTTP/2)
2019-02-09T02:35:15.0086390Z    | [0.060s] Microsoft.AspNetCore.Server.Kestrel Debug: Connection id "0HLKE6S61NCL3" started.
2019-02-09T02:35:15.0094188Z    | [0.062s] Interop.FunctionalTests.H2SpecTests Debug:   4. HTTP Frames
2019-02-09T02:35:15.0101296Z    | [0.062s] Interop.FunctionalTests.H2SpecTests Debug:     4.3. Header Compression and Decompression
2019-02-09T02:35:15.0109784Z    | [0.062s] Interop.FunctionalTests.H2SpecTests Debug:            [send] SETTINGS Frame (length:6, flags:0x00, stream_id:0)
2019-02-09T02:35:15.0117699Z    | [0.062s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" sending SETTINGS frame for stream ID 0 with length 18 and flags NONE
2019-02-09T02:35:15.0125687Z    | [0.062s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" sending WINDOW_UPDATE frame for stream ID 0 with length 4 and flags 0x0
2019-02-09T02:35:15.0138673Z    | [0.062s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" received SETTINGS frame for stream ID 0 with length 6 and flags NONE
2019-02-09T02:35:15.0153366Z    | [0.062s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] SETTINGS Frame (length:18, flags:0x00, stream_id:0)
2019-02-09T02:35:15.0164659Z    | [0.063s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" sending SETTINGS frame for stream ID 0 with length 0 and flags ACK
2019-02-09T02:35:15.0174787Z    | [0.063s] Interop.FunctionalTests.H2SpecTests Debug:            [send] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
2019-02-09T02:35:15.0185840Z    | [0.063s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" received SETTINGS frame for stream ID 0 with length 0 and flags ACK
2019-02-09T02:35:15.0194456Z    | [0.063s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)
2019-02-09T02:35:15.0212247Z    | [0.063s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] SETTINGS Frame (length:0, flags:0x01, stream_id:0)
2019-02-09T02:35:15.0221798Z    | [0.063s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" received HEADERS frame for stream ID 1 with length 16 and flags NONE
2019-02-09T02:35:15.0227797Z    | [0.063s] Interop.FunctionalTests.H2SpecTests Debug:            [send] HEADERS Frame (length:16, flags:0x00, stream_id:1)
2019-02-09T02:35:15.0231510Z    | [0.063s] Interop.FunctionalTests.H2SpecTests Debug:            [send] HEADERS Frame (length:4, flags:0x05, stream_id:3)
2019-02-09T02:35:15.0238248Z    | [0.063s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" received HEADERS frame for stream ID 3 with length 4 and flags END_STREAM, END_HEADERS
2019-02-09T02:35:15.0244902Z    | [0.064s] Microsoft.AspNetCore.Server.Kestrel Information: Connection id "0HLKE6S61NCL3": HTTP/2 connection error.
2019-02-09T02:35:15.0252832Z    | Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2ConnectionErrorException: HTTP/2 connection error (PROTOCOL_ERROR): The client sent a HEADERS frame to stream ID 3 before signaling of the header block for stream ID 1.
2019-02-09T02:35:15.0256896Z    |    at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2Connection.ProcessHeadersFrameAsync[TContext](IHttpApplication`1 application, ReadOnlySequence`1 payload) in /_/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs:line 487
2019-02-09T02:35:15.0261630Z    |    at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2Connection.ProcessFrameAsync[TContext](IHttpApplication`1 application, ReadOnlySequence`1 payload) in /_/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs:line 410
2019-02-09T02:35:15.0263193Z    |    at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2Connection.ProcessRequestsAsync[TContext](IHttpApplication`1 application) in /_/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs:line 306
2019-02-09T02:35:15.0274497Z    | [0.064s] Microsoft.AspNetCore.Server.Kestrel Debug: Connection id "0HLKE6S61NCL3" is closed. The last processed stream ID was 1.
2019-02-09T02:35:15.0319184Z    | [0.064s] Microsoft.AspNetCore.Server.Kestrel Trace: Connection id "0HLKE6S61NCL3" sending GOAWAY frame for stream ID 0 with length 8 and flags 0x0
2019-02-09T02:35:15.0338233Z    | [0.064s] Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets Debug: Connection id "0HLKE6S61NCL3" sending FIN because: "The Socket transport's send loop completed gracefully."
2019-02-09T02:35:15.0344876Z    | [0.065s] Microsoft.AspNetCore.Server.Kestrel Debug: Connection id "0HLKE6S61NCL3" stopped.
2019-02-09T02:35:15.0348639Z    | [0.102s] Interop.FunctionalTests.H2SpecTests Debug:            [send] CONTINUATION Frame (length:3512, flags:0x04, stream_id:1)
2019-02-09T02:35:15.0355131Z    | [0.103s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:35:15.0362897Z    | [0.103s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:35:15.0379015Z    | [0.103s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:35:15.0392690Z    | [0.103s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:35:15.0398480Z    | [0.103s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
... repeated a few thousand times
2019-02-09T02:36:24.3117269Z    | [29.999s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:36:24.3118781Z    | [30.000s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:36:24.3119660Z    | [30.000s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:36:24.3120918Z    | [30.000s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:36:24.3122019Z    | [30.000s] Interop.FunctionalTests.H2SpecTests Debug:            [recv] Error: read tcp 127.0.0.1:63806->127.0.0.1:63805: wsarecv: An established connection was aborted by the software in your host machine.
2019-02-09T02:36:24.3123038Z    | [30.013s] Microsoft.AspNetCore.Hosting.Internal.WebHost Debug: Hosting shutdown
2019-02-09T02:36:24.3124463Z    | [30.048s] TestLifetime Information: Finished test RunIndividualTestCase-http2/4.3/3, HTTPS:False, Sends a HEADERS frame to another stream while sending the header blocks in 30.0465251s

This maps to WSAECONNABORTED 10053 (0x2745) An established connection was aborted by the software in your host machine.

It looks like WaitEvent is only prepared to handle EOF and ECONNRESET. It should probobly assume all read errors are fatal here, no?

Show information why test failed

Currently if test fails, it just say error with read color.
It is a bit hard to debug this, since we have no way to know what was wrong.
It is extremely valuable to show some context of failure.
For example, we expected A but got B. Or we expected A but timed out.

generic/3.1 sends non-zero padding and doesn't like the ensuing GOAWAY

generic/3.1.3 is not correct:

      × 3: Sends a DATA frame with padding
        -> The endpoint MUST accept DATA frame with padding.
           Expected: HEADERS Frame (stream_id:1)
             Actual: GOAWAY Frame (length:30, flags:0x00, stream_id:0)

From RFC 7540 Section 6.1:

A receiver is not obligated to verify padding but MAY treat non-zero padding as a connection error of type PROTOCOL_ERROR.

My server's debug logs:

NEW CONNECTION	http.h2_connection{type="server"}
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	FLAGS=0
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=0	StreamID=0
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	FLAGS=1
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=1	StreamID=0
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="idle";parent=0;dependees={}}	HEADERS	Flags=4	StreamID=1
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="open";parent=0;dependees={}}	DATA	Flags=9	StreamID=1
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={1}}	GOAWAY	FLAGS=0	PROTOCOL_ERROR(0x1): Protocol error detected: padding not null bytes

Test 5.1: STREAM_CLOSED vs PROTOCOL_ERROR

× 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: GOAWAY frame (Length: 48, Flags: 0, ErrorCode: STREAM_CLOSED)

In my implementation, this check from section 6.1 happens first:

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.

Part of https://github.com/daurnimator/lua-http/milestones/h2spec%20compliance

Send DATA frame on Idle stream

It seems that 5_1.go violate rfc7540 and 6_1.go

DATA frames are subject to flow control and can only be sent when a
stream is in the "open" or "half-closed (remote)" state. The entire
DATA frame payload is included in flow control, including the Pad
Length and Padding fields if present. 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.

Is that the test should be removed?

Missing tests

I have a few extra test cases in my servers test suite that I think could be moved to h2spec. I just wanted to check I hadn't missed them before trying to open a PR. (I've never written any go so don't want to waste my time if these tests are already implemented)

possible extra tests

  • http/3.5

    • Send a valid http2 preface but then follow it with not a settings frame. expect GoAway with protocol error
    • A test that expects an ack for the clients initial settings from the server.
  • Can there be a test that makes 1+ valid requests then behaves incorrectly and we expect the last stream id to be equal to the last request processed?

Strict mode option

Some test cases about the closed state is difficult to pass in some implementations.
So I'm going to mark these test cases as "strict". The "strict" test cases are executed only if "strict" option is enabled.

Test http2/6.10/1 and others do not pay attention to MAX_HEADER_LIST_SIZE

I was receiving failures when running http2/6.10/1 and a few other tests because the client is ignoring the negotiated header size.
In particular with 6.10/1, sends dummy headers with a size that is based on the config's MaxHeaderLen. However, as part of the handshake, the server is sending
2018/07/12 09:12:54 http2: Framer 0xc0001bc000: wrote SETTINGS len=24, settings: MAX_FRAME_SIZE=1048576, MAX_CONCURRENT_STREAMS=250, MAX_HEADER_LIST_SIZE=2000, INITIAL_WINDOW_SIZE=1048576
I can adjust what the maximum header bytes are on the server side, just like the client can. However, there are other test cases that also treat the client value as gospel. If I set my server to 4000 to match the client, the server will fail this test with a COMPRESS_ERROR since the CONTINUATION frame has a length of 4387.

It seems that the client should respect the server setting where necessary.

Cannot build from source

Error is:

% make build
go build -ldflags "-X main.VERSION=2.1.0 -X main.COMMIT=d99d17171179c709cf23a3df1aadc9657addf825" cmd/h2spec/h2spec.go
cmd/h2spec/h2spec.go:8:2: cannot find package "github.com/spf13/cobra" in any of:
	/usr/local/Cellar/go/1.8.1/libexec/src/github.com/spf13/cobra (from $GOROOT)
	/Users/yozh/usr/gopath/src/github.com/spf13/cobra (from $GOPATH)
make: *** [build] Error 1

Support servers that rst_stream on unsupported request

I have a server/set of endpoints that will RST_STREAM if the request is not of a certain form.

Failures: 

Generic tests for HTTP/2 server
  2. Streams and Multiplexing
    × 2: Sends a WINDOW_UPDATE frame on half-closed (remote) stream
      -> The endpoint MUST accept WINDOW_UPDATE frame.
         Expected: DATA frame
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 3: Sends a PRIORITY frame on half-closed (remote) stream
      -> The endpoint MUST accept PRIORITY frame.
         Expected: DATA frame
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

  3. Frame Definitions
    3.1. DATA
      × 1: Sends a DATA frame
        -> The endpoint MUST accept DATA frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)
      × 2: Sends multiple DATA frames
        -> The endpoint MUST accept multiple DATA frames.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Connection closed
      × 3: Sends a DATA frame with padding
        -> The endpoint MUST accept DATA frame with padding.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Connection closed

    3.2. HEADERS
      × 1: Sends a HEADERS frame
        -> The endpoint MUST accept HEADERS frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 2: Sends a HEADERS frame with padding
        -> The endpoint MUST accept HEADERS frame with padding.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 3: Sends a HEADERS frame with priority
        -> The endpoint MUST accept HEADERS frame with priority.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

    3.3. PRIORITY
      × 1: Sends a PRIORITY frame with priority 1
        -> The endpoint MUST accept PRIORITY frame with priority 1.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 2: Sends a PRIORITY frame with priority 256
        -> The endpoint MUST accept PRIORITY frame with priority 256.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 3: Sends a PRIORITY frame with stream dependency
        -> The endpoint MUST accept PRIORITY frame with stream dependency.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 4: Sends a PRIORITY frame with exclusive
        -> The endpoint MUST accept PRIORITY frame with exclusive.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 5: Sends a PRIORITY frame for an idle stream, then send a HEADER frame for a lower stream ID
        -> The endpoint MUST respond the HEADER frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

    3.9. WINDOW_UPDATE
      × 2: Sends a WINDOW_UPDATE frame with stream ID 1
        -> The endpoint MUST accept WINDOW_UPDATE frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

    3.10. CONTINUATION
      × 1: Sends a CONTINUATION frame
        -> The endpoint MUST accept CONTINUATION frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
      × 2: Sends multiple CONTINUATION frames
        -> The endpoint MUST accept multiple CONTINUATION frames.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

  4. HTTP Message Exchanges
    × 1: Sends a GET request
      -> The endpoint MUST respond to the request.
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 2: Sends a HEAD request
      -> The endpoint MUST respond to the request.
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 3: Sends a POST request
      -> The endpoint MUST respond to the request.
         Expected: HEADERS Frame (stream_id:1)
           Actual: WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)
    × 4: Sends a POST request with trailers
      -> The endpoint MUST respond to the request.
         Expected: HEADERS Frame (stream_id:1)
           Actual: Connection closed

  5. HPACK
    × 1: Sends a indexed header field representation
      -> The endpoint MUST accept indexed header field representation
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 2: Sends a literal header field with incremental indexing - indexed name
      -> The endpoint MUST accept literal header field with incremental indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 3: Sends a literal header field with incremental indexing - indexed name (with Huffman coding)
      -> The endpoint MUST accept literal header field with incremental indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 4: Sends a literal header field with incremental indexing - new name
      -> The endpoint MUST accept literal header field with incremental indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 5: Sends a literal header field with incremental indexing - new name (with Huffman coding)
      -> The endpoint MUST accept literal header field with incremental indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 6: Sends a literal header field without indexing - indexed name
      -> The endpoint MUST accept literal header field without indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 7: Sends a literal header field without indexing - indexed name (with Huffman coding)
      -> The endpoint MUST accept literal header field without indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 8: Sends a literal header field without indexing - new name
      -> The endpoint MUST accept literal header field without indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 9: Sends a literal header field without indexing - new name (huffman encoded)
      -> The endpoint MUST accept literal header field without indexing
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 10: Sends a literal header field never indexed - indexed name
      -> The endpoint MUST accept literal header field never indexed
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 11: Sends a literal header field never indexed - indexed name (huffman encoded)
      -> The endpoint MUST accept literal header field never indexed
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 12: Sends a literal header field never indexed - new name
      -> The endpoint MUST accept literal header field never indexed
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 13: Sends a literal header field never indexed - new name (huffman encoded)
      -> The endpoint MUST accept literal header field never indexed
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 14: Sends a dynamic table size update
      -> The endpoint MUST accept dynamic table size update
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)
    × 15: Sends multiple dynamic table size update
      -> The endpoint MUST accept multiple dynamic table size update
         Expected: HEADERS Frame (stream_id:1)
           Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

Hypertext Transfer Protocol Version 2 (HTTP/2)
  4. HTTP Frames
    4.2. Frame Size
      × 1: Sends a DATA frame with 2^14 octets in length
        -> The endpoint MUST be capable of receiving and minimally processing frames up to 2^14 octets in length.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Connection closed

  6. Frame Definitions
    6.5. SETTINGS
      6.5.3. Settings Synchronization
        × 1: Sends multiple values of SETTINGS_INITIAL_WINDOW_SIZE
        Error: Unable to get server data length

    6.9. WINDOW_UPDATE
      6.9.1. The Flow-Control Window
        × 1: Sends SETTINGS frame to set the initial window size to 1 and sends HEADERS frame
        Error: Unable to get server data length

      6.9.2. Initial Flow-Control Window Size
        × 1: Changes SETTINGS_INITIAL_WINDOW_SIZE after sending HEADERS frame
        Error: Unable to get server data length
        × 2: Sends a SETTINGS frame for window size to be negative
        Error: Unable to get server data length

    6.10. CONTINUATION
      × 1: Sends multiple CONTINUATION frames preceded by a HEADERS frame
        -> The endpoint must accept the frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: RST_STREAM Frame (length:4, flags:0x00, stream_id:1)

Invalid DATA frame flags for the padding test in 6.1

In section 6.1, \x00\x00\x05\x00\x0b\x00\x00\x00\x01 appears as the frame header for a DATA frame in the padding test. This results in \x0b as the frame flags octet, which is invalid since a DATA frame header does not define a flag for the 0x2 bit. Perhaps it should be \x09?

From the spec:
Flags that have no defined semantics for a particular frame type MUST be ignored and MUST be left unset (0x0) when sending.

Test case HPACK/4.2 is wrong

According to the RFC 7541, section 4.2:

A change in the maximum size of the dynamic table is signaled via a dynamic table size update (see Section 6.3). This dynamic table size update MUST occur at the beginning of the first header block following the change to the dynamic table size. In HTTP/2, this follows a settings acknowledgment (see Section 6.5.3 of [HTTP2])
As I understand this, when HTTP/2 sends a SETTINGS frame with a SETTINGS_HEADER_TABLE_SIZE change, only then it is mandatory (MUST) for the headers block to start with the dynamic table size update (notice the word "this" I highlighted in the quote). All the other dynamic table size updates can be in any location in the headers block.

client tests?

As of now, h2spec is only capable of testing servers, acting as a client. IMO it would be great if h2spec could be run as a server, so that it could be used as a tool to test client implementations.

Can the output have the summary last?

73 tests, 29 passed, 0 skipped, 44 failed

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

  4.2. Frame Size
    × Sends large size frame that exceeds the SETTINGS_MAX_FRAME_SIZE
      - The endpoint MUST send a FRAME_SIZE_ERROR error.
...

The summary is buried under 300 lines of failure details. Is it possible to print the summary after the details?

generic/3.10 expects response without sending END_STREAM

    3.10. CONTINUATION
      × 1: Sends a CONTINUATION frame
        -> The endpoint MUST accept CONTINUATION frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Timeout
      × 2: Sends multiple CONTINUATION frames
        -> The endpoint MUST accept multiple CONTINUATION frames.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Timeout

Hypertext Transfer Protocol Version 2 (HTTP/2)
  6. Frame Definitions
    6.10. CONTINUATION
      × 1: Sends multiple CONTINUATION frames preceded by a HEADERS frame
        -> The endpoint must accept the frame.
           Expected: HEADERS Frame (stream_id:1)
             Actual: Timeout

Server debug logs for 3.10 section 1:

NEW CONNECTION	http.h2_connection{type="server"}
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	FLAGS=0
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=0	StreamID=0
WRITE	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	FLAGS=1
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=0;state="idle";parent=nil;dependees={}}	SETTING	Flags=1	StreamID=0
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="idle";parent=0;dependees={}}	HEADERS	Flags=1	StreamID=1
HANDLING	http.h2_stream{connection=http.h2_connection{type="server"};id=1;state="idle";parent=0;dependees={}}	CONTINUATION	Flags=4	StreamID=1

This particular server then sits there waiting for a data chunk (as END_STREAM was not sent).
Eventually h2spec fails with a timeout error.

go get .../cmd/h2spec fails with h2spec.go vs h2specd.go conflict

Looks like the h2spec.go and h2specd.go files are both top-level main files in the same package-directory. The h2specd.go should probably be in a separate cmd/h2specd package, in order to keep the package go get-able?

Mind, that wouldn't respect the glide.yaml...

$ go get github.com/summerwind/h2spec/cmd/h2spec
# github.com/summerwind/h2spec/cmd/h2spec
../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2specd.go:14: VERSION redeclared in this block
	previous declaration at ../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2spec.go:14
../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2specd.go:15: COMMIT redeclared in this block
	previous declaration at ../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2spec.go:15
../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2specd.go:18: main redeclared in this block
	previous declaration at ../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2spec.go:18
../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2specd.go:55: run redeclared in this block
	previous declaration at ../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2spec.go:50
../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2specd.go:161: version redeclared in this block
	previous declaration at ../../go/src/github.com/summerwind/h2spec/cmd/h2spec/h2spec.go:143

Order of response frames

Hello, thanks for your help.

Async::HTTP::Protocol::HTTP2::Server new write_frame: class=HTTP::Protocol::HTTP2::SettingsFrame flags=0 length=24
Async::HTTP::Protocol::HTTP2::Server new read_frame: class=HTTP::Protocol::HTTP2::SettingsFrame flags=0 length=6
6 > 16383
Async::HTTP::Protocol::HTTP2::Server new write_frame: class=HTTP::Protocol::HTTP2::SettingsFrame flags=1 length=0
Async::HTTP::Protocol::HTTP2::Server new read_frame: class=HTTP::Protocol::HTTP2::SettingsFrame flags=1 length=0
0 > 16383
Async::HTTP::Protocol::HTTP2::Server open read_frame: class=HTTP::Protocol::HTTP2::HeadersFrame flags=4 length=15
15 > 1048576
Async::HTTP::Protocol::HTTP2::Server open write_frame: class=HTTP::Protocol::HTTP2::HeadersFrame flags=4 length=37
Async::HTTP::Protocol::HTTP2::Server open write_frame: class=HTTP::Protocol::HTTP2::DataFrame flags=0 length=11
Async::HTTP::Protocol::HTTP2::Server open write_frame: class=HTTP::Protocol::HTTP2::DataFrame flags=1 length=0
Async::HTTP::Protocol::HTTP2::Server open read_frame: class=HTTP::Protocol::HTTP2::DataFrame flags=1 length=1048577
1048577 > 1048576
Async::HTTP::Protocol::HTTP2::Server open write_frame: class=HTTP::Protocol::HTTP2::GoawayFrame flags=0 length=48

I am running

x_x > ./h2spec --host localhost --port 9292 --tls http2/4.2/2
Hypertext Transfer Protocol Version 2 (HTTP/2)
  4. HTTP Frames
    4.2. Frame Size
      × 2: Sends a large size DATA frame that exceeds the SETTINGS_MAX_FRAME_SIZE
        -> The endpoint MUST send an error code of FRAME_SIZE_ERROR.
           Expected: GOAWAY Frame (Error Code: FRAME_SIZE_ERROR)
                     RST_STREAM Frame (Error Code: FRAME_SIZE_ERROR)
                     Connection closed
             Actual: DATA Frame (length:0, flags:0x01, stream_id:1)

This happens because I send a response before reading the request data, because that response does not require request body.

It seems like h2spec should be capable of consuming valid frames until finally GOAWAY, RST_STREAM, etc is received. WDYT?

3.5 & 5.4.1 should allow GOAWAY

It's possible I'm missing something here but it seems like GOAWAY should be allowed in these two tests. The only other tests where VerifyConnectionClose seems to be used are ones where GOAWAY is first sent by h2spec.

3.5:

Clients and servers MUST treat an invalid connection preface as a connection error (Section 5.4.1) of type PROTOCOL_ERROR. A GOAWAY frame (Section 6.8) MAY be omitted in this case, since an invalid preface indicates that the peer is not using HTTP/2.

and

5.4.1:

An endpoint that encounters a connection error SHOULD first send a GOAWAY frame (Section 6.8) with the stream identifier of the last stream that it successfully received from its peer. The GOAWAY frame includes an error code that indicates why the connection is terminating. After sending the GOAWAY frame for an error condition, the endpoint MUST close the TCP connection.

JUnit fails to report test errors: error is *errors.errorString, not *spec.TestError

JUnit reporter can handle test Pass and test Failure but it cannot handle a test Error, e.g. server timeout, misconfiguration, arbitrary test implementation error.

Example

/home/karm/go/src/github.com/Karm/h2spec/bin/h2spec -h 192.168.122.204 -k --junit-report junit.report -p 3443 -t -v

Actual result

  5.4.1. Connection Error Handling
    × 1: Sends invalid frame for connection close
    Error: <nil>

panic: interface conversion: error is *errors.errorString, not *spec.TestError

goroutine 1 [running]:
panic(0x6faaa0, 0xc4202a8980)
    /opt/go/src/runtime/panic.go:500 +0x1a1
github.com/summerwind/h2spec/reporter.convertJUnitReport(0xc42006f490, 0x2, 0x2, 0x0, 0x0, 0x0)
    /home/karm/go/src/github.com/Karm/h2spec/reporter/junit_report.go:102 +0xb36
github.com/summerwind/h2spec/reporter.convertJUnitReport(0xc42010c9a0, 0x3, 0x4, 0x0, 0x0, 0x0)
    /home/karm/go/src/github.com/Karm/h2spec/reporter/junit_report.go:69 +0x1a5
github.com/summerwind/h2spec/reporter.convertJUnitReport(0xc42004bba0, 0x1, 0x1, 0x0, 0x0, 0x3)
    /home/karm/go/src/github.com/Karm/h2spec/reporter/junit_report.go:69 +0x1a5
github.com/summerwind/h2spec/reporter.JUnitReport(0xc42004bba0, 0x1, 0x1, 0x7ffd6e1e563e, 0x1, 0xc4202bae60, 0x1b)
    /home/karm/go/src/github.com/Karm/h2spec/reporter/junit_report.go:51 +0x5d
github.com/summerwind/h2spec.Run(0xc42006a7e0, 0xc42006a7e0, 0x9)
    /home/karm/go/src/github.com/Karm/h2spec/h2spec.go:37 +0x345
main.run(0xc420071440, 0xc4200982d0, 0x0, 0x9, 0x0, 0x0)
    /home/hudson/go/src/github.com/Karm/h2spec/cmd/h2spec/h2spec.go:137 +0x49e
github.com/spf13/cobra.(*Command).execute(0xc420071440, 0xc420082010, 0x9, 0x9, 0xc420071440, 0xc420082010)
    /home/hudson/go/src/github.com/spf13/cobra/command.go:632 +0x23e
github.com/spf13/cobra.(*Command).ExecuteC(0xc420071440, 0x752cbf, 0x1a, 0xc42006f3fa)
    /home/hudson/go/src/github.com/spf13/cobra/command.go:722 +0x367
github.com/spf13/cobra.(*Command).Execute(0xc420071440, 0x74b5f2, 0x4)
    /home/hudson/go/src/github.com/spf13/cobra/command.go:681 +0x2b
main.main()
    /home/hudson/go/src/github.com/Karm/h2spec/cmd/h2spec/h2spec.go:44 +0x454

Expected

Correct JUnit report saying that such and such test ended up with an Error, not Failure. e.g. Timeout:

<testsuite name="5.1.2. Stream Concurrency" package="http2/5.1.2" id="5.1.2" tests="1" skipped="0" failures="1" errors="0">
  <testcase package="http2/5.1.2" classname="Sends HEADERS frames that causes their advertised concurrent stream limit to be exceeded" time="2.0001">
    <failure>Test in Error: Timeout</failure>
  </testcase>
</testsuite>

Error code for 6.5 SETTINGS Sends a incomplete SETTINGS frame

Currently, the test case requires PROTOCOL_ERROR.
I could not find the reference of it, but it seems FRAME_SIZE_ERROR is also a good candidate.

4.2.  Frame Size
   ...
   If a frame size exceeds any defined limit, or is too small to contain
   mandatory frame data, the endpoint MUST send a FRAME_SIZE_ERROR
   error.  A frame size error in a frame that could alter the state of
   the entire connection MUST be treated as a connection error
   (Section 5.4.1); this includes any frame carrying a header block
   (Section 4.3) (that is, HEADERS, PUSH_PROMISE, and CONTINUATION),
   SETTINGS, and any WINDOW_UPDATE frame with a stream identifier of 0.

Thoughts?

Test 6.9.1 does not allow treating stream errors as connection errors.

Test 6.9.1 appears not to allow a connection error in place of a stream error:

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

  6.9. WINDOW_UPDATE
    6.9.1. The Flow Control Window
      × 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: GOAWAY frame (Length: 8, Flags: 0, ErrorCode: FLOW_CONTROL_ERROR)

This is out of line with several of the other tests which do allow this logic, as discussed in #10.

I'll see if I can provide a pull request to fix this logic.

Trailing header test passes when we send a RST_STREAM frame is sent

Trailing header test passes when we send a RST_STREAM frame is sent before the HEADERS and DATA sent.

h2spec running:
[bcall@homer cmd]$ ./h2spec -t -k -p 4443 -h 127.0.0.1 -s 8.1
8.1. HTTP Request/Response Exchange
✓ Sends a HEADERS frame as HEAD request
✓ Sends a HEADERS frame containing trailer part

68 tests, 2 passed, 66 skipped, 0 failed
All tests passed

Headers sent from client:
[Nov 5 22:33:00.975] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:178 (rcv_headers_frame)> (http2_cs) [2] Received HEADERS frame.
[Nov 5 22:33:00.975] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:72 (rcv_data_frame)> (http2_cs) [2] Received DATA frame.
[Nov 5 22:33:00.975] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:178 (rcv_headers_frame)> (http2_cs) [2] Received HEADERS frame.

Headers send from server:
[Nov 5 22:33:00.975] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:1012 (send_rst_stream_frame)> (http2_cs) [2] Send RST_STREAM frame.
[Nov 5 22:33:00.976] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:960 (send_headers_frame)> (http2_cs) [2] Send HEADERS frame.
[Nov 5 22:33:00.976] Server {0x7f18d532c840} DEBUG: <Http2ConnectionState.cc:895 (send_data_frame)> (http2_cs) [2] Send DATA frame

HTTP/2 5.1/5 is racy

I am getting the following failure:

    × 5: half closed (remote): Sends a DATA frame
        -> The endpoint MUST respond with a stream error of type STREAM_CLOSED.
           Expected: GOAWAY Frame (Error Code: STREAM_CLOSED)
                     RST_STREAM Frame (Error Code: STREAM_CLOSED)
                     Connection closed
             Actual: WINDOW_UPDATE Frame (length:4, flags:0x00, stream_id:0)

My understanding is that WINDOW_UPDATE is allowed per 5.1 half-closed (local):
In this state, a receiver can ignore WINDOW_UPDATE frames, which might arrive for a short period after a frame bearing the END_STREAM flag is sent.

Set non-zero return code if tests fail.

I'd like to add h2spec to the hyper-h2 CI test suite, but right now the only way I can do that is by checking the output for a non-zero number of failures. It would be nicer if h2spec would return a non-zero return code when some number of tests fail so that the CI tools will automatically spot that.

Execute only selected tests

Most likely we would like to run failed test only, or just one selected test to make server log inspection easier. curl test suite has this feature.

Add a test for sending an initial frame other than SETTINGS?

The spec says "A SETTINGS frame MUST be sent by both endpoints at the start of a connection". How about adding a test that verifies conformance for this? I'm guessing if a non-SETTINGS frame arrives first the server could GOAWAY with PROTOCOL_ERROR?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.