HTTP/2 support - Githubissues

hidingturtle commented 1 year ago

PR Checklist

[x] Applies to work item: engine/transport_layer
[x] CLA signed. If not, go over here and sign.
[x] Tests added/passed.
[x] Requires documentation to be updated
[x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different approach. Issue number where discussion took place: [WIP] #277: HTTP2 support

Info on Pull Request

This pull request includes the first iteration of HTTP20Sock, which aims to implement HTTP/2 support for RESTler. However, HTTP/2 is more complicated than HTTP/1.1 (e.g. the state machine of HTTP/2 and streams), which means that unique situations could lead to RESTler crashing or being stuck in a loop since I could not test it in the individual unique situation. I use the h2 library for the HTTP/2 connections. Since it is relatively low-level, malformed requests should work library-wise.

h2 uses 2-Tuples since the HTTP/2 headers must be ordered (:path, :method must be first etc). I decided to "parse" these and stringify those tuples to HTTP/1.1 headers in order to work with HttpResponse. However, it is also possible to create another Http20Response class which works with the Tuples directly. I would need feedback for this decision. I did not want to modify many places in the source code, since RESTler is mostly hardcoded for HTTP/1.1.

I added a --http2 flag (in the Python and F# part).

Validation Steps Performed

I mostly used Open5GS with the Nnrf_NFManagement OpenAPI spec. However, Open5GS has problems with DATA frames bigger than 8192 Bytes. Since it does not share that limitation via the SETTINGS frame, Open5GS crashes once RESTler sends the PUT request in the aforementioned OpenAPI spec. RESTler works in such cases.

To validate, install the restler/requirements.txt and add the --http2 flag to your restler engine command.

Comment

I intend to implement a HTTP/2 test_server for RESTler. However, I am slightly confused by the documentation of restler/test_servers/README.md. I understand that the new test_server has to inherit from test_servers.test_server_base.TestServerBase and implement the parse_message function. However, I did not understand to which requests the new test_server should react and with what status code.

I am ready to work on fully implementing this version of HTTP/2 in RESTler. However, I need feedback and help (answering questions) in regards to implementing tests and a test_server. The HTTP/1.1 part of RESTler is unaffected, so merging this should be no problem.

hidingturtle commented 1 year ago

@microsoft-github-policy-service agree []

hidingturtle commented 1 year ago

@microsoft-github-policy-service agree

hidingturtle commented 1 year ago

Hi @marina-p !

The pull request is ready for a review. However, I do not know how to mark it as ready for review. Could you please clarify this for me? Thank you.

marina-p commented 1 year ago

Hello @hidingturtle,

Thanks for the ping, I'll take a look. The PR is already in the right state now (there's a "convert to draft" on the right which can be used for early drafts where you do not want feedback on the PR).

marina-p commented 1 year ago

Hello @hidingturtle,

Thank you for contributing this enhancement. For adding a test server with http2 support - can you please share a few examples for the types of things you'd like to cover? Are there some corner cases, or would a simple sanity test be sufficient?

If your main goal is to just have a sanity test for http2, how about making a configuration of an http2-based demo_server? This would be a good test to have anyway, in addition to specific cases in the test server. I have not tried this, but I saw these instructions for fastapi using hypercorn here: https://fastapi.tiangolo.com/deployment/manually/#__tabbed_2_2. If this approach works, another quick start test can be added to cover this case (an --http2 option to test_quick_start.py). If you get this working locally, I can take care of adding this test into the pipeline.

Thanks,

Marina

hidingturtle commented 1 year ago

Hi @marina-p,

A sanity test for HTTP/2 sounds good. I will look into your provided link and test out if it works (on first sight, it should work). If it works, shall I already try to edit restler-fuzzer/demo_server/demo_server/app.py and restler-fuzzer/restler/end_to_end_tests/test_quick_start.py and put it into the pull request?

My initial thoughts for doing so would be:

For app.py, I think we only need to add hypercorn.run(...) and a -http2 flag to check if it should run hypercorn or univcorn.
For test_quick_start.py, as you said, we need to add a -http2 flag and add if-conditions for the called subprocesses of the tests (depending on if -http2 is set or not, the subprocesses use -http2 or not). In addition, the -http2 flag should also be used to run app.py.
I see that test_quick_start.py uses restler-quick-start.py. The quick-start script uses its own argument parser, so I will have to add a -http2 flag to it, too.

For adding a test server with http2 support - can you please share a few examples for the types of things you'd like to cover? Are there some corner cases, or would a simple sanity test be sufficient?

I am just "scared" that while fuzzing with HTTP/2, some situations could occur on a protocol basis, where the implemented state-machine (line 346 - 374 from restler/engine/transport_layer/messaging.py of this pull request) would get stuck or do anything that it should not do. I tried to do my best against it (reading the RFC etc.) but of course some unique situations could occur, where it fails. However, such situations could also be reported as issues and then subsequently get fixed.

hidingturtle commented 1 year ago

I see that test_quick_start.py uses restler-quick-start.py. The quick-start script uses its own argument parser, so I will have to add a -http2 flag to it, too.

Done.

For app.py, I think we only need to add hypercorn.run(...) and a -http2 flag to check if it should run hypercorn or univcorn.

Actually after testing hypercorn out, it turns out that it supports HTTP/1.1 and HTTP/2. Therefore, we could just remove uvicorn and replace it with hypercorn. However for this, I would need your opinion on this @marina-p.

For test_quick_start.py, as you said, we need to add a -http2 flag and add if-conditions for the called subprocesses of the tests (depending on if -http2 is set or not, the subprocesses use -http2 or not). In addition, the -http2 flag should also be used to run app.py.

Working on this.

hidingturtle commented 1 year ago

Hello @marina-p!

I think the PR is ready now for a review. We would only need two new checks (Azure Pipelines), which you said you would take care of:

If you get this working locally, I can take care of adding this test into the pipeline.

Essentially you just need to copy the tests QuickStartTests_Linux, QuickStartTests_Windows and add a --use_http2 to the quick-start command in the Run quick start step (e.g. python ./restler/end_to_end_tests/test_quick_start.py /home/vsts/work/1/restler_test_drop --use_http2) for the same tests but now for HTTP/2 this time.

Thank you.

GKnirps commented 1 year ago

How is the status of this PR? It would really help me if this can be merged soon.

Yanqiulei commented 8 months ago

How can I resolve the following error when using Restler for testing functionality? 2024-01-24 16:39:01.091: Connection error: 'Exception: [Errno 104] Connection reset by peer'

fabpiaf commented 7 months ago

For my case this does not work, but I am not sure why.

You obviously used the h2 Plain Sockets Example Client in your code which is why I tried to create a MWE. Unfortunately this works (see mark "THIS DOES NOT WORK CORRECTLY IN YOUR PR"):

#!/usr/bin/env python3

"""
plain_sockets_client.py
~~~~~~~~~~~~~~~~~~~~~~

Just enough code to send a GET request via h2 to an HTTP/2 server and receive a response body.
This is *not* a complete production-ready HTTP/2 client!
"""

import socket
import ssl
import certifi

import h2.connection
import h2.events

SERVER_NAME = 'google.com'
SERVER_PORT = 80 #443

# generic socket and ssl configuration
socket.setdefaulttimeout(15)
ctx = ssl.create_default_context(cafile=certifi.where())
ctx.set_alpn_protocols(['h2'])

# open a socket to the server and initiate TLS/SSL
s = socket.create_connection((SERVER_NAME, SERVER_PORT))
if SERVER_PORT == 443:
    s = ctx.wrap_socket(s, server_hostname=SERVER_NAME)

c = h2.connection.H2Connection()
c.initiate_connection()
d2s=c.data_to_send()
print(f"A send {d2s}")
s.sendall(d2s)
print(f"state {c.state_machine.state}")
headers = [ 
    (':method', 'HEAD'),
    (':authority', SERVER_NAME),
    (':scheme', 'https' if SERVER_PORT==443 else "http"),
    (':path', '/api/v2/namespaces/fuzzstring/repositories/fuzzstring/tags/fuzzstring'),
    ('Accept','application/json')
]
print(f'Sendingh: {headers}')
c.send_headers(1, headers) #, end_stream=True)
c.end_stream(1)
d2s=c.data_to_send()
print(f"B send {d2s}")
s.sendall(d2s)
print(f"state {c.state_machine.state}")

body = b'' 
response_stream_ended = False
while not response_stream_ended:
    # read raw data from the socket
    data = s.recv(65536 * 1024) # THIS DOES NOT WORK CORRECTLY IN YOUR PR
    print(f"data: {data}")
    if not data:
        print("no data break")
        break

    # feed raw data into h2, and process resulting events
    events = c.receive_data(data)
    print(f"recv: {events}")
    print(f"state {c.state_machine.state}")
    for event in events:
    #    print(event)
        if isinstance(event, h2.events.DataReceived):
            print("case1")
            # update flow control so the server doesn't starve us
            c.acknowledge_received_data(event.flow_controlled_length, event.stream_id)
            # more response body data received
            body += event.data
        if isinstance(event, h2.events.StreamEnded):
            print("case2")
            # response body completed, let's exit the loop
            response_stream_ended = True
            break
        print("endcase")
    # send any pending data to the server
    d2s=c.data_to_send()
    print(f"C send {d2s}")
    s.sendall(d2s)
    print(f"state {c.state_machine.state}")

#print("Response fully received:")
#print(body.decode())

# tell the server we are closing the h2 connection
c.close_connection()
d2s=c.data_to_send()
print(f"D send {d2s}")
s.sendall(d2s)
print(f"state {c.state_machine.state}")

# close the socket
s.close()

vs the following restler setup (compiliation fails for your PR thats why I used the master for this task)

curl https://docs.docker.com/docker-hub/api/latest.yaml | sed "s@/v2/@/api/v2/@g" > dockerv2.yaml
restler-fuzzer/restler_bin/restler/Restler  compile --api_spec dockerv2.yaml
restler-fuzzer/restler_binhttp2/restler/Restler test --host google.com  --grammar_file Compile/grammar.py --dictionary_file Compile/dict.json --settings Compile/engine_settings.json --http2  --no_ssl

The actual problem: restler log:

A send b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n\x00\x00*\x04\x00\x00\x00\x00\x00\x00\x01\x00\x00\x10\x00\x00\x02\x00\x00\x00\x01\x00\x04\x00\x00\xff\xff\x00\x05\x00\x00@\x00\x00\x08\x00\x00\x00\x00\x00\x03\x00\x00\x00d\x00\x06\x00\x01\x00\x00'
state ConnectionState.IDLE
Sendingh: [(':method', 'HEAD'), (':authority', 'google.com'), (':scheme', 'http'), (':path', '/api/v2/namespaces/fuzzstring/repositories/fuzzstring/tags/fuzzstring'), ('Accept', 'application/json')]
B send b'\x00\x00P\x01\x04\x00\x00\x00\x01B\x84\xc7\x82\x1b\xffA\x87\x98\xe7\x9a\x82\xaeC\xd3\x86D\xb1`u\x99\x8e\xe2b\xa1\xd2TV2\x15\x0cKo\xbfhM\x86\xaaf,-gA\x92{\x0cT1-\xbe\xfd\xa16\x1a\xa9\x98H\xe6C\x12\xdb\xef\xda\x13a\xaa\x9bS\x8b\x1du\xd0b\r&=LtA\xea\x00\x00\x00\x00\x01\x00\x00\x00\x01'
state ConnectionState.CLIENT_OPEN
data: 
no data break

vs mwe.py

A send b'PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n\x00\x00*\x04\x00\x00\x00\x00\x00\x00\x01\x00\x00\x10\x00\x00\x02\x00\x00\x00\x01\x00\x04\x00\x00\xff\xff\x00\x05\x00\x00@\x00\x00\x08\x00\x00\x00\x00\x00\x03\x00\x00\x00d\x00\x06\x00\x01\x00\x00'
state ConnectionState.IDLE
Sendingh: [(':method', 'HEAD'), (':authority', 'google.com'), (':scheme', 'http'), (':path', '/api/v2/namespaces/fuzzstring/repositories/fuzzstring/tags/fuzzstring'), ('Accept', 'application/json')]
B send b'\x00\x00P\x01\x04\x00\x00\x00\x01B\x84\xc7\x82\x1b\xffA\x87\x98\xe7\x9a\x82\xaeC\xd3\x86D\xb1`u\x99\x8e\xe2b\xa1\xd2TV2\x15\x0cKo\xbfhM\x86\xaaf,-gA\x92{\x0cT1-\xbe\xfd\xa16\x1a\xa9\x98H\xe6C\x12\xdb\xef\xda\x13a\xaa\x9bS\x8b\x1du\xd0b\r&=LtA\xea\x00\x00\x00\x00\x01\x00\x00\x00\x01'
state ConnectionState.CLIENT_OPEN
data: b'HTTP/1.0 400 Bad Request\r\nContent-Type: text/html; charset=UTF-8\r\nReferrer-Policy: no-referrer\r\nContent-Length: 1555\r\nDate: Fri, 08 Mar 2024 11:07:39 GMT\r\n\r\n<!DOCTYPE html>\n<html lang=en>\n  <meta charset=utf-8>\n  <meta name=viewport content="initial-scale=1, minimum-scale=1, width=device-width">\n  <title>Error 400 (Bad Request)!!1</title>\n  <style>\n    *{margin:0;padding:0}html,code{font:15px/22px arial,sans-serif}html{background:#fff;color:#222;padding:15px}body{margin:7% auto 0;max-width:390px;min-height:180px;padding:30px 0 15px}* > body{background:url(//www.google.com/images/errors/robot.png) 100% 5px no-repeat;padding-right:205px}p{margin:11px 0 22px;overflow:hidden}ins{color:#777;text-decoration:none}a img{border:0}@media screen and (max-width:772px){body{background:none;margin-top:0;max-width:none;padding-right:0}}#logo{background:url(//www.google.com/images/branding/googlelogo/1x/googlelogo_color_150x54dp.png) no-repeat;margin-left:-5px}@media only screen and (min-resolution:192dpi){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat 0% 0%/100% 100%;-moz-border-image:url(//www.google.com/images/branding/googlelogo/2x/googlelo'
recv: []
state ConnectionState.CLIENT_OPEN

Both send the same data (A send and B send) but the MWE actually receives data back in sock.recv, the PR does not data:

microsoft / restler-fuzzer

HTTP/2 support #651

PR Checklist

Info on Pull Request

Validation Steps Performed

Comment