-
Notifications
You must be signed in to change notification settings - Fork 306
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
HTTP/2 support #651
base: main
Are you sure you want to change the base?
HTTP/2 support #651
Conversation
@microsoft-github-policy-service agree [] |
@microsoft-github-policy-service agree |
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. |
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). |
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 Thanks, Marina |
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 My initial thoughts for doing so would be:
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 |
9ab6bd4
to
0270095
Compare
Done.
Actually after testing hypercorn out, it turns out that it supports HTTP/1.1 and HTTP/2. Therefore, we could just remove
Working on this. |
Hello @marina-p! I think the PR is ready now for a review.
Essentially you just need to copy the tests Thank you. |
How is the status of this PR? It would really help me if this can be merged soon. |
How can I resolve the following error when using Restler for testing functionality? |
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:
vs
Both send the same data ( |
PR Checklist
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 withHttpResponse
. However, it is also possible to create anotherHttp20Response
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 theSETTINGS
frame, Open5GS crashes once RESTler sends thePUT
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 fromtest_servers.test_server_base.TestServerBase
and implement theparse_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.