Nginx Unit not conforming to ASGI spec on websocket connections

[FixFlare]

I'm using the "Litestar" package to host a python API using Nginx Unit.

During development I used Litestar's recommended dev server "uvicorn" and did not experience any issues until I deployed to production running Nginx Unit and then got the error "405: Method Not Allowed" when I tried to make my websocket connections.

Tracing the issue, I found that the method being sent on the websocket connection was set to "GET" and the way Litestar handles determining if it's a websocket connection is based on the method being "None" so the fact that Nginx Unit is sending "GET" for the request is breaking it.

When I check the websocket spec, page 17, point 2 it says

"The method of the request MUST be GET, and the HTTP version MUST be at least 1.1."

So I initially thought Litestar was the problem and filed an issue over there.

The Litestar maintainers advised that the ASGI spec does not include a "method" key and so it would in theory be against the ASGI spec for Nginx Unit to be sending it.

If you take a look at the ASGI specs, the websocket scope does not define a method key: https://asgi.readthedocs.io/en/latest/specs/www.html#websocket-connection-scope.

The RFC you are referring to is the initial request, which isn't handled by the ASGI app, but the ASGI server.

I have a more detailed writeup over on the Litestar github here

I also made a git repo that allows you to reproduce the problem here: https://github.com/FixFlare/bug_3887

If I can clarify anything about the repo or the issue please let me know.

Thanks,

[gourav-kandoria]

@FixFlare My say in this would be that deducing the connection type should not be done from the "method" field of the scope object.
"type" field is there for that. I agree, "method" field is not relevant in websocket scope object. But in that case, framework shouldn't be bothered about it whether it is present or not.
BTW, can't comment directly whether it is to be considered as breaking specification from unit's end or not. In this case, it might be seen as breaking specification. But in other cases, where framework don't bother about this field. things should work correctly

Although, If this need handling at nginx unit end. can contribute to that.
@callahad @ac000

[provinzkraut]

My say in this would be that deducing the connection type should not be done from the "method" field of the scope object.
"type" field is there for that. I agree, "method" field is not relevant in websocket scope object. But in that case, framework shouldn't be bothered about it whether it is present or not.

As a framework author, I'm not sure I agree with this. While it may not be ideal to make assumptions based on the method, it's certainly valid according to the spec, and I would assume this to work with every server implementing the spec (correctly, which I think is the goal here 🙂).

We could of course change the behaviour in Litestar, but that doesn't prevent other frameworks from having the same issues when deployed with nginx unit, if they rely on the spec being implemented the same way we do, so I believe fixing this here would be more beneficial overall.

[FixFlare]

@gourav-kandoria

I understand the point being made by both you and @provinzkraut.

As a neutral party I think it is a slippery slope if we say that you can take any specification and start adding your own fields as long as they don't already exist in the spec.

What if the spec decides to officially add that field at a later date?

IMO, I think that would set a bad precedent for the open source community.