host header - Fixed support for STOMP protocol versions 1.1

[nser77]

[EDITED]

Hi, if the vhost option is not specified in stomp.py, the STOMP command (CONNECT) has unexpected behavior for STOMP versions 1.1.

STOMP 1.1 frame:

if vhost option is not specified and the selected STOMP version is 1.1, stomp.py will send a STOMP command without host header.

STOMP
accept-version:1.1
login:user
passcode:password

.

---

Ss per the STOMP protocol specification, the host header MUST be set:

[...]
STOMP 1.{1,2} clients MUST set the following headers:

    accept-version : The versions of the STOMP protocol the client supports. 
    See [Protocol Negotiation](https://stomp.github.io/stomp-specification-1.1.html#Protocol_Negotiation) for more details.

    host : The name of a virtual host that the client wishes to connect to. It is recommended clients set this to the host name that the 
    socket was established against, or to any name of their choosing. If this header does not match a known virtual host, servers 
    supporting virtual hosting MAY select a default virtual host or reject the connection.
[...]

I tried to fix this protocol compatibility for versions 1.1 by setting the host header with self.transport.current_host_and_port[0] , same as protocol version 1.2.

In addition, the CLI also needs to add an argument to pass the host header via the command line (working on it) and maybe the code related to the header set block could be changed to a method shared betwen classes to avoid code repetition (?).

Am I wrong with this patch?

Reproduce

Run a simple tcpdump as following:

 tcpdump -An port 61613 and proto 6

Then run the following script:

import sys
import stomp

stomp.logging.verbose = True
conn = stomp.Connection11([("10.1.0.14", 61613)])
conn.connect('user', 'password', wait=True)
conn.send(body=' '.join(sys.argv[1:]), destination='/queue/test')
conn.disconnect()

or the following commands:

stomp -H 10.1.0.14 -P 61613 -U user -W password -V -S 1.1

[nser77]

STOMP protocol 1.2 doesn't really need to be fixed so I reverted the changes.

I also aligned the protocol version 1.1 by setting the host header with self.transport.current_host_and_port[0] the same as version 1.2.

The STOM documentation says the following:

It is recommended clients set this to the host name that the socket was established against, or to any name of their choosing.

So it seems that putting the dst ip in the host header is not wrong, but rather a particular escenario in my case; however, the host header must be setted in STOMP 1.1.

[jasonrbriggs]

Unforunately not mergeable because it breaks the test suite:

=============================================== short test summary info ================================================
ERROR tests/test_misc.py::TestMessageTransform::test_transform - stomp.exception.ConnectFailedException
ERROR tests/test_misc.py::TestMiscellaneousLogic::test_original_headers - stomp.exception.ConnectFailedException
ERROR tests/test_rabbitmq.py::TestRabbitMQSend::test_send - stomp.exception.ConnectFailedException
FAILED tests/test_basic.py::TestBasic::test_default_to_localhost - stomp.exception.ConnectFailedException
FAILED tests/test_basic.py::TestBasic::test_host_bind_port - stomp.exception.ConnectFailedException
================================== 2 failed, 92 passed, 3 errors in 115.73s (0:01:55) ==================================

[nser77]

Unforunately not mergeable because it breaks the test suite:

=============================================== short test summary info ================================================
ERROR tests/test_misc.py::TestMessageTransform::test_transform - stomp.exception.ConnectFailedException
ERROR tests/test_misc.py::TestMiscellaneousLogic::test_original_headers - stomp.exception.ConnectFailedException
ERROR tests/test_rabbitmq.py::TestRabbitMQSend::test_send - stomp.exception.ConnectFailedException
FAILED tests/test_basic.py::TestBasic::test_default_to_localhost - stomp.exception.ConnectFailedException
FAILED tests/test_basic.py::TestBasic::test_host_bind_port - stomp.exception.ConnectFailedException
================================== 2 failed, 92 passed, 3 errors in 115.73s (0:01:55) ==================================

Hi and many thanks for your answer, from my understanding, stomp.exception.ConnectFailedException is a socket error and is not related to the STOMP host header (application level); seems that this exception is raised when a connection to a remote host (i.e: ssl, destination ip, etc..) fails.

https://github.com/jasonrbriggs/stomp.py/blob/ec070683efaefc64e4eec7dc35a97256a89e0857/stomp/transport.py#L806

Is it possible that those test suite errors are only related to your local environment?

[jasonrbriggs]

I don't think so. If I run the tests on the main dev branch, they run fine:

============================================ 97 passed in 115.34s (0:01:55) ============================================

[nser77]

Hi, many thanks for your input.

I will open a Issue, maybe someone can help this case.