python-hyper / h11

A pure-Python, bring-your-own-I/O implementation of HTTP/1.1
https://h11.readthedocs.io/
MIT License
490 stars 62 forks source link

Connection close on Response #156

Open Kludex opened 1 year ago

Kludex commented 1 year ago

Hi there :wave:

I was trying to improve the test coverage on Uvicorn, and I noticed something interesting...

The server can't send connection: close headers on Response. If you set it, h11 will convert it to Connection: close (capitalized version).

This code:

            print(headers)
            event = h11.Response(
                status_code=status_code, headers=headers, reason=reason
            )
            print(event)
            output = self.conn.send(event)
            print(output)

Outputs the following:

[[b'content-type', b'text/plain; charset=utf-8'], [b'content-length', b'12'], (b'connection', b'close')]
Response(status_code=200, headers=<Headers([(b'content-type', b'text/plain; charset=utf-8'), (b'content-length', b'12'), (b'connection', b'close')])>, http_version=b'1.1', reason=b'OK')
b'HTTP/1.1 200 OK\r\ncontent-type: text/plain; charset=utf-8\r\ncontent-length: 12\r\nConnection: close\r\n\r\n'

So... I'm here looking for instructions, more than "asking to change". Is it fine to be like this? I want this info, so I can write proper tests on uvicorn, as the other HTTP implementation uses httptools, and I can send connection: close there.

njsmith commented 1 year ago

It does sound like an oversight in h11 since we try to preserve header case, but it shouldn't have much practical effect. HTTP header names are case-insensitive, so Connection and connection mean the same thing.

On Fri, Nov 25, 2022 at 1:31 AM Marcelo Trylesinski < @.***> wrote:

Hi there 👋

I was trying to improve the test coverage on Uvicorn, and I noticed something interesting...

The server can't send connection: close headers on Response. If you set it, h11 will convert it to Connection: close (capitalize version).

This code:

        print(headers)

        event = h11.Response(

            status_code=status_code, headers=headers, reason=reason

        )

        print(event)

        output = self.conn.send(event)

        print(output)

Outputs the following:

[[b'content-type', b'text/plain; charset=utf-8'], [b'content-length', b'12'], (b'connection', b'close')]

Response(status_code=200, headers=<Headers([(b'content-type', b'text/plain; charset=utf-8'), (b'content-length', b'12'), (b'connection', b'close')])>, http_version=b'1.1', reason=b'OK')

b'HTTP/1.1 200 OK\r\ncontent-type: text/plain; charset=utf-8\r\ncontent-length: 12\r\nConnection: close\r\n\r\n'

So... I'm here looking for instructions, more than "asking to change". Is it fine to be like this? I want this info, so I can write proper tests on uvicorn, as the other HTTP implementation uses httptools, and I can send connection: close there.

— Reply to this email directly, view it on GitHub https://github.com/python-hyper/h11/issues/156, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEU42EI5C4YW5IPLBJRVY3WKCBPLANCNFSM6AAAAAASLEAINA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Nathaniel J. Smith -- https://vorpus.org http://vorpus.org

Kludex commented 1 year ago

So... Should I create a PR to change this behavior, or is it fine?

Really not a big deal for me... I just noticed it when working on this: https://github.com/encode/uvicorn/pull/1776/

njsmith commented 1 year ago

Sure, it wouldn't hurt to fix -- like I said, h11 does try to preserve case.

On Fri, Nov 25, 2022 at 1:55 AM Marcelo Trylesinski @.***> wrote:

So... Should I create a PR to change this behavior, or is it fine?

Really not a big deal for me... I just noticed it when working on this: encode/uvicorn#1776

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

-- Nathaniel J. Smith -- https://vorpus.org