stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

Anonymous connections #75

Closed OrazioCW closed 10 years ago

OrazioCW commented 10 years ago

Fixed anonymous connections by not setting the credentials in the headers of the request if not specified by the initiator.

ismith commented 10 years ago

So, you can already do anonymous connections if you use hash-based params. My dev environment's client setup, for example, is something like Stomp::Client.new(..., :hosts => [{:host => localhost, :port => 61613, :ssl => false}]).

I'd rather make hash-based params the way of the future; positional params should probably stick around as an option so as not to break for existing installations, but I'd rather not add options to them.

Does that make sense? I'm open to hearing arguments for doing this, but I'm not inclined to accept this pull req without that.

PaulGale commented 10 years ago

+1

Let the positional constructors wither on the vine.

On Mon, Sep 23, 2013 at 6:01 PM, Ian Smith notifications@github.com wrote:

So, you can already do anonymous connections if you use hash-based params. My dev environment's client setup, for example, is `Stomp::Client.new({:host => localhost, :port => 61613, :ssl => false}).

I'd rather make hash-based params the way of the future; positional params should probably stick around as an option so as not to break for existing installations, but I'd rather not add options to them.

Does that make sense? I'm open to hearing arguments for doing this, but I'm not inclined to accept this pull req without that.

— Reply to this email directly or view it on GitHubhttps://github.com/stompgem/stomp/pull/75#issuecomment-24958970 .

OrazioCW commented 10 years ago

Thank you for the suggestion. I tried with that setup before but unfortunately it did not work for me.

Even if login and passcode are not provided, when connecting, the login and passcode are set in the headers of the request (nil OR "") making the request itself not anonymous.

def connect(used_socket) ... headers[:login] = @login headers[:passcode] = @passcode ...

When the broker receives the request, fires the following exception:

I am using the simple authentication plugin allowing anonymous connections

Please let me know if I am doing something wrong.

PaulGale commented 10 years ago

I believe you're correct sir. This is a bug. Those headers should not be present if the login or passcode have no values.

On Tue, Sep 24, 2013 at 5:23 AM, OrazioWE7 notifications@github.com wrote:

Thank you for the suggestion. I tried with that setup before but unfortunately it did not work for me.

Even if login and passcode are not provided, when connecting, the login and passcode are set in the headers of the request (nil OR "") making the request itself not anonymous.

def connect(used_socket) ... headers[:login] = @login headers[:passcode] = @passcode https://github.com/passcode ...

When the broker receives the request, fires the following exception:

  • java.lang.SecurityException: User name [] or password is invalid.

I am using the simple authentication plugin allowing anonymous connections

Please let me know if I am doing something wrong.

— Reply to this email directly or view it on GitHubhttps://github.com/stompgem/stomp/pull/75#issuecomment-24987884 .

ismith commented 10 years ago

This could be solved without the added anonymous param via: headers[:login] = @login unless @login.nil? || @login.empty?.

OrazioCW commented 10 years ago

It looks like a good solution to me.

The same solution should be applied to the passcode as well.

otherwise it will not work. The broker will raise the following exception:

PaulGale commented 10 years ago

FYI you can simplify the expression:

headers[:passcode] = @passcode unless @login.nil? || @login.empty?

to

headers[:passcode] = @passcode unless @login.to_s.empty?

Just a thought.

OrazioCW commented 10 years ago

I have changed the pull request based on the comments you gave me.

ismith commented 10 years ago

@OrazioWE7 I'm getting an error and a failure when I run test_anonymous (ruby -Ilib test/test_anonymous.rb). lib/connection/utils.rb:142, sleep can't convert Nil into a time interval; and test/test_anonymous.rb:434 raises because it can't convert nil to string.

ismith commented 10 years ago

Interesting - lib/connection/netio.rb:342 wants (@parameters && @parameters[:tcp_nodelay]) to be a boolean, and nil doesn't work there. Prefacing that parenthetical with !! fixes it. You mind making that change for me, @OrazioWE7? That, plus a nicer commit message, and I think it's ready to go.

EDIT: My apologies, the tcp_nodelay is a bug in my code; I'll handle that. I do want a nicer commit message, though.

OrazioCW commented 10 years ago

I have updated the comment.