HTTP/2: Many specification compliancy issues #5761
Comments
|
@buchgr thanks for having a look! I think I should write a maven plugin to do this as part of our build just as we do with the autobahn test suite (web sockets) |
|
@normanmaurer ... and first fix all the errors 😜 |
|
@buchgr great, thanks for looking at this! Yes ... fixing errors sgtm 😜 |
|
Working on a fix. Most errors seem pretty straightfoward... |
|
Thanks a lot!
|
|
Everybody here, my platform in addition to use asynchronous circuit also used in the synchronous circuit, and I did not find netty synchronous architecture, thinking, I realized a framework, called net Rio, it to NiO based synchronization response mechanism, basic to meet the needs of our platform. The project was open source, hope you want to join |
|
thanks for uncovering and patching @buchgr ! |
Motivation: h2spec [1] reported 32 issues [2] with Netty's HTTP/2 implementation. Modifications: Fixed 11 of those issues. Mostly wrong error codes or added missing validation code in the frame reader. Result: 11 fewer errors. h2spec now reports: 70 tests, 48 passed, 1 skipped, 21 failed [1] https://github.com/summerwind/h2spec [2] netty#5761
Motivation: h2spec [1] reported 32 issues [2] with Netty's HTTP/2 implementation. Modifications: Fixed 11 of those issues. Mostly wrong error codes or added missing validation code in the frame reader. Result: 11 fewer errors. h2spec now reports: 70 tests, 48 passed, 1 skipped, 21 failed [1] https://github.com/summerwind/h2spec [2] #5761
|
Running When increasing the |
|
May I ask that if I could help on this issue? |
|
@chhsiao90 sure just open PRs, we love contributions |
Motivation: h2spec [1] reported 32 issues [2] with Netty's HTTP/2 implementation. Modifications: Fixed 11 of those issues. Mostly wrong error codes or added missing validation code in the frame reader. Result: 11 fewer errors. h2spec now reports: 70 tests, 48 passed, 1 skipped, 21 failed [1] https://github.com/summerwind/h2spec [2] netty#5761
Motivation: Netty HTTP/2 implementation is not 100% compliant to the spec. This commit improves the compliance regarding headers validation, in particular pseudo-headers and connection ones. According to the spec: 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). Modifications: - Introduce Http2HeadersValidator class capable of validating HTTP/2 headers - Invoke validation from DefaultHttp2ConnectionDecoder#onHeadersRead - Modify tests to use valid headers when required - Modify HttpConversionUtil#toHttp2Headers to not add :scheme and :path header on CONNECT method in order to conform to the spec Result: - Initial requests without :method, :path, :scheme will fail - Initial requests with multiple values for :method, :path, :scheme will fail - Initial requests with an empty :path fail - Requests with connection-specific header field will fail - Requests with TE header different than "trailers" will fail - - Fixes 8.1.2.2 tests from h2spec netty#5761 - Fixes 8.1.2.3 tests from h2spec netty#5761
Motivation: Netty HTTP/2 implementation is not 100% compliant to the spec. This commit improves the compliance regarding headers validation, in particular pseudo-headers and connection ones. According to the spec: 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). Modifications: - Introduce Http2HeadersValidator class capable of validating HTTP/2 headers - Invoke validation from DefaultHttp2ConnectionDecoder#onHeadersRead - Modify tests to use valid headers when required - Modify HttpConversionUtil#toHttp2Headers to not add :scheme and :path header on CONNECT method in order to conform to the spec Result: - Initial requests without :method, :path, :scheme will fail - Initial requests with multiple values for :method, :path, :scheme will fail - Initial requests with an empty :path fail - Requests with connection-specific header field will fail - Requests with TE header different than "trailers" will fail - - Fixes 8.1.2.2 tests from h2spec #5761 - Fixes 8.1.2.3 tests from h2spec #5761
Motivation: Netty HTTP/2 implementation is not 100% compliant to the spec. This commit improves the compliance regarding headers validation, in particular pseudo-headers and connection ones. According to the spec: 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). Modifications: - Introduce Http2HeadersValidator class capable of validating HTTP/2 headers - Invoke validation from DefaultHttp2ConnectionDecoder#onHeadersRead - Modify tests to use valid headers when required - Modify HttpConversionUtil#toHttp2Headers to not add :scheme and :path header on CONNECT method in order to conform to the spec Result: - Initial requests without :method, :path, :scheme will fail - Initial requests with multiple values for :method, :path, :scheme will fail - Initial requests with an empty :path fail - Requests with connection-specific header field will fail - Requests with TE header different than "trailers" will fail - - Fixes 8.1.2.2 tests from h2spec #5761 - Fixes 8.1.2.3 tests from h2spec #5761
I ran h2spec against the hello world HTTP/2 server (4.1 branch, with removed HTTP upgrade logic). The result: 70 tests, 37 passed, 1 skipped, 32 failed.
From skimming over the list of errors, the failed tests seem to be mostly due to Netty responding with wrong error codes. Here is a detailed list of what went wrong.
cc: @Scottmitch @nmittler @louiscryan @ejona86
The text was updated successfully, but these errors were encountered: