Go: x/net/http2: HTTP/2 conformance failures using h2spec

Created on 24 Apr 2018  路  25Comments  路  Source: golang/go

What version of Go are you using (go version)?

go version 1.10.1 linux/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="arm64"
GOBIN=""
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/XXXX/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_arm64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build393280655=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

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

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

What did you expect to see?

146 tests, 146 passed, 0 skipped, 0 failed

What did you see instead?

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)

146 tests, 137 passed, 0 skipped, 9 failed

As a Caddy contributor, I'm redirecting the original issue https://github.com/mholt/caddy/issues/2132 here because it's an upstream issue

cc @mholt @bazzadp

FrozenDueToAge NeedsFix help wanted
Source
picture elcore

Most helpful comment

These would be nice to fix but I don't think any of them are critical. It's nice that we do better than google.com at least. :-)

I agree let's just keep this one bug for all of them.

/cc @tombergan

picture bradfitz on 4 May 2018
👍3

All 25 comments

I see similar failures even without strict mode. Only difference is that in my runs 6.9.1 passes but 5.1x6 fails.

ping @bradfitz.

agnivade picture agnivade on 24 Apr 2018

Running against http2.golang.org gives same success grade 137/146, but slightly different failures. (Though, I'm guessing caddyserver.com is using a newer version.)

In terms of priority:

  • As bazzadp noted in the caddy counterpart issue, "most of these tests are about how to handle bad client requests".
  • google.com has a lower success grade: 135/146.

Probably best to keep this as one issue which each CL Updates. Maybe also add help wanted.

meirf picture meirf on 4 May 2018

These would be nice to fix but I don't think any of them are critical. It's nice that we do better than google.com at least. :-)

I agree let's just keep this one bug for all of them.

/cc @tombergan

bradfitz picture bradfitz on 4 May 2018
👍3

Change https://golang.org/cl/111675 mentions this issue: x/net/http2: correct overflow protection

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111676 mentions this issue: x/net/http2: a closed stream cannot receive data

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111677 mentions this issue: x/net/http2: headers cannot be received on half closed streams

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111678 mentions this issue: x/net/http2: send GOAWAY when headers exceed the max frame size

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111679 mentions this issue: x/net/http2: receiving too much data is a protocol error

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111680 mentions this issue: x/net/http2: reject connection-level headers with a protocol error

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/111681 mentions this issue: x/net/http2: dynamic table updates must occur first

gopherbot picture gopherbot on 5 May 2018

Change https://golang.org/cl/121415 mentions this issue: http2: make Server send GOAWAY if Handler sets "Connection: close" header

gopherbot picture gopherbot on 28 Jun 2018

Change https://golang.org/cl/122877 mentions this issue: net/http: update bundled http2

gopherbot picture gopherbot on 10 Jul 2018

Can we get an updated summary of where we're at on this bug using the latest (master) code?

bradfitz picture bradfitz on 11 Jul 2018
❤1

Sure

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:14, flags:0x01, stream_id:1)

  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)

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:14, flags:0x01, stream_id:1)

Finished in 14.6203 seconds
146 tests, 142 passed, 0 skipped, 4 failed
elcore picture elcore on 11 Jul 2018
👍2

Okay, at this point we have few enough, let's file separate bugs for them, with titles of the form:

   x/net/http2: h2spec violation 8.1.2.2

@fraenkel, you want to file those?

IIRC, one of those @fraenkel already identified as a bug in the h2spec test. If so, let's file the bug anyway just for reference and we can contact the h2spec authors to get it fixed.

bradfitz picture bradfitz on 11 Jul 2018

HTTP 8.1.2.x is https://go-review.googlesource.com/c/net/+/111680

HTTP 4.2.3 is an incorrect test, it is using the wrong frame size, see https://go-review.googlesource.com/c/net/+/111678

HPACK 4.2.1 I need to investigate. We have a fix in place but either its not being triggered or it could be a sequencing issue.

Will go do some digging...

fraenkel picture fraenkel on 11 Jul 2018

Hmm, I just ran the latest h2specs with Go 1.10.3 + latest http2 and Go 1.11(tip) and got different results than above as well as between the 2 releases and between runs. I will see anywhere from 3 - 6 failures.
Running with GODEBUG creates more consistent failures.

fraenkel picture fraenkel on 11 Jul 2018

The most errors I see are below. HTTP/2 6.10, HTTP/2 8.1.2.2.1 and HTTP/2 8.1.2.2.2 always occur.
HTTP/2 8.1.2.2.x are fixed with the CL above.

Failures: 

Hypertext Transfer Protocol Version 2 (HTTP/2)
  5. Streams and Multiplexing
    5.1. Stream States
      脳 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: DATA Frame (length:42, flags:0x01, stream_id:1)
      脳 6: half closed (remote): Sends a HEADERS 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: DATA Frame (length:42, flags:0x01, stream_id:1)

  6. Frame Definitions
    6.1. DATA
      脳 2: Sends a DATA frame on the stream that is not in "open" or "half-closed (local)" state
        -> 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: DATA Frame (length:42, flags:0x01, stream_id:1)

    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: Connection closed

  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)
fraenkel picture fraenkel on 11 Jul 2018

I have created

26321 - 8.1.2.2 (which is has a CL)

26322 - 5.1, I believe is an h2spec issue

26323 - 6.1.2, also an h2spec issue

26324 - 6.10.1 (needs investigation)

The others I do not see. I am using a simple h2 server, not caddy.

fraenkel picture fraenkel on 11 Jul 2018

When I switched my handler to return no body, 2 more failures appeared and the causes for the others changed.

26326 generic 2/2 & 2/3

26327 http/2 5.1.2

fraenkel picture fraenkel on 11 Jul 2018

@fraenkel, thanks. I'm going to close this meta bug, but if you or others open other h2spec bugs, feel free to reference this bug in the new bug, or reference the new bug from this bug so they're all linked together. (Or we can search for "h2spec", too.)

bradfitz picture bradfitz on 11 Jul 2018

Change https://golang.org/cl/123515 mentions this issue: net/http: update bundled http2

gopherbot picture gopherbot on 12 Jul 2018

Change https://golang.org/cl/127355 mentions this issue: vendor: update golang.org/x/net/http2/hpack

gopherbot picture gopherbot on 1 Aug 2018

Change https://golang.org/cl/153977 mentions this issue: http2: Revert a closed stream cannot receive data

gopherbot picture gopherbot on 13 Dec 2018

Change https://golang.org/cl/153978 mentions this issue: http2/hpack: track the beginning of a header block

gopherbot picture gopherbot on 13 Dec 2018
Was this page helpful?
0 / 5 - 0 ratings

Related issues

michaelsafyan picture michaelsafyan  路  3Comments
longzhizhi picture longzhizhi  路  3Comments
dominikh picture dominikh  路  3Comments
go101 picture go101  路  3Comments
gopherbot picture gopherbot  路  3Comments