Python: Fix header conversion to byte-pair on scope building

[morgan9e]

headers: These are byte strings of the exact byte sequences sent by the client/to be sent by the server. While modern HTTP standards say that headers should be ASCII, older ones did not and allowed a wider range of characters. Frameworks/applications should decode headers as they deem appropriate. (https://asgi.readthedocs.io/en/latest/specs/www.html#wsgi-compatibility)

Workerd uses Pyodide JSProxy object when passing fetch(request, env) to Python, but JSProxy objects translates into string, not byte-string.

In order to match ASGI specification, it needs to be translated into byte-pair when building scopes.

[github-actions[bot]]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[morgan9e]

I have read the CLA Document and I hereby sign the CLA

[leobuskin]

That's not the only trouble w/ ASGI and binary strings. We also need to decode resulting headers in asgi.py, if I understand correctly:

async def send(got):
        ...
        if got['type'] == 'http.response.start':
            status = got['status']
            headers = [(k.decode(), v.decode()) for k, v in got['headers']]

[spikerwork]

That's not the only trouble w/ ASGI and binary strings. We also need to decode resulting headers in asgi.py, if I understand correctly:

async def send(got):
        ...
        if got['type'] == 'http.response.start':
            status = got['status']
            headers = [(k.decode(), v.decode()) for k, v in got['headers']]

Confirmed! This patch fixed the problem with b'' I've seen before

[morgan9e]

That's not the only trouble w/ ASGI and binary strings. We also need to decode resulting headers in asgi.py, if I understand correctly:

async def send(got):
        ...
        if got['type'] == 'http.response.start':
            status = got['status']
            headers = [(k.decode(), v.decode()) for k, v in got['headers']]

Yes, right. I fixed it. Thanks!

[hoodmane]

Since #2096 merged, we're actually running Python tests on workerd so it should be possible to add a test now.

[morgan9e]

@hoodmane: Since #2096 merged, we're actually running Python tests on workerd so it should be possible to add a test now.

Should I add test for ASGI only or if its working with Starlette? I think it might be better if we make asgi.py independant to FastAPI

[hoodmane]

Agreed we should make it independent. Currently the only thing in there that is dependent on fastapi is Env. Perhaps we can just delete that for now and we can figure out the appropriate place to add it later.

[dom96]

Hey @morgan9e, would love to get this merged in, are you still planning to create a test for this?

[morgan9e]

Hey @morgan9e, would love to get this merged in, are you still planning to create a test for this?

Yep, thanks I'll do it ASAP.

[morgan9e]

Hey @morgan9e, would love to get this merged in, are you still planning to create a test for this?

Yep, thanks I'll do it ASAP.

I added the test, I wrote sample ASGI Application rather than using FastAPI because I thought it was better to seperate it from FastAPI.

[morgan9e]

Sorry, I accidently merged and removed upstream branch, I reopened it. #3142