Hi all,
At Stefan's suggestion, I took a look at the new Upgrade-related hooks
with the Protocols directive, and I tried to get mod_websocket to use
the same machinery as an experiment. I have some thoughts and questions
based on that -- hopefully they're of interest and/or use to you.
== Immediate Concerns ==
1) WebSocket requires a handshake to be made with the 101 response,
which means a module that implements the protocol_switch hook needs to
be able to affect the response headers, as well as abort the upgrade
with a 4xx/5xx if necessary. To do this I added a hook that I called
pre_protocol_switch; you can take a look at the patch at
https://github.com/jchampio/httpd/commit/26fba7261b2d1d194c693cf7206633766af56afb
I also ported mod_websocket to use this new experimental hook along with
the other Protocols-related hooks, so you can see what a module using it
might look like:
https://github.com/jchampio/apache-websocket/blob/dev/protocol_switch/mod_websocket.c
I'm not sure I like my new hook, to be honest. I think I may have made
it too general with the RUN_ALL hook implementation and I'm not
convinced that more than one module needs to be able to affect an
Upgrade handshake. I also don't like the brittle coupling the
mod_websocket implementation has: when the module responds to
pre_protocol_switch, it takes several actions (mutex locking,
subprotocol negotiation, allocation) with the assumption that it will be
the module handling the protocol_switch hook. If another module steals
the switch, we'll be left with leaked mutexes and probably hung client
plugins.
But I haven't come up with a better idea yet. A corresponding
post_protocol_switch hook _might_ help with cleanup in the general case,
but I don't think mod_websocket's implementation could make use of it.
Perhaps a hook is not the right way to do what I'm trying to do.
2) WebSocket requires the Upgrade protocol to be handled
case-insensitively (`Upgrade: websocket` and `Upgrade: WebSocket` are
both valid), but the current protocol_propose implementation dance is
case-sensitive. I hacked ap_array_str_index to use strcasecmp just to
get it to work on my end, but it made me wonder: are Upgrade header
values case-sensitive as a general rule? I wasn't able to find any
statement in RFC 7230 on this, unlike the rule for Connection header
values (they're case-insensitive).
== Medium-term (more WebSocket-specific) ==
1) WebSocket prefers that the server close the TCP connection before the
client, and if I don't do an explicit close before returning from
protocol_switch, Autobahn fails my test cases because the server is
apparently waiting for the client to close first. But if I close the
connection explicitly, mpm_event complains about a "network write
failure in core output filter", so that doesn't seem good either. Is
there a "proper" way to do this on my end, or is there more
architectural work that would be needed to support server-led TCP closes?
2) Protocols is per-virtual-host, but I think typically people want to
enable WebSocket for a particular URI, not an entire server. And it
doesn't make much sense to me to set a "preference" for WebSocket vs.
HTTP/2 in the Protocols directive -- they're doing entirely different
things, and I don't think a client is likely to send them both in the
same upgrade request anyway. Maybe WebSocket is just an oddball protocol
from Upgrade's point of view?
== Long-term/Idle Thoughts ==
In no particular order:
- The access_log is only updated (with the 101 status) once the entire
connection comes to an end, which seems undesirable.
- h2_session_process seems conceptually similar to mod_websocket's
framing loop, which I think is a good thing. We seem to have settled on
the same way to solve the "read and write simultaneously" problem.
- How does mod_h2 interact with MPMs? From a first reading it seemed
like it would always split requests into separate threads. Are there any
plans to have it use different strategies according to which MPM is
installed? (In particular, I'm hoping that one WebSocket connection
won't necessarily have to steal an entire thread from httpd, and that
MotorZ might help us out here...)
--Jacob
