search

iande / onstomp

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

stomp+ssl failover not working? #37

Closed l8nite closed 5 years ago

l8nite commented 5 years ago

I have the following connection string: failover:(stomp+ssl://host1:61614,stomp+ssl://host2:61614)?transport.timeout=5000

I'm also passing the ssl: option in the OnStomp::Client constructor, with a path to a CA cert bundle.

host2 is our currently active broker, and host1 is currently unreachable.

The onstomp connection never seems to establish (seems to hang). I am seeing an error in our host2 broker logs that indicates the SSL handshake is failing ("unknown_ca").

I'm wondering if the failover client isn't specifying the ssl CA cert option on the failover? I see #26 was filed, but I'm not sure if this is the same issue?

If I switch the order of hosts in my connection string (host2, then host1), it connects and works as expected.

Unfortunately this is production-impacting for us at the moment, so I'm hoping you might have some ideas?

l8nite commented 5 years ago

I think I found the problem, https://github.com/iande/onstomp/blob/master/lib/onstomp/client.rb#L67 is deleting the :ssl config from the options hash, which makes it so that the second failover client doesn't receive the configuration

iande commented 5 years ago

I think you’re absolutely right, Issue #26 was related, but I’m going to bet I stupidly pass the options along by reference, without duping, so the delete in the base client at line 67 wrecks it for the rest. Let me get a PR up for this.

iande commented 5 years ago

In an unrelated note: I appreciate all the work you’ve done in calling out and patching bugs you’ve found in the library recently. I did most of the OnStomp development at a time when I was interacting with ActiveMQ on a daily basis, which hasn’t been the case for a few years now and the gem has languished because of it. These PRs and issue reports have been helping motivate me to at least bring this up to date with current Ruby standards.

l8nite commented 5 years ago

Ah sorry was working on a spec and PR and didn't see your comment - hopefully this passes muster, but I'll let you decide :)

iande commented 5 years ago

No problem, our fixes were pretty much identical and you got there first, so I've merged in yours. I'll do a version bump on master and push a new build of the gem.