Add an HTTP/2-capable handler to the test suite

[sethmlarson]

Currently our test suite is written with HTTP/1.1 in mind. We need to be able to receive and respond with HTTP/2 in order to implement the protocol.

Requirements

• Add an HTTP/2 capable handler to the test suite (Hypercorn?)
• Have one test case that uses the new handler with HTTP/1.1

[Mr-Sunglasses]

@sethmlarson Is this Issue open for work? I'm working on this Issue 👍🏻 .

[Ousret]

@sethmlarson Are you open to switching tornado for another (mature) solution? They don't support h2 as you may already know and it may not be appropriate to make unstable arrangements for it.

[sethmlarson]

@Ousret Yes! @pquentin suggested Hypercorn+Quart as a potential solution.

[pquentin]

@Ousret Yes! @pquentin suggested Hypercorn+Quart as a potential solution.

Indeed, if Tornado supported HTTP/2, all the with_dummyserver tests could be migrated essentially for free. But it doesn't, so we need an alternative.

It seems to me there are two general directions we could take:

• Use a popular server like nginx, and either:
  1. use it as a reverse proxy to terminate HTTP/2 and speak to Tornado using HTTP/1.1
  2. use it as an actual HTTP/2 server and reimplement our TestingApp using nginx.
• Use a Python HTTP/2 server, which is simpler to implement and gives us more flexibility as we can use an API instead of launching a new process, and looking at the code and patching it is very useful: we have done this in the past with Tornado.

The only disadvantage I can see with using a Python HTTP/2 server is that we would also use h2, so we won't discover interoperability problems as we would have with a different stack. But the benefits (flexibility, maintainability, speed of conversion) sound more important to me. (I keep mentioning a "Python HTTP/2 server", but I only know of one: the Hypercorn+Quart combo nicely maintained by @pgjones.)

What features would be ideally supported by this server?

• Since we generate many different certificates, the server should start quickly while accepting arbitrary certificates on launch: https://pgjones.gitlab.io/hypercorn/how_to_guides/configuring.html#configuration-options
• Allow reimplementing the HTTPS proxy we use to test HTTPS-in-HTTPS support
• Be reliable enough to not cause flakiness in our test suite

[pquentin]

Something else to consider carefully are the "socket tests" mentioned in the issue. More generally, we have 81 instances of "HTTP/1" in our test suite, the vast majority being HTTP/1.1. Many tests use sock.send() to send HTTP/1.1 data directly. This is nice and simple, but does not work with HTTP/2. It seems to me that we should use something that abstracts over h11 and h2 to allow building those messages independently of the version of HTTP.

But a more general survey is needed to understand what exactly needs to be converted. For example, chunked testing is not needed with h11 or h2 (and was in fact removed when Cory Benfield worked on urllib3 v2 years ago, see 51d0efe (#1068)). So it's fine if we continue using hand-crafted messages there.

[pquentin]

What I wanted to highlight with my two messages above is that while this issue is going to imply a lot of tedious work at some point, it also needs careful planning and a solid plan to make sure we go in the direction we want. This is why I'm reluctant to just assign it to someone: I would hate for that person to disappear for days/weeks, spend 10+ hours on the problem and come up with a pull request that we would have to reject.

[pgjones]

Are you considering HTTP/3 as well (would be nice to get more client side support)? Hypercorn does support HTTP/3 as well, currently via an extra but it will be core (when OpenSSL supports QUIC so it is easier to install).

[pquentin]

Good question! We are considering HTTP/3 but more as a second step for now. However it does not hurt to make sure the choices we make are future proof, so it's good to know that Hypercorn supports HTTP/3 and that this support will improve.

By the way, if you are in a position to receive money to help with urllib3 HTTP/2 support, we should talk! On our Discord or over email.

[Ousret]

Were you interested, @pgjones to work on that task? So that I know if I we shall wait upon you. If you were interested, we could collaborate to bring that up, we could also, aside, use my draft experiment to further discover interesting things.

Secondly, I am wondering about the risks involved with completely rewritten tests using generated content from respective state machines. Should not we keep everything and solo expand the tests?
And about what @pquentin said on the limited confidence in testing h11, h2, and aioquic against themselves. Shouldn't we also bring a completely different aspect of the tests by confronting at least one implementation outside of Python (e.g. httpd; nginx, traefik, caddy, etc...)?

[pquentin]

I don't think @pgjones was interested, this is still up for grabs.

[sethmlarson]

@Mr-Sunglasses @Ousret To avoid ambiguity, this issue is not assigned to anyone currently so is available to anyone. Submit a draft PR getting started on the work and we can assign it.

[Ousret]

I do not think that I am going to pursue that issue.

see my latest update fyi #3030 (comment)

Maybe urllib3 should put a cron workflow that matches usual expressions like "I want to" "work" "can I work", and so on. And reply with the documentation once. The conditions are already clear enough.

[pquentin]

Another possible test server is Granian: https://github.com/emmett-framework/granian from @gi0baro. It is Rust-based and provides a Python API.

[gi0baro]

@pquentin if you want to run Granian in the same process, avoiding further spwans for workers, I suggest to subclass it and do something like this:

from granian import Granian

class GranianSameProcess(Granian):
    def _spawn_workers(self, sock, spawn_target, target_loader):
        return

    def _stop_workers(self):
        return

    def startup(self, spawn_target, target_loader):
        sock = super().startup(spawn_target, target_loader)
        spawn_target(
            1,
            target_loader,
            sock,
            self.loop,
            self.threads,
            self.pthreads,
            self.threading_mode,
            self.http,
            self.http1_buffer_size,
            self.websockets,
            self.loop_opt,
            self.log_level,
            self.log_config,
            self.ssl_ctx,
            {
                "url_path_prefix": self.url_path_prefix
            }
        )

    def _serve(self, spawn_target, target_loader):
        self.startup(spawn_target, target_loader)
        self.shutdown()

[sigmavirus24]

That relies on private method overrides which is not reasonable.

[pquentin]

That said, not spawning workers isn't a huge concern I think in the context of CI, any specific reason you brought that up?