autobahn-python
autobahn-python copied to clipboard
crossbario
[Feature] Option to pass extra headers to Component
It would be nice if there was some higher-level way to configure extra headers for a Component. Currently, this can only be done by changing the session_factory.
Our use-case is auth-headers, hence we would probably prefer extra headers as an argument to the Component constructor (I would volunteer for a PR). I am unsure if there is a use-case for extra-headers from the config. Opinions?
If you mean HTTP headers: Component is not concerned about transports, only about the WAMP level, and HTTP headers don't exist there. Processing custom HTTP headers must happen in WampWebSocket.onConnect at the WebSocket transport level (eg WAMP over RawSocket does not have HTTP headers at all). And the natural way is to provide a custom factory. So from my point of view, we already provide all that we should.
Sorry, I did not manage to write-up what we are doing properly. As far as I understand the transport factory is created by autobahn.twisted.component._create_transport_factory. If the transport type is websocket a WampWebSocketClientFactory will be created. Extra headers can be provided to WampWebSocketClientFactory via __init__. However, there seems to be no way to get these extra headers through Component into _create_transport_factory.
What we ended-up doing is subclassing Component and overloading Component._connect_transport. Could we have done something smarter?
Ah, ok, so the Q is how to achieve with Component what ApplicationRunner allows with headers here https://github.com/crossbario/autobahn-python/blob/e00005e297048b05d1e1c46d94c8a2383ce89fed/autobahn/twisted/wamp.py#L154
@meejah not sure .. I remember we've been working on getting Component and ApplicationRunner to feature parity rgd knobs .. like timeouts and such. Does that cover ^ also?
I'll second this. I'm having to drop down to ApplicationRunner to include custom headers. I'd rather use the new, Component API instead.
Yes, I think this counts as a missing feature in Component -- if @asodeur has a PR or proposed method in mind, that sounds great! (If you haven't written code yet, probably best to propose something here on this ticket and we can discuss)
I haven't looked deeply, but probably the right thing is to accept some extra config in the transports= list since this is a transport-specific option
We are currently using the linked code. This a minimal extension to Component to pass in extra headers. However, as @meejah mentions above a better approach is probably enhancing the transports= config.
@asodeur if an enhancement to transport= configurations will work for your use-case and you're still interested in doing a PR, that sounds like a sensible approach to me!
I am in #autobahn as meejah if you want real-time discussion. The reason I think "transports" is the right place is because it's a transport-specific option (e.g. you might have a "unix" socket and a websocket in the transports list; the header only makes sense for the websocket one).
What is the recommended way in 2024 to add a custom HTTP header to autobahn.asyncio.component.Component? The code posted by @asodeur seems awfully complex and reimplements many things like TLS, which should probably be avoided.
ultimately, when using WAMP over WebSocket - and thus over HTTP - custom HTTP headers to be sent by a connecting client should move over this API - both for Twisted, and asyncio, and regardless of wrapping/access like via Component:
https://github.com/crossbario/autobahn-python/blob/359f868f9db410586cf01c071220994d8d7f165a/autobahn/websocket/interfaces.py#L257
headers: An optional mapping of additional HTTP headers to send during the WebSocket opening handshake.
now, having said that, I'm not sure that is available when using asyncio or/and when using Component ...
I patched autobahn-python for a proof of concept with minimal changes, so that the transport config supports a keyword headers which is simply passed. These are really minimal changes (~4 lines of variable assignnments) , and it works.
- I can create a PR for the changes, if you like.
- I took the approach that was easiest with the current code base, where
headersis an optional argument at the root of the transport configuration. I'm not sure, if it would logically fit better into the options parameter, but that is parsed differently and would require more changes.
I can create a PR for the changes, if you like.
That would be great and highly welcome! because then, CI (tests) will run and we have a concrete proposal to discuss (changes to APIs mostly), and such.