When connect's host param is left as None the server hostname should be used instead of '' about stompest

[nikipore]

That's a good suggestion. Would you mind to provide a patch for that, or do you prefer me to do that?

[dantman]

Right now I'm just working on a side project in my spare time that happens to use STOMP and tracking issues I run into along the way.

If I get into the project seriously with more time to play with I'll write the stuff in myself. But in the meantime if you feel like implementing it go ahead.

[nikipore]

I just meditated over this issue, and these questions came up:

1. What do you mean by a "working" host header? For ActiveMQ, you don't need that header at all to get a connection to "work", and for Apollo it should be a virtual (that is, logical) host rather that a physical address, so I right now don't see the rationale of putting a physical address in that header. The broker will know its physical address anyway. I'd rather expect a virtual host to remain the same upon failover to another broker instance.
2. What if you used another transport? Say, a non-IP transport. What would you fill in for the host then?

To me it looks like one shouldn't intermingle the physical transport and the logical STOMP session, and the code seems to confirm that, because it is not so easy to implement the desired behavior for both clients (sync and async) without mixing transport and STOMP session concerns (breaking SRP) and writing redundant code in both clients (breaking DRY).

@dantman: What's your opinion on this?

[dantman]

Apollo's default (and/or example) configs setup a virtual host named (localhost, 207.0.0.1, etc...). When configured like that Apollo will generally reject requests with an empty host header if you use STOMP 1.1+. By working I'm talking about what you need to do to get something that's non-empty. Because host doesn't carry over anywhere from config leaving you stuck in an awkward flow where you have to explicitly carry some extra custom config into other parts of your code just to set the host header.

virtual hosts in STOMP are basically carried over from HTTP. They work the same way. Filing the host header with whatever address or hostname you used to connect with is the expected behaviour. The purpose is basically so that if you point the dns for foo.tld and bar.tld to the same address the host header will respectively contain foo.tld and bar.tld and the server will be able to differentiate between the two. It's practically never an explicit name. There is nothing really strange about two different servers even for fallover getting different hosts. Virtual host configuring is unique to each server. Using the same virtual host name on each server is not a requirement for connecting them to the same messaging group.

The STOMP spec is actually quite clear on this behaviour "It is recommended clients set this to the host name that the socket was established against, or to any name of their choosing." which would basically in our case would translate to defaulting to the hostname of the connected server but providing a config that lets you set a fixed host for all connections.

On a non-IP transport. IF you were directly connecting through it you'd probably leave that blank. host is inherently an IP transport oriented header. You don't have the same kind of limitations you do in IP with say a unix socket. In that case if there were any header at all it should be explicitly set. (Although for that circumstance we should still add some extra config to allow that to be specified in config instead of forcing it to be passed to connect).

[nikipore]

In the Apollo integration tests, I set the hostheader to the logical host name mybroker, on RabbitMQ, it is /. Both aren't really socket addresses.

But I think you're right: In their connect() methods, sync and async client should store the "name" of the host they connected against on the wire level and substitute a missing host argument (None). But the transport level should decide what that "name" is. It is very important to keep both clients transport agnostic.

In the async client, we should set the hostattribute on the StompProtocolinstance and let the Stomp.connect() method substitute the hostheader here. The definition of framehas to move further down; there is no reason for it to be so far up anyway.

The StompFrameTransport of the sync client already has this hostattribute, so the substitution in Stomp.connect() can take place in exactly the same way.