iande / onstomp

A STOMP messaging client library for Ruby
http://mathish.com/projects/onstomp.html
Other
23 stars 11 forks source link

Empty login/passcode headers being sent in CONNECT frame. #28

Open iande opened 9 years ago

iande commented 9 years ago

From the description of pull request #25 by @skanct:

In the current implementation, if the user does not define a login and passcode, then the library sets their values to '' and the headers that are sent to the STOMP server are the following:

CONNECT
accept-version:1.0,1.1
host:stomp-server.example.com
heart-beat:0,0
login:
passcode:

When this is tested against ActiveMQ (version 5.8.0), the connection fails with the following error:

ERROR
content-type:text/plain
message:User name [] or password is invalid.

According to the STOMP protocol specification, the clients MAY set the login and passcode headers. It seems that when the user does not define a login and passcode, then the CONNECT frame must not include these headers.

This commit changes the existing behaviour and when the user does not define a login and a passcode, then the library does not include these headers in the CONNECT frame.

iande commented 9 years ago

First, thank you @skanct for bringing this to my attention and apologies for taking so long to address your pull request.

How I've handled header/value generation when empty/nil values are in OnStomp has been wrong from pretty much the beginning. I think any empty or nil header value should be omitted from the frame serialization, and making that correction will also address this issue.

skanct commented 9 years ago

Thank you @iande for the response. I was not very familiar with the OnStomp architecture. I had a look at ba979e2 and it certainly seems to be cleaner and more elegant solution. Thank you for addressing this.

iande commented 9 years ago

I'll do a version bump on the gem with this change in it, but I'm holding off for the moment while I try to think of any scenario where an empty header value would be needed. Once I'm reasonably sure this won't screw up existing use cases, I'll push out the new version to rubygems.org