feat: supports setting multiple hosts

[vvanglro]

Summary

On the realization of this discussion
#1529

Checklist

• I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
• I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
• I've updated the documentation accordingly.

[vvanglro]

Not sure why that test case is failing, doesn't seem to have anything to do with this PR. The local tests have always passed.

Both PRs failed. #2444 #2431

The last few commits haven't involved changes to failed use cases either, so why is that?🤔

[vvanglro]

Well, the investigation revealed that httptools changed its behavior in 0.6.3.
MagicStack/httptools@560bd9e
#2375

pip install --upgrade httptools==0.6.3

import httptools

REQUEST_AFTER_CONNECTION_CLOSE = b"\r\n".join(
    [
        b"GET / HTTP/1.1",
        b"Host: example.org",
        b"Connection: close",
        b"",
        b"",
        b"GET / HTTP/1.1",
        b"Host: example.org",
        b"",
        b"",
    ]
)

parser = httptools.HttpRequestParser("a")
# Adding this line works fine.
# parser.set_dangerous_leniencies(lenient_data_after_close=True)
print(parser.feed_data(REQUEST_AFTER_CONNECTION_CLOSE))

output:

print(parser.feed_data(REQUEST_AFTER_CONNECTION_CLOSE))
  File "httptools/parser/parser.pyx", line 259, in httptools.parser.parser.HttpParser.feed_data
httptools.parser.errors.HttpParserError: Data after `Connection: close`

[Kludex]

Is it a problem on uvicorn or on httptools?

[vvanglro]

Is it a problem on uvicorn or on httptools?

REQUEST_AFTER_CONNECTION_CLOSE = b"\r\n".join(
    [
        b"GET / HTTP/1.1",
        b"Host: example.org",
        b"Connection: close",
        b"",
        b"",
        b"GET / HTTP/1.1",
        b"Host: example.org",
        b"",
        b"",
    ]
)

If such a message exists in a real environment, it's a problem with uvicorn.

[Kludex]

I don't understand. Is it because of a security issue that was fixed?

[vvanglro]

MagicStack/httptools@560bd9e

Here are the details of this commit:

  Expose leniency flags via the new `set_dangerous_leniencies` parser
  method if somebody needs to opt into the old vulnerable behavior.

It looks like the error was passed to the caller, maybe it hadn't been before.

I'm not a cython expert.

[vvanglro]

Oh, I see, now using a more standardized and strict parsing method. That's why the above message, when processed, reported an error. After the connection is closed, no messages should be sent after it.

[Kludex]

Oh, I see, now using a more standardized and strict parsing method. That's why the above message, when processed, reported an error. After the connection is closed, no messages should be sent after it.

So uvicorn should not send anything to the parser after, right?

[vvanglro]

So uvicorn should not send anything to the parser after, right?

yes.

class HttpToolsProtocol(asyncio.Protocol):
    def __init__(
        self,
        config: Config,
        server_state: ServerState,
        app_state: dict[str, Any],
        _loop: asyncio.AbstractEventLoop | None = None,
    ) -> None:
        if not config.loaded:
            config.load()

        self.config = config
        self.app = config.loaded_app
        self.loop = _loop or asyncio.get_event_loop()
        self.logger = logging.getLogger("uvicorn.error")
        self.access_logger = logging.getLogger("uvicorn.access")
        self.access_log = self.access_logger.hasHandlers()
        self.parser = httptools.HttpRequestParser(self)
++      self.parser.set_dangerous_leniencies(lenient_data_after_close=True)

It seems we can only add one line of code to handle this.
After adding this line of code, the response before the connection is closed can be processed normally.
Messages after the connection is closed will not be parsed, That is, the headers value in the figure, only resolved to the pre-close.

[vvanglro]

#2238

This is wrong behavior, right?

that's right?

[Kludex]

I don't know, I'd have to read some resources before answering that.

[vvanglro]

We'll take the next step when we figure it out.

[Kludex]

@vvanglro What do you feel we need to do to support both this PR and this issue #571?

Sorry the delay here.

[vvanglro]

@vvanglro What do you feel we need to do to support both this PR and this issue #571?

This problem requires multiple ports, and this PR was originally designed with multiple hosts, so if we want to solve these two problems, then I suggest adding -bind to gradually replace the current host portmode.

Similar to https://github.com/pgjones/hypercorn/blob/main/src/hypercorn/__main__.py#L45, But our bind only supports host:port, and uds and fd are still separate. Avoid the burden of complicated analysis.

[Kludex]

Let's go with #2818. Thanks for this and sorry the delay.