Skip to content

Experimentation around support for HTTP/1.1, HTTP/2 and HTTP/3 outside of http.client #3030

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

Closed
wants to merge 3 commits into from
Closed

Conversation

Ousret
Copy link
Contributor

@Ousret Ousret commented May 14, 2023

Disclaimer:

  • ⚠️While usable, it is not sufficiently battle-tested. Use at your own risk as support is not provided. ⚠️

Introduction
This PR experiment the possibility for urllib3 to allow detaching ourselves from the http.client package.
The following add support to the final specification (RFCs) of HTTP/1.1, HTTP/2, and HTTP/3 over QUIC.

Are you curious about the results? Install that experimental patch by doing the following:

pip install git+https://github.com/Ousret/urllib3.git@experimental-h2n3 urllib3-ext-hface -U

The how
I felt it is time to try moving away from the httplib while remaining the default.

I know that async has a certain appeal and usefulness. But let's face it, sync libs are here to stay (numbers speak for themselves github depends, stats google dataset, etc..), so we might as well
implementing the newest protocol in the sync enclosure.

I saw the attraction around the hface library posted on the Akamai GH organization and saw an opportunity.
Immediately I decided to fork the project because:

  • Make sure that having the extra lib is recognized as the user willing to use the experimental feature
  • There were a lot of server and network stuff that are not needed for our purpose.
  • The project seems unmaintained.

So the project name is urllib3-ext-hface and is located at https://github.com/Ousret/urllib3-ext-hface (for now)

  graph TD;
      urllib3-->urllib3-ext-hface;
      urllib3-ext-hface-->h11;
      urllib3-ext-hface-->h2;
      urllib3-ext-hface-->aioquic;
      aioquic-->pylsqpack;
      aioquic-->pyopenssl;
      pyopenssl-->cryptography;
      aioquic-->certifi;
      cryptography-->cffi;
      cffi-->pycparser;
      h2-->hyperframe;
      h2-->hpack;
Loading

Backward compatibility

Initially seeing the current implementation that relies on http.client was scary. I decided to make the alternative implementation follow as closely as possible the HttpConnection behavior so that the rest of the library would remain stable.

Protocol upgrade

For the time being, this implementation act as follow:

  • Is HTTP? Initiate an HTTP/1.1 connection only.
  • Is HTTPS? Initiate an HTTP/2.0 if and only if ALPN/TLS ends on specifying h2, otherwise HTTP/1.1

And for HTTP/3?

  • Must be on HTTPS connection, regardless of HTTP/1.1 or HTTP/2, and the last response (in cursor conn) must include the Alt-Svc headers.

⚠️Only the non-draft version is supported (final spec) of h3 over QUIC.

Remaining tasks

  • Tunnelling (Proxy) raising NotImplementedError
  • Having a reliable parsing solution for Alt-Svc (in utils)
  • Extend the tests (add nox session+experimental)
  • Identify unit tests that should be skipped (e.g. not applicable to newer protocols)
  • Mypy errors
  • Unify the manual sock conn creation for DTLS and TLS
  • Migrate ciphers TLS to DTLS if supported
  • Making aioquic optional? no need in that experiment
  • Ensure downstream test passes, e.g. requests.
  • Learn things around that experimentation
  • Massively test this continuously on real-world servers (and not only the best-behaved ones)
  • Support QUIC session resumption/ticket
  • Investigate, and monitor: Memory usage, and performance across state machines (h11, h2, and h3)

Glimpses

from urllib3

from urllib3 import request
from time import time_ns

if __name__ == "__main__":

    for i in range(3):
        before_ns = time_ns()
        resp = request("GET", "https://www.cloudflare.com/img/nav/globe-lang-select-dark.svg", retries=1, timeout=1)
        print(round((time_ns() - before_ns) / 10e5), 'ms')

        print(
            'content-length:', resp.headers.get('content-length')
        )

        print(
            f"h{resp.version}"
        )

        print(
            type(resp.data), len(resp.data), f"{resp.status=} {resp.reason=}"
        )

        print("==================================================================")
93 ms
content-length: 1840
h2
<class 'bytes'> 1840 resp.status=200 resp.reason='OK'
==================================================================
82 ms
content-length: 1840
h3
<class 'bytes'> 1840 resp.status=200 resp.reason='OK'
==================================================================
29 ms
content-length: 1840
h3
<class 'bytes'> 1840 resp.status=200 resp.reason='OK'
==================================================================

@sethmlarson
Copy link
Member

sethmlarson commented May 14, 2023

Firstly, thank you for the work you've done here. It's exciting to see the next steps towards HTTP/2 and HTTP/3 support in urllib3 that we've made possible with the v2.0 work! 🚀

I have some high level thoughts on direction:

  • I would like to keep the complexity of our dependency chain as small as possible. In my mind, having h11, h2, and aioquic (along with potentially hface) as dependencies makes sense. I am unsure about having more than this, what is the motivation behind having another layer with urllib3-ext-hfrace (it seems this is a fork of hface, can these changes/our needs be upstreamed, going to cc @mila to start a conversation).
  • Is hface capable of using a background thread for communication? This is the proper way to implement HTTP/2 and /3 as the streams are much closer to a TCP stream rather than HTTP with the need to respond to ping and window update frames. Happy to have the implementation of this live in urllib3.contrib as hface seems to be sans-I/O.

Maybe this is something you and @mila can collaborate on to reach the right home for whatever changes are necessary?

Lastly, I think it'd be useful to break this work down into individual issues so everything can be tracked and worked on by others concurrently. For me, the most important part of this work is being able to reuse our high-level test suite to test HTTP/1.1, 2 and 3 with the same level of assuredness. Maybe this can be the first task and we can write up more? Perhaps this is the right feature to write up in #3000 :)

@sethmlarson
Copy link
Member

@Ousret I've changed #3000 to be a high-level tracking task for HTTP/2 support specifically. I think we should focus on supporting HTTP/2 and then try to tackle HTTP/3 since most of the fundamental changes will have to happen for both protocols and HTTP/2 gets us a long ways already.

Take a look at the list I've written there already and let me know what you think.

@Ousret
Copy link
Contributor Author

Ousret commented May 15, 2023

Hello @sethmlarson

Yes, seeing it actually work is pretty compelling.
So, you have probably guessed it, this PR is more of an exploratory surgery. And it aims at building a wide view of what we shall expect in the final design. I am looking to find at most issues as I can.

So, let's talk about your initial thoughts.

I would like to keep the complexity of our dependency chain as small as possible. In my mind, having h11, h2, and aioquic (along with potentially hface) as dependencies makes sense.

hface requires all of that and more, so it is much worse than the foremost. starting this PoC, I already knew the dependency footprint would matter (a lot).

Everything depends upon what you expect to release. Our minds did not see the same things,
I did not imagine implementing a separate BaseHttpConnection per protocol but rather a unified one.

Why unify? To ease the protocol upgrade for example. -- Not having too much code when alternatives arrive and/or newer protocol surfaces (which will undoubtedly happen).

Also, that is why I am considering not enforcing aioquic as a dependency due to the footprint and potential issue with crypto and built whl at setup.

what I drafted

  graph TD;
      HttpLib-->BaseHttpConn;
      HfaceExt-->BaseHttpConn;
      BaseHttpConn-->BaseHttpsConn;
Loading

What I think I understood from your comment?

  graph TD;
      HttpLib-->BaseHttpConn;
      BaseHttpConn-->BaseHttpsConn;
      h11-->BaseHttpConn;
      BaseHttpConn-->BaseHttpsConn;
      h2-->BaseHttpsConn;
      h3-->BaseHttpsConn;
Loading

There is a need to sit down and carefully study impact on the complexity and sustainability.

  • does urllib3 will assist the user on what protocol would be used like a traditional browser?
  • should the end user pass on the HTTP version he like/want to enforce? or pass on the HTTP version he does not want? or both?

I am unsure about having more than this, what is the motivation behind having another layer with urllib3-ext-hfrace

I wanted to ease the migration for the end user. Having to tell them to patch their code brings friction.
What I imagined is that having specifically installed urllib3-ext-hface would explicitly give us permission to switch from http.client from the next implementation.

Of course, if you want this in contrib, I am okay with this.

Is hface capable of using a background thread for communication? This is the proper way to implement HTTP/2 and /3 as the streams are much closer to a TCP stream rather than HTTP with the need to respond to ping and window update frames.

Honestly don't know. I would not do threads in urllib3, given the risks.
Maybe we should start by not doing that, but simply use the ping in the conn whenever needed or re-do handshake altogether.
asyncio would fit the need for streams, ws, ping, window update frames, early resp, and so on.

(it seems this is a fork of hface, can these changes/our needs be upstreamed,

Most likely won't happen, I removed as many things as possible to keep it lightweight. including dependencies, we don't need.
I am pretty much convinced that we can also slim down the dependency chain from aioquic.

Lastly, I think it'd be useful to break this work down into individual issues so everything can be tracked and worked on by others concurrently.

OK.

For me, the most important part of this work is being able to reuse our high-level test suite to test HTTP/1.1, 2 and 3 with the same level of assuredness.

I agree.

I've changed #3000 to be a high-level tracking task for HTTP/2 support specifically. I think we should focus on supporting HTTP/2 and then try to tackle HTTP/3 since most of the fundamental changes will have to happen for both protocols and HTTP/2 gets us a long ways already.

Thanks, I will look into it at my early.

@sethmlarson
Copy link
Member

Just to be clear of expectations, I don't think this PR is going to be reviewable as-is (so if it's an experiment that's okay!). We've tried giant PRs like this before and they always peter out, I think a better approach is to work incrementally starting with the most critical part first. Following that thinking, getting the test suite working with HTTP/2 first before starting on HTTP/2 client support at all (even using curl w/ HTTP/2 support to verify that our test suite is able to verify requests!) seems like the best option forward.

I did not imagine implementing a separate BaseHttpConnection per protocol but rather a unified one.

I think I also would like a unified HTTPSConnection class that supports both HTTP/2 and HTTP/1.1, but I think that the HTTP/1.1 support could use http.client instead of h11. Most servers these days support HTTP/2 so the investment into HTTP/1.1 should be minimal, let's focus on the value add of HTTP/2

Of course, if you want this in contrib, I am okay with this.

I think having this logic all in contrib is the way to go. Everything should either be in contrib or in h2.

Honestly don't know. I would not do threads in urllib3, given the risks.

This is fine, let's start without threads.

This is all very exciting, let me know if you need assistance. Also I think we should create a bunch of separate issues and bounties for this work in order to reward you and also encourage involvement from other contributors.

@sethmlarson
Copy link
Member

I've created this issue for our test suite: #3038

@mila
Copy link
Contributor

mila commented May 19, 2023

It's great to see that hface motivated somebody to look at h2 and h3 support in urllib3. It would be great if I were able to made hface more stable, but priorities at Akamai changed, so I'm not working on that now.

I agree with @sethmlarson that small steps one at a time would be better. If I thought that hface can be integrated to urllib3 in a week or two of full-time work, I would try to do that. But I am afraid that this i would be much more work.

@Ousret
Copy link
Contributor Author

Ousret commented May 20, 2023

Just to be clear of expectations, I don't think this PR is going to be reviewable as-is (so if it's an experiment that's okay!).

Yes, I never expected this to be merged, ever. This is a proof of concept that is public knowledge.
As open source is not under the same pressure as companies ruled by profits, I thought that producing a proof of concept for all three implementations (outside http.client) is the first thing to be done, why?

  • 🔬 discover walls (as much as possible) before drafting the final specifications (a crash test one could say)
  • and one could say (should say) that the second time will be better designed, why constrain ourselves to the one-shot design?
  • time isn't running out 😉

..and so on...

I think I also would like a unified HTTPSConnection class that supports both HTTP/2 and HTTP/1.1, but I think that the HTTP/1.1 support could use http.client instead of h11.

Just an opinion: I highly discourage this. It would produce a hazardous code base, Sans-IO mix with IO bound, that is most likely more difficult to implement and test. Using the abstraction behind hface makes a harmonized code base for all three protocols.

This is all very exciting, let me know if you need assistance. Also I think we should create a bunch of separate issues and bounties for this work in order to reward you and also encourage involvement from other contributors.

OK by me.
Finally, are you interested in eventual discovery or advancement in that PoC PR that could lead to additional thinking? And optionally testing it, reading it. Do not hesitate to say if that PR is more noise than contributive.


Hello @mila

Indeed, I found hface in a non-working state, which is why I forked the project. I am working on a highly slimmed version that solely focuses on clients. This (forked) module may or may not find itself in urllib3.contrib.

Regards,

@Ousret
Copy link
Contributor Author

Ousret commented May 21, 2023

update:

so, now, I am near the end of having solid proof of concept. I will be able to publish a full report soon and potentially archive this so that we can move on to the next steps.

on requests

= 4 failed, 575 passed, 14 skipped, 1 xfailed, 15 warnings in 72.50s (0:01:12) =

Of the 4 failures, three are subject to discussion, one is caused by this incomplete PoC.

  • (I) tests/test_requests.py::TestRequests::test_unicode_header_name

That test fail because requests push Content-Length: 1 even though that is not true. urllib3 correct behavior infer Content-Length: 2.
The origin of that is because the test passes ... data=str .. and urllib3 encodes it to utf-8 by default, witch produces two bytes.
Finally, the state machines are stricter than http.client and crash mid-flight.

  • (II) tests/test_lowlevel.py::test_redirect_rfc1808_to_non_ascii_location

Headers are decoded using utf-8 and the Location header produce something that requests cannot handle gracefully.
If not mistaken, the current state of the art is headers values are unicode. Am I wrong?

  • (III) tests/test_lowlevel.py::test_chunked_encoding_error

The initial premise push an incomplete headers frame and the state machine behind does not produce an HeaderReceived event, therefore, inducing a ProtocolError in-flight.
Does the test need to be patched? Or is it correct in that form?

on urllib3

Those tests are no longer valid (identified so far)

  • (I) test/contrib/test_pyopenssl.py::TestSocketClosing::test_retry_weird_http_version

The sub-dependencies cannot proceed without a valid HTTP version/svn.

  • test/with_dummyserver/test_connectionpool.py::TestConnectionPool::test_skip_header

The sub-dependencies cannot proceed without a valid :authority input.

@Ousret
Copy link
Contributor Author

Ousret commented May 22, 2023

🎉now, requests downstream tests pass with this experiment.

The test tests/test_requests.py::TestRequests::test_unicode_header_name actually revealed a bug in urllib3. I drafted a provisional fix, you may read the actual code at bd10e96#diff-542c95910a277028550d2c8943e8c49bbcb10f9af96e35d0a0eee99b8cfe9e8cR355
it will be easier to fully understand from it.

@Ousret
Copy link
Contributor Author

Ousret commented May 23, 2023

Some details that must be studied for impacts:

  • all stream, e.g. giving __amt will slightly differ, the returned content will be as expected, but inner mechanisms will differ

so h2 and h3 no longer stream in plain, encapsulation, and complex format makes it impossible to expect sock.read(__amt) to output len(data/body) == __amt.
no really a bother, but it must be noted.

  • say goodbye to the read_chunked() method that accesses the sock conn directly. can't be done with h2, h3, and more generally the Sans-IO state machines won't allow us such a thing.

  • header name won't likely preserve the case, as some dependencies may make them lowercase/normalize.

so, the order is preserved, but the case (original) won't be guaranteed in response.
don't really care as HTTPHeaderDict makes sure it is not an issue.

  • I would not support tunneling across QUIC (in the foreseeable future) as it greatly complicates the tests and conception.

  • HTTP/0.x in response does not crash h11, it considers the value ok. 🤔

  • across urllib3 typing, headers are often described as Mapping[str, str] but in fact allow bytes as value (and there is tests that prove it). it must be changed so that we can spot eventual errors with mypy.


Coverage
TOTAL 3203 72 98%
Not bad, as we don't do h2 and h3 tests.
Unified code has its perks.

@sethmlarson
Copy link
Member

Awesome progress! 🚀

Just an opinion: I highly discourage this. It would produce a hazardous code base, Sans-IO mix with IO bound, that is most likely more difficult to implement and test. Using the abstraction behind hface makes a harmonized code base for all three protocols.

Hmm, I would like to dig into the feasibility of this more. If we're only supporting HTTP/2 when it occurs from ALPN there's less mixing of the two protocol implementations, am I missing something?

The test tests/test_requests.py::TestRequests::test_unicode_header_name actually revealed a bug in urllib3.

Could you open a separate issue with this information (and separately any other behaviors you want to discuss whether they are bugs/not) so they can be triaged?

so h2 and h3 no longer stream in plain, encapsulation, and complex format makes it impossible > to expect sock.read(__amt) to output len(data/body) == __amt.
no really a bother, but it must be noted.

We created a memory stream buffer to achieve this property for decompression, I think we can use a similar mechanism for HTTP/2? I would like to preserve this feature of len(HTTPResponse.read(N)) == N unless we're at the end of the stream.

say goodbye to the read_chunked() method that accesses the sock conn directly. can't be done > with h2, h3, and more generally the Sans-IO state machines won't allow us such a thing.

I'm curious about this one, is this due to the above constraint or something else? We shouldn't need access to the socket directly, only calls to .read()?

so, the order is preserved, but the case (original) won't be guaranteed in response.
don't really care as HTTPHeaderDict makes sure it is not an issue.

Yeah order is the only thing that matters here, HTTP/2 forces lowercase so this makes sense.

I would not support tunneling across QUIC (in the foreseeable future) as it greatly complicates the tests and conception.

No worries about tunneling over HTTP/2 or HTTP/3, I think we should keep the proxy code simple and focus on HTTP/2 for origins first so tunnel CONNECT can use HTTP/1.x as is done today.

@Ousret
Copy link
Contributor Author

Ousret commented May 28, 2023

Hmm, I would like to dig into the feasibility of this more. If we're only supporting HTTP/2 when it occurs from ALPN there's less mixing of the two protocol implementations, am I missing something?

Yes, there is some point missed. Let me propose some heavy pros:

  • Simplicity increased for producing a test generator instead of hardcoding HTTP response through socket that comply with h11, h2, and maybe h3. Letting http.client will introduce some pain in making it compatible.
  • h11 better complies with RFC than http.client
  • You can control h11, contribute, and pin version range. You may not be able to vouch against undesirable changes or bugs in cpython. Simply put consistent behavior across interpreters.
  • This PR proves (?so I think?) that it is clearly doable.
  • Doing h2 and http.client is for sure more difficult than doing h11 and h2, I will try later to show concrete evidence of my process of thoughts.
  • Relieve cpython of the burden?

Could you open a separate issue with this information (and separately any other behaviors you want to discuss whether they are bugs/not) so they can be triaged?

Already done. Will do it anytime I think this can lead to constructive exchanges.

We created a memory stream buffer to achieve this property for decompression, I think we can use a similar mechanism for HTTP/2? I would like to preserve this feature of len(HTTPResponse.read(N)) == N unless we're at the end of the stream.

Yes! Saw that, actually, yes this is a non-issue. Just a note.

I'm curious about this one, is this due to the above constraint or something else? We shouldn't need access to the socket directly, only calls to .read()?

This is also a non-issue, the original author did know in advance that this would die eventually. And in case direct access isn't available, fallback on .read().
So there is an interesting subject of discussion, we should deprecate it and schedule its complete removal.
It could have landed due to limitations in http.client, so I imagine.

No worries about tunneling over HTTP/2 or HTTP/3, I think we should keep the proxy code simple and focus on HTTP/2 for origins first so tunnel CONNECT can use HTTP/1.x as is done today.

So we are on the same page,
This PR now ships with complete tunneling capabilities as discussed. The tunnel always gets done through HTTP/1.1 and it disables completely HTTP/3.

@Ousret
Copy link
Contributor Author

Ousret commented May 28, 2023

update

This PR is now a valid proof of concept.

Tests

All tests pass (target: h2n3)

========= 1743 passed, 570 skipped, 158 warnings in 762.50s (0:12:42) ==========

I only had to disable 10!

SKIPPED [10] test/conftest.py:357: Test require httplib backend

  • test_preserve_transfer_encoding_header ◀️ transfer-encoding not supported (not chunked)
  • test_read_chunked_short_circuit, test_read_chunked_on_closed_response ◀️ read_chunked access directly fp (aka. sock makefile)
  • test_retry_weird_http_version ◀️ HTTP status line is broken but accepted by h11 (HTTP/0.5 is acceptable)
  • test_httplib_headers_case_insensitive, test_headers_are_sent_with_the_original_case ◀️ As discussed, headers no longer will match the case, only order.
  • test_header_without_name, test_header_without_name_or_value, test_header_without_colon_or_value ◀️ httplib is non-strict and urllib3 skipped the header afterward. h11, h2, and h3 are not so much nice about this nonsense.

Coverage

The coverage reached 100% again. While it does not guarantee stability, it is a firm good starting point.

Fake server

With #3038 uncertain outcoming, I decided to take another road, for this experiment.
I put in place a Traefik server linked to a Httpbin (rewritten in Golang).

So, the server is completely written in Golang, it could permit us to detect anomalies that would not be found if we constrained ourselves in the same dependencies in and out as stated by @pquentin rightfully.

I pushed a modest number of tests to verify basic functionalities and reach an acceptable coverage.

nb. There is two Traefik instance, it is done on purpose. Only one of them (non-alt) supports HTTP/3 over QUIC.

I could write a dedicated Github Action if that is desirable.

Python 3.12

Failures are unrelated. I despatched a specific issue to track progress on it.

GHA

Two things:

concurrency:
  group: ci-${{ github.ref_name }}
  cancel-in-progress: true

It's better to enable this, so we can pollute less. We let CIs run when there's no reason to.

continue-on-error: ${{ matrix.experimental }}

Does not work as intended. Tried to enable debug run on the fork repo without success.
${{ matrix.experimental }} worth True but does not take effect (3.12)..

So what's next?

The last thing to produce would be :

  • Investigate, and monitor: Memory usage, and performance across state machines (h11, h2, and h3)

@Ousret
Copy link
Contributor Author

Ousret commented May 29, 2023

update

Performance

A brief glimpse at early performance from that experiment without any fine-tuning.

10 thousand requests to the Golang server Traefik + GoHttpbin.
1 out of 4 requests are POST with 1024 bytes body, otherwise, simple GET.
This is a very fictive scenario, the target is not the network condition but rather the overage Python instructions/complexity.

The above scenario ran multiple times of course.

in average

  • HTTP
    • http.client 7.49 ms
    • hface 7.94 ms (-6% slower)
  • HTTPS
    • http.client 7.49 ms (negligible diff)
    • hface 10ms w/ h2 (-25% slower)
    • hface 14ms w/ h3 (-46% slower) (0-RTT disabled, crypto outside of cpython native capabilities)

Python 3.11, Docker Alpine

Note that the Golang server always responds in less than 1 ms.

No timeout, bad response, or request, and no crash is to be reported.
I am confident that those numbers can be tightened to -8 to -9% slower.
So in conjunction with network consideration (real world), h3 should perform better than h2 by a bit.

Memory usage

  • httplib
    • HTTP: 15 MiB
    • HTTPS: 16 MiB
  • hface
    • HTTP: 29 MiB (+48% usage)
    • HTTPS: 30 MiB

nb. here hface ships with all three protocols' support, and does not import lazily. For more details, look at the tree posted in the first post of this thread.
Most importantly, no memory leaks occurred.

Python 3.11, Docker Alpine

OpenSSL, aioquic

So, OpenSSL does not ship with what is required for QUIC. That's what it is.
aioquic compose to be able to offer its support today.

My rough estimation on when Python may ship with QUIC support TLS natively is one and a half years to two full years starting now.

  • OpenSSL needs roughly a year to fully implement it.
  • Depending on the timing, cpython might get it bundled/compiled against for the future minor (in the given year).

To be fair, aioquic does a pretty good job at offering support for QUIC and H3. They can drop external SSL once its support is available, thus making the dependencies optional for the latest interpreter version.

What can be done today at aioquic to better the experience:

  • Drop certifi as a mandatory dependency.
  • Migrate the verify hostname function from SSL to what urllib3 already does.
  • Call load default cert from SSL context like done in urllib3 create default context.

Finally

I expected those numbers, it was easily foreseeable.

I don't think that I will do the massive real-world test for a while.
All of this is in a hopeful bookend that will be useful.

I purposely set a weird version in the fork, in case...you'd get unsolicited support.

@Ousret Ousret changed the title Add experimental support for HTTP/1.1, HTTP/2 and HTTP/3 Experimentation around support for HTTP/1.1, HTTP/2 and HTTP/3 outside of http.client Jun 1, 2023
@Ousret
Copy link
Contributor Author

Ousret commented Jun 2, 2023

update

statistics from requests, fetching a simple resource from Cloudflare static cdn with TLS 1.3. Using sufficient samples for stabilization without getting banned from the server. By a burst of 256 requests, repeated multiples times.

here, Handshake Included always means the initial TCP/TLS connection. (for h3 the two firsts -- h2/h11,h3)

Stats with http.client
Handshake Included: Average 38.17 ms
Handshake Excluded: Average 37.91 ms
-----------------
Stats with h11
Handshake Included: Average 38.35 ms
Handshake Excluded: Average 38.06 ms
------------------
Stats with h2
Handshake Included: Average 40.76 ms
Handshake Excluded: Average 40.33 ms
------------------
Stats with h3
Handshake Included: Average 38.61 ms
Handshake Excluded: Average 37.8 ms

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub insists that I review this given the changes in CI, let's see if a comment is enough to make it happy.

@Ousret
Copy link
Contributor Author

Ousret commented Jun 9, 2023

Now basically covered everything.
I could negociate http/2 using tls-in-tls 🎉

wrote tests that prove it using dummy proxy (local thread) + traefik + httpbin containers.

@Ousret
Copy link
Contributor Author

Ousret commented Jun 16, 2023

Final notes on my experiment

I consider that my experiment in #3030 reached its final form.
Now is the time to share with the community the conclusion that arose from working on it.

The main objectives of this experiment were to identify as broadly as possible concerns, issues, or design complications in an attempt
to :

  • Stop using http.client and use instead hyper/h11.
  • Integrate hyper/h2 and aiortc/aioquic to support HTTP/2 and HTTP/3.

All of that while remaining in a synchronous context.

👋 Farewell http.client?

Yes..!
Why?

  • ✔️ Avoid the same request to succeed on HTTP/1 and unexpectly fail with HTTP/2.
  • ✔️ Enough sarcastic comments have been written about http.client, let's move on progressively.
  • ✔️ Will see bellow, but will permit a single base code for every (and future) protocol.
  • ✔️ Consistent behavior across interpreter version! 🎉

1️⃣ Unify all protocols

Introduction

We just cannot integrate directly the three underlying libraries as-is.
A bit of history,

Akamai, through @mila, prepended a hackable interface client and server that follow the Sans-IO guidelines.
The idea itself is compelling as it implies "We talk HTTP, don't care about the protocol nor the IO!".

Unfortunately, the project is unmaintained, not working, and contains too many things that we do not need.

Fork and Divergence

You have properly guessed it alright from the introduction. I forked the project located at https://github.com/akamai/hface to
https://github.com/Ousret/urllib3-ext-hface

The complete changeset is available at https://github.com/Ousret/urllib3-ext-hface/blob/main/CHANGELOG.rst
But most importantly:

  • Removed anything server-side-related.
  • Removed anything not-Sans-IO-related.
  • Removed every dependency except underlying HTTP Sans-IO implementations.
  • There was a kind of global Protocol Registry that did not conform to our standards (Thread-safety).
  • Refactor interfaces for simplicity.
  • Fix protocol implementations.
  • Rewrite/unify ProtocolFactory.

In a nutshell

Sans-IO Common Interface

HTTP1Protocol, HTTP2Protocol, HTTP3Protocol are abstract types.
HTTPProtocolFactory is the factory that can produce a ready-to-use instance of a given abstract type.

from urllib3_ext_hface import HTTPProtocolFactory, HTTP1Protocol, HTTP2Protocol, HTTP3Protocol

protocol = HTTPProtocolFactory.new(HTTP2Protocol)

# ...
protocol.bytes_to_send()  # Out to your network I/O implementation
# ...
protocol.bytes_received()  # Whatever comes from your network I/O implementation
# ...
protocol.next_event()  # Output the next event in order that the state-machine generated

In my own implementation of that work, you may find three major concepts applied, find them at:

shows how to handle the IN/OUT in a synchronous context

shows how to initialize protocol instance

Events

Each underlying libraries have its own set of yield events, so the module has to propose another common set (e.g. translated internally)

Event Type Notes
Event It is the base class for all events.
ConnectionTerminated Something final happened to our HTTP connection.
DataReceived Got partially or complete body.
GoawayReceived The server is asking us to stop sending further requests after the last one we made.
HandshakeCompleted The protocol state machine reached a point where it can start sending requests. Aka. IDLE mode.
HeadersReceived Self-explanatory.
StreamResetReceived Special case where the server is aware that the current request/stream is broken on his side (at least) and asks us to retry.

In those, HeadersReceived and DataReceived contain a special boolean end_stream to know whenever it is the last event to be expected. It implies True if StreamResetReceived or ConnectionTerminated is received.

Port of the module

Maintainers expressed an interest in having this module ported in urllib3.contrib. It should be pretty straightforward to do so. Do not forget to include the tests!

⚠️ I would not do that, at least not until a year or so, my opinion remains the same. Meaning keeping a separate package.

Why keeping a separate package is a good idea for now?

  • ✔️ Having an external marker that does not imply patching downstream libraries.
  • ✔️ Monitoring usage and interest from users instead of forcing that change onto them (public dataset stats PyPI).

*forcing that change onto them: even by enabling HTTP/2 through the optional hyper/h2 there are about 4M downloads per month, they might be surprised. Even worst for hyper/h11 with nearly 30M downloads per month.

So if the separate package is still not an option:

  • Adding an argument and propagating it through HttpConn, PoolConn, ProxyManager, etc...

Downside to this is how limited this would impact people, as requests won't do anything as per the indefinite feature freeze.

  • Setting environment variable.

Downside to this also limits its range, not many people are comfortable with it.

🏗️ Conception

Let's see together the possible outcome of integrating said module. I do not expose the possibility of hybrid http.client with h2 and h3 as discussed in (why drop http.client?).

  1. Create another HttpConnection and HTTPSConnection
  • 💥 Break the DRY principle, most things in current HttpConnection and HTTPSConnection still apply!
  • 💥 The HttpConnection own the sock property and is the first aware of the ALPN negotiated, has to do observable between the pool and re-create another HttpConnection which is bad.

This is by far the implementation I most dislike because it causes havoc and complicates the code paths.

  1. Create a unified parent BaseConnection (chosen option in experiment)
  • 💥 Can host some anti-pattern things (like handling QUIC/TLS outside of child HTTPSConnection)

We should not care about it (the anti-pattern) as QUIC will in the foreseeable future be mainstream in Python.
By anti-pattern, ~I mean that horrible thing https://github.com/Ousret/urllib3/blob/experimental-h2n3/src/urllib3/backend/hface.py#L107 ~ (that can be replaced by a mere scheme class var property as the pool did, updated: done) remains that the base class (in practice) should not be aware of tls things.

  1. Rewrite part of HttpConnection and PoolManager to delegate the socket/tls creation to the PoolManager and instantiate appropriate HttpConnection and port common code (socket wrap, hostname verify, etc..) to a proper (instantiable) BaseHttpsConnection.

That is going to take so much time and is going to be more uncertain than the other two.

Then

I created and propagated two arguments:

  • disabled_svn

A set of HttpVersion that effectively disable one or more protocols. It is taken into account in PoolKey.

  • preemptive_quic_cache

It's a MutableMapping that is optional, on the user resort, to save and load the Alt-Svc header. Not taken into PoolKey for obvious reasons but can be propagated.

Finally

For security considerations I purposely disabled hostname swapping when interpreting the Alt-Svc.

✅ Tests

Today urllib3 ships with more than 1.7k test cases, a non-negligible part concerning the HTTP 1.1 protocol at the socket level.
The good news is that I successfully ran the complete (nearly; -10 tests see below) test suite using hyper/h11, the bad news is that we don't test HTTP/2 and HTTP/3 support reliably, yet!

So a discussion was started at #3038
I took a large amount of time to assess the possibilities, pro/con-wise.

My conclusion is as follows:

  • We should not rewrite the tests, we should extend them.

Why did you ask? It is impossible to guarantee backward compatibility with the previous hard-coded plain socket tests.
Rest assured, I tried starting that huge task, but find myself stopping pretty quickly in flight..!

Why you asked--again? I increasingly felt like I am writing whole over the test suite from hyper/h11, hyper/h2, and.. aioquic. I felt a bit dumb.
So many tests unit may be dropped in the next major, delegating it to hyper/h11, hyper/h2, and.. aioquic.

  • Focus on the high-level aspects of the tests.

We should keep the existing test suite to ensure no extravagant breakage is inserted in flight,
Instead, we can start testing all possible requests outcomes/paths.

  • Stop testing Python implementations against Python implementations!

I drafted an interesting test scenario (so I think) that set up a Traefik instance linked to an httpbin (all written in Golang) that permitted me to find many interesting cases. Which would not have been the case otherwise.

  • yeah, it just works, everything passes in the end.

👀 Observations

We assess impacts based on the test suite.
I had to find out what tests are unsuitable for that change.

⚡ Breaking changes

The most notable item in the menu is that underlying libraries (meaning h11, h2, and aioquic) are stricter than what the general mass is used to.

  • test_preserve_transfer_encoding_header ◀️ transfer-encoding not supported (not chunked)
  • test_read_chunked_short_circuit, test_read_chunked_on_closed_response ◀️ read_chunked access directly fp (aka. sock makefile)
  • test_retry_weird_http_version ◀️ HTTP status line is broken but accepted by h11 (HTTP/0.5 is acceptable)
  • test_httplib_headers_case_insensitive, test_headers_are_sent_with_the_original_case ◀️ As discussed, headers no longer will match the case, only order.
  • test_header_without_name, test_header_without_name_or_value, test_header_without_colon_or_value ◀️ httplib is non-strict and urllib3 skipped the header afterward. h11, h2, and aioquic are not so much nice about this nonsense.

So as I stated in #3045 urllib3 must either document that you will have to catch more often protocol error or be stricter itself.

As discussed with the maintainer, the header name case does not matter only the order does.

📝 Notable

In this section, we can invoke that:

  • We can no longer skip the Host header.
  • Some part of the code (internally) like .read_chunked() in Response is made irrelevant.
  • Some headers are allowed in HTTP 1.1 but not with HTTP 2 or/and 3.

⏩ Performance

As you can see on preliminary results in #3030 all three implementations perform nearly as fast as http.client.
The sole exception is that HTTP/3 is increasingly faster (than others) depending on your WAN capabilities.

⛓️ Dependency chain

When this adventure started, we had the following chain:

graph TD;
    urllib3-->urllib3-ext-hface;
    urllib3-ext-hface-->h11;
    urllib3-ext-hface-->h2;
    urllib3-ext-hface-->aioquic;
    aioquic-->pylsqpack;
    aioquic-->pyopenssl;
    pyopenssl-->cryptography;
    aioquic-->certifi;
    h2-->hyperframe;
    h2-->hpack;
Loading

In that chain, aioquic was the heaviest weight in the tree.

  • 💥 Require pyOpenSSL which is planned to be unsupported by urllib3 itself soon enough! Awkward situation.
  • 💥 Contain non-native code. Specific wheels to be available or build required.
  • 💥 TLS capabilities not aligned with urllib3. e.g. certifi is used!
  • 💥 Well.. the elephant in the room, is too many dependencies!

Fortunately!

  • ✔️ I proposed a patch that effectively drops certifi.
  • ✔️ Then made a patch to align certificate verification with urllib3. Remove deprecated code and such...
  • ✔️ Implemented a fellow contributor code to make the wheel abi3.
  • ✔️ Found a way to drop pyOpenSSL by vendoring a tiny part of it (certificate verification) mostly based on cryptography.
  • ✔️ Ported pylsqpack into the parent project and updated the underlying c library. Also made it abi3 compatible.

The original author seems to take a break, let's wait for its return (or not). I made a fork that complies with expectations.

To finally render:

graph TD;
    urllib3-->urllib3-ext-hface;
    urllib3-ext-hface-->h11;
    urllib3-ext-hface-->h2;
    urllib3-ext-hface-->qh3;
    qh3-->cryptography;
    h2-->hyperframe;
    h2-->hpack;
Loading

🎉 More reasonable isn't it?

There is more. I patched the urllib3-ext-hface to only include qh3 (fork of aioquic) for a specific platform (where there is an abi3 whl). Others must install and build themselves. This effectively covers more than 90% of users (general population).

edit: made the whl from 1 MiB+ to ~200KiB and dropped the need for statically linked OpenSSL developments res.

🗓️ aioquic future

So, OpenSSL does not ship with what is required for QUIC. That's what it is.
aioquic compose to be able to offer its support today.

My rough estimation on when Python may ship with QUIC support TLS natively is one and a half years to two full years starting now.

  • OpenSSL needs roughly a year to fully implement it.
  • Depending on the timing, cpython might get it bundled/compiled against for the future minor (in the given year).

To be fair, aioquic does a pretty good job at offering support for QUIC and H3. They can drop external SSL and QUIC once its support is available, thus making the dependencies optional (cryptography) for the latest interpreter version.

For now, using my proposed changes, it is acceptable as-is.

✋ Conclusion

So this reach the end of my journey assessing, and experimenting toward said goals.
Any willing contributor can inspect and use this "journal", and the code I produced. It would be appreciable to get mentioned (by name) where it is used so that I could see the progress.

I am willing to help and collaborate with the rest. Get in touch directly.
I won't continue pursuing this all by myself. It was pretty exhausting, but worth the time spent. Knowledge acquired felt good.

The fork works correctly for the usage I do personally and at work. I can maintain the fork pending official support within urllib3.

Good luck to anyone tackling the rest, partially or in full.

➕ Bonus note

I drafted a solution that takes more advantage of the multiplexed aspect of HTTP/2 and 3. But that requires that to pass first.
Bear with me,

With streams, we can send as many requests as we want (provided we do not run out of stream ids) and then wait for responses,
marked with the specific stream id.

After getting a HandshakeCompleted event, we set the socket to non-blocking (sendall to send).

What I did, is make the HttpLibProxyResponse completely lazy. Meaning it does not load anything from the response until accessed (even headers, and status code).

The function/method inside HttpConn called __exhange_until is patched to support multi-stream responses and dispatch the content to a bound list of lazy responses inside the HttpConn.

Immediately I get a robust performance increase. But.. tests (in a small portion started to fail consistently) It can be fixed, but unfortunately too tired to fix them.

So this is more of an idea shared. Async does not have to be the answer.
Also, there is an HTTPie port of that experiment, for whoever wants to try without coding anything! See httpie/cli#1512

That's all, folks!

Detach ourselves from the http.client package. This requires a separate package to be installed, namely `urllib3-ext-hface` for the Sans-IO interfaces.
@sethmlarson
Copy link
Member

Thank you for the thorough analysis @Ousret, we'll have to spend some time picking apart your comment and creating individual issues that we can assign bounties to and others can start working on.

@sethmlarson
Copy link
Member

Closed due to jawah#46 (comment)

@Ousret Ousret deleted the experimental-h2n3 branch January 17, 2024 06:12
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.

4 participants