ASGI Flow error bug

[dfourie]

Background I have granian serving a django channels application. We noticed we were getting an ASGI Flow error on some of the connections. This was on granian 1.1.0.

I did see there was 1.2.0 out, so I upgraded that on our staging server. During my initial testing, I found that there was an ASGI flow error when granian was sent the following ASGI message from python.

{'type':'websocket.accept', 'subprotocol': None}

Where None is python's None type

This message was triggering the ASGI flow error. After looking at the source code, I saw that in io.rs, it was trying to extract a value from the key.

If I look at the ASGI specification, the message sent above technically complies with it:

class WebSocketAcceptEvent(TypedDict):
    type: Literal["websocket.accept"]
    subprotocol: Optional[str]
    headers: Iterable[Tuple[bytes, bytes]]

as the subprotocol can either be a string or None https://github.com/django/asgiref/blob/db3ff43e9fa1daf7c6b1cb67db8eec1885be911d/asgiref/typing.py#L161

From what my limited skills in rust could tell me, it seems there was an assumption in the code that if the key is there, it is a valid string key. It doesn't cover the None case.

As I have zero skill in Rust, I used Copilot to implement a fix, to which I have added tests for. https://github.com/dfourie/granian/tree/subprotocol

The new code return None if there is an error extracting the subprotocol key

                "websocket.accept" => {
                    let subproto: Option<String> = match message.get_item(pyo3::intern!(py, "subprotocol")) {
                        Ok(Some(item)) => {
                            match item.extract::<String>() {
                                Ok(s) => Some(s),
                                Err(_) => None,
                            }
                        },
                        _ => None,
                    };
                    Ok(ASGIMessageType::WSAccept(subproto))
                }

Which I believe is more the way that the python asgi servers handle the issue. The tests should show where the error is, but the tests I addeed it seemed to mess up the rsgi test, so I bypassed it.

Thanks for all the work