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.

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

improve ci/h2spec.sh (macOS compat, /tmp dir and overwrite) #809

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

GlenDC
Copy link
Contributor

@GlenDC GlenDC commented Nov 4, 2024

  • detect if run on MacOS, so we download h2spec macos build in that case
  • support overwriting h2spec detection so we anyway download new file, useful in case you switch to new version for example
  • move h2spec, archive and log all to /tmp dir as to not polute the repo dir

Note one or more follow up tasks might be required because this is also using an out of date h2spec archive. We are already a handful of major versions further. I tried to switch it over but there were a lot of failures. Is that because of:

  • a) how we use h2spec
  • b) how we define the h2-server example
  • c) bugs in the h2 crate?

Some guidance is appreciated here and if you have me I could help contribute some or all fixes for this? At the very least this hopefully gets the ball rolling on this as I do have intuitively the feeling that we might want to stay up to date with a tool we use, especially when it tests for h2spec compatibility / correctness?

This is however separate from this patch as this are just some improvements that allow it to be run on my dev machine as well as in CI, it should not affect the existing usage by (linux) users.

- detect if run on MacOS, so we download h2spec macos build in that case
- support overwriting h2spec detection so we anyway download new file,
  useful in case you switch to new version for example
- move h2spec, archive and log all to /tmp dir as to not polute
  the repo dir
@seanmonstar
Copy link
Member

We just haven't remembered to keep up to date. What are the failures?

@GlenDC
Copy link
Contributor Author

GlenDC commented Nov 4, 2024

We just haven't remembered to keep up to date. What are the failures?

I mentioned it here, but either way I do think it deserves a separate PR.

Diff:

diff --git a/ci/h2spec.sh b/ci/h2spec.sh
index 9b3e14b..4cb08bd 100755
--- a/ci/h2spec.sh
+++ b/ci/h2spec.sh
@@ -14,10 +14,10 @@ done
 if ! [ -e "/tmp/h2spec" ] || $override_h2spec ; then
     # if we don't already have a h2spec executable, wget it from github
     if [[ "$OSTYPE" == "darwin"* ]]; then
-        curl -L -o /tmp/h2spec_darwin_amd64.tar.gz https://github.com/summerwind/h2spec/releases/download/v2.1.1/h2spec_darwin_amd64.tar.gz \
+        curl -L -o /tmp/h2spec_darwin_amd64.tar.gz https://github.com/summerwind/h2spec/releases/download/v2.6.0/h2spec_darwin_amd64.tar.gz \
             && tar xf /tmp/h2spec_darwin_amd64.tar.gz -C /tmp
     else
-        curl -L -o /tmp/h2spec_linux_amd64.tar.gz https://github.com/summerwind/h2spec/releases/download/v2.1.1/h2spec_linux_amd64.tar.gz \
+        curl -L -o /tmp/h2spec_linux_amd64.tar.gz https://github.com/summerwind/h2spec/releases/download/v2.6.0/h2spec_linux_amd64.tar.gz \
             && tar xf /tmp/h2spec_linux_amd64.tar.gz -C /tmp
     fi
 fi

Cmd + Output:

bash ci/h2spec.sh -F
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 5101k  100 5101k    0     0  3463k      0  0:00:01  0:00:01 --:--:-- 5879k
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.18s
Error: Os { code: 48, kind: AddrInUse, message: "Address already in use" }
Generic tests for HTTP/2 server
  1. Starting HTTP/2
    ✔ 1: Sends a client connection preface

  2. Streams and Multiplexing
    ✔ 1: Sends a PRIORITY frame on idle stream
    ✔ 2: Sends a WINDOW_UPDATE frame on half-closed (remote) stream
    ✔ 3: Sends a PRIORITY frame on half-closed (remote) stream
    ✔ 4: Sends a RST_STREAM frame on half-closed (remote) stream
    ✔ 5: Sends a PRIORITY frame on closed stream

  3. Frame Definitions
    3.1. DATA
      ✔ 1: Sends a DATA frame
      ✔ 2: Sends multiple DATA frames
      ✔ 3: Sends a DATA frame with padding

    3.2. HEADERS
      ✔ 1: Sends a HEADERS frame
      ✔ 2: Sends a HEADERS frame with padding
      ✔ 3: Sends a HEADERS frame with priority

    3.3. PRIORITY
      ✔ 1: Sends a PRIORITY frame with priority 1
      ✔ 2: Sends a PRIORITY frame with priority 256
      ✔ 3: Sends a PRIORITY frame with stream dependency
      ✔ 4: Sends a PRIORITY frame with exclusive
      ✔ 5: Sends a PRIORITY frame for an idle stream, then send a HEADER frame for a lower stream ID

    3.4. RST_STREAM
      ✔ 1: Sends a RST_STREAM frame

    3.5. SETTINGS
      ✔ 1: Sends a SETTINGS frame

    3.7. PING
      ✔ 1: Sends a PING frame

    3.8. GOAWAY
      using source address 127.0.0.1:61897
      × 1: Sends a GOAWAY frame
        -> The endpoint MUST accept GOAWAY frame.
           Expected: Connection closed
                     PING Frame (length:8, flags:0x01, stream_id:0, opaque_data:h2spec)
             Actual: Error: read tcp 127.0.0.1:61897->127.0.0.1:5928: read: connection reset by peer

    3.9. WINDOW_UPDATE
      ✔ 1: Sends a WINDOW_UPDATE frame with stream ID 0
      ✔ 2: Sends a WINDOW_UPDATE frame with stream ID 1

    3.10. CONTINUATION
      ✔ 1: Sends a CONTINUATION frame
      ✔ 2: Sends multiple CONTINUATION frames

  4. HTTP Message Exchanges
    ✔ 1: Sends a GET request
    ✔ 2: Sends a HEAD request
    ✔ 3: Sends a POST request
    ✔ 4: Sends a POST request with trailers

  5. HPACK
    ✔ 1: Sends a indexed header field representation
    ✔ 2: Sends a literal header field with incremental indexing - indexed name
    ✔ 3: Sends a literal header field with incremental indexing - indexed name (with Huffman coding)
    ✔ 4: Sends a literal header field with incremental indexing - new name
    ✔ 5: Sends a literal header field with incremental indexing - new name (with Huffman coding)
    ✔ 6: Sends a literal header field without indexing - indexed name
    ✔ 7: Sends a literal header field without indexing - indexed name (with Huffman coding)
    ✔ 8: Sends a literal header field without indexing - new name
    ✔ 9: Sends a literal header field without indexing - new name (huffman encoded)
    ✔ 10: Sends a literal header field never indexed - indexed name
    ✔ 11: Sends a literal header field never indexed - indexed name (huffman encoded)
    ✔ 12: Sends a literal header field never indexed - new name
    ✔ 13: Sends a literal header field never indexed - new name (huffman encoded)
    ✔ 14: Sends a dynamic table size update
    ✔ 15: Sends multiple dynamic table size update

Hypertext Transfer Protocol Version 2 (HTTP/2)
  3. Starting HTTP/2
    3.5. HTTP/2 Connection Preface
      ✔ 1: Sends client connection preface
      using source address 127.0.0.1:61922e
      × 2: Sends invalid connection preface
        -> The endpoint MUST terminate the TCP connection.
           Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                     Connection closed
             Actual: Error: read tcp 127.0.0.1:61922->127.0.0.1:5928: read: connection reset by peer

  4. HTTP Frames
    4.1. Frame Format
      ✔ 1: Sends a frame with unknown type
      ✔ 2: Sends a frame with undefined flag
      ✔ 3: Sends a frame with reserved field bit

    4.2. Frame Size
      ✔ 1: Sends a DATA frame with 2^14 octets in length
      ✔ 2: Sends a large size DATA frame that exceeds the SETTINGS_MAX_FRAME_SIZE
      ✔ 3: Sends a large size HEADERS frame that exceeds the SETTINGS_MAX_FRAME_SIZE

    4.3. Header Compression and Decompression
      ✔ 1: Sends invalid header block fragment
      ✔ 2: Sends a PRIORITY frame while sending the header blocks
      ✔ 3: Sends a HEADERS frame to another stream while sending the header blocks

  5. Streams and Multiplexing
    5.1. Stream States
      ✔ 1: idle: Sends a DATA frame
      ✔ 2: idle: Sends a RST_STREAM frame
      ✔ 3: idle: Sends a WINDOW_UPDATE frame
      ✔ 4: idle: Sends a CONTINUATION frame
      ✔ 5: half closed (remote): Sends a DATA frame
      ✔ 6: half closed (remote): Sends a HEADERS frame
      ✔ 7: half closed (remote): Sends a CONTINUATION frame
      ✔ 8: closed: Sends a DATA frame after sending RST_STREAM frame
      ✔ 9: closed: Sends a HEADERS frame after sending RST_STREAM frame
      ✔ 10: closed: Sends a CONTINUATION frame after sending RST_STREAM frame
      ✔ 11: closed: Sends a DATA frame
      ✔ 12: closed: Sends a HEADERS frame
      ✔ 13: closed: Sends a CONTINUATION frame

      5.1.1. Stream Identifiers
        ✔ 1: Sends even-numbered stream identifier
        ✔ 2: Sends stream identifier that is numerically smaller than previous

      5.1.2. Stream Concurrency
        1: Sends HEADERS frames that causes their advertised concurrent stream limit to be exceededed

    5.3. Stream Priority
      5.3.1. Stream Dependencies
        ✔ 1: Sends HEADERS frame that depends on itself
        ✔ 2: Sends PRIORITY frame that depend on itself

    5.4. Error Handling
      5.4.1. Connection Error Handling
        ✔ 1: Sends an invalid PING frame for connection close

    5.5. Extending HTTP/2
      ✔ 1: Sends an unknown extension frame
      ✔ 2: Sends an unknown extension frame in the middle of a header block

  6. Frame Definitions
    6.1. DATA
      ✔ 1: Sends a DATA frame with 0x0 stream identifier
      ✔ 2: Sends a DATA frame on the stream that is not in "open" or "half-closed (local)" state
      ✔ 3: Sends a DATA frame with invalid pad length

    6.2. HEADERS
      ✔ 1: Sends a HEADERS frame without the END_HEADERS flag, and a PRIORITY frame
      ✔ 2: Sends a HEADERS frame to another stream while sending a HEADERS frame
      ✔ 3: Sends a HEADERS frame with 0x0 stream identifier
      ✔ 4: Sends a HEADERS frame with invalid pad length

    6.3. PRIORITY
      ✔ 1: Sends a PRIORITY frame with 0x0 stream identifier
      ✔ 2: Sends a PRIORITY frame with a length other than 5 octets

    6.4. RST_STREAM
      ✔ 1: Sends a RST_STREAM frame with 0x0 stream identifier
      ✔ 2: Sends a RST_STREAM frame on a idle stream
      ✔ 3: Sends a RST_STREAM frame with a length other than 4 octets

    6.5. SETTINGS
      ✔ 1: Sends a SETTINGS frame with ACK flag and payload
      ✔ 2: Sends a SETTINGS frame with a stream identifier other than 0x0
      ✔ 3: Sends a SETTINGS frame with a length other than a multiple of 6 octets

      6.5.2. Defined SETTINGS Parameters
        ✔ 1: SETTINGS_ENABLE_PUSH (0x2): Sends the value other than 0 or 1
        ✔ 2: SETTINGS_INITIAL_WINDOW_SIZE (0x4): Sends the value above the maximum flow control window size
        ✔ 3: SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value below the initial value
        ✔ 4: SETTINGS_MAX_FRAME_SIZE (0x5): Sends the value above the maximum allowed frame size
        ✔ 5: Sends a SETTINGS frame with unknown identifier

      6.5.3. Settings Synchronization
        ✔ 1: Sends multiple values of SETTINGS_INITIAL_WINDOW_SIZE
        ✔ 2: Sends a SETTINGS frame without ACK flag

    6.7. PING
      ✔ 1: Sends a PING frame
      ✔ 2: Sends a PING frame with ACK
      ✔ 3: Sends a PING frame with a stream identifier field value other than 0x0
      ✔ 4: Sends a PING frame with a length field value other than 8

    6.8. GOAWAY
      ✔ 1: Sends a GOAWAY frame with a stream identifier other than 0x0

    6.9. WINDOW_UPDATE
      ✔ 1: Sends a WINDOW_UPDATE frame with a flow control window increment of 0
      ✔ 2: Sends a WINDOW_UPDATE frame with a flow control window increment of 0 on a stream
      ✔ 3: Sends a WINDOW_UPDATE frame with a length other than 4 octets

      6.9.1. The Flow-Control Window
        ✔ 1: Sends SETTINGS frame to set the initial window size to 1 and sends HEADERS frame
        ✔ 2: Sends multiple WINDOW_UPDATE frames increasing the flow control window to above 2^31-1
        ✔ 3: Sends multiple WINDOW_UPDATE frames increasing the flow control window to above 2^31-1 on a stream

      6.9.2. Initial Flow-Control Window Size
        ✔ 1: Changes SETTINGS_INITIAL_WINDOW_SIZE after sending HEADERS frame
        ✔ 2: Sends a SETTINGS frame for window size to be negative
        ✔ 3: Sends a SETTINGS_INITIAL_WINDOW_SIZE settings with an exceeded maximum window size value

    6.10. CONTINUATION
      ✔ 1: Sends multiple CONTINUATION frames preceded by a HEADERS frame
      ✔ 2: Sends a CONTINUATION frame followed by any frame other than CONTINUATION
      ✔ 3: Sends a CONTINUATION frame with 0x0 stream identifier
      ✔ 4: Sends a CONTINUATION frame preceded by a HEADERS frame with END_HEADERS flag
      ✔ 5: Sends a CONTINUATION frame preceded by a CONTINUATION frame with END_HEADERS flag
      ✔ 6: Sends a CONTINUATION frame preceded by a DATA frame

  7. Error Codes
    ✔ 1: Sends a GOAWAY frame with unknown error code
    ✔ 2: Sends a RST_STREAM frame with unknown error code

  8. HTTP Message Exchanges
    8.1. HTTP Request/Response Exchange
      ✔ 1: Sends a second HEADERS frame without the END_STREAM flag

      8.1.2. HTTP Header Fields
        ✔ 1: Sends a HEADERS frame that contains the header field name in uppercase letters

        8.1.2.1. Pseudo-Header Fields
          ✔ 1: Sends a HEADERS frame that contains a unknown pseudo-header field
          ✔ 2: Sends a HEADERS frame that contains the pseudo-header field defined for response
          ✔ 3: Sends a HEADERS frame that contains a pseudo-header field as trailers
            4: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regul          ✔ 4: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field

        8.1.2.2. Connection-Specific Header Fields
          ✔ 1: Sends a HEADERS frame that contains the connection-specific header field
          ✔ 2: Sends a HEADERS frame that contains the TE header field with any value other than "trailers"

        8.1.2.3. Request Pseudo-Header Fields
          ✔ 1: Sends a HEADERS frame with empty ":path" pseudo-header field
          ✔ 2: Sends a HEADERS frame that omits ":method" pseudo-header field
          ✔ 3: Sends a HEADERS frame that omits ":scheme" pseudo-header field
          ✔ 4: Sends a HEADERS frame that omits ":path" pseudo-header field
          ✔ 5: Sends a HEADERS frame with duplicated ":method" pseudo-header field
          ✔ 6: Sends a HEADERS frame with duplicated ":scheme" pseudo-header field
          ✔ 7: Sends a HEADERS frame with duplicated ":path" pseudo-header field

        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 paylo          ✔ 1: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length
            2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multi          ✔ 2: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length

    8.2. Server Push
      ✔ 1: Sends a PUSH_PROMISE frame

HPACK: Header Compression for HTTP/2
  2. Compression Process Overview
    2.3. Indexing Tables
      2.3.3. Index Address Space
        ✔ 1: Sends a indexed header field representation with invalid index
        ✔ 2: Sends a literal header field representation with invalid index

  4. Dynamic Table Management
    4.2. Maximum Table Size
      ✔ 1: Sends a dynamic table size update at the end of header block

  5. Primitive Type Representations
    5.2. String Literal Representation
      ✔ 1: Sends a Huffman-encoded string literal representation with padding longer than 7 bits
      ✔ 2: Sends a Huffman-encoded string literal representation padded by zero
      ✔ 3: Sends a Huffman-encoded string literal representation containing the EOS symbol

  6. Binary Format
    6.1. Indexed Header Field Representation
      ✔ 1: Sends a indexed header field representation with index 0

    6.3. Dynamic Table Size Update
      ✔ 1: Sends a dynamic table size update larger than the value of SETTINGS_HEADER_TABLE_SIZE

Failures:

Generic tests for HTTP/2 server
  3. Frame Definitions
    3.8. GOAWAY
      using source address 127.0.0.1:61897
      × 1: Sends a GOAWAY frame
        -> The endpoint MUST accept GOAWAY frame.
           Expected: Connection closed
                     PING Frame (length:8, flags:0x01, stream_id:0, opaque_data:h2spec)
             Actual: Error: read tcp 127.0.0.1:61897->127.0.0.1:5928: read: connection reset by peer

Hypertext Transfer Protocol Version 2 (HTTP/2)
  3. Starting HTTP/2
    3.5. HTTP/2 Connection Preface
      using source address 127.0.0.1:61922
      × 2: Sends invalid connection preface
        -> The endpoint MUST terminate the TCP connection.
           Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
                     Connection closed
             Actual: Error: read tcp 127.0.0.1:61922->127.0.0.1:5928: read: connection reset by peer

Finished in 0.0471 seconds
146 tests, 143 passed, 1 skipped, 2 failed
h2spec failed! server logs:
ci/h2spec.sh: line 42: kill: (79131) - No such process

h2server.log:

cat /tmp/h2server.log

@seanmonstar
Copy link
Member

Sure, separate PR fixing those and upgrading would be great! I don't think they're concerning, it looks like differences on error conditions. Would you want to investigate and try to fix?

@seanmonstar seanmonstar merged commit 848885b into hyperium:master Nov 4, 2024
6 checks passed
@GlenDC
Copy link
Contributor Author

GlenDC commented Nov 4, 2024

Sure, separate PR fixing those and upgrading would be great! I don't think they're concerning, it looks like differences on error conditions. Would you want to investigate and try to fix?

Sure thing, added it to my whiteboard backlog, for a little snack this week. Will follow up. Thx!

@GlenDC GlenDC deleted the patch/h2spec-sh-improvements branch November 4, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants