Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amqplib 0.5.3 doesn't connect if rabbitmq password contains a "@" sign. #483

Closed
tolik-u opened this issue Dec 3, 2018 · 8 comments
Closed

Comments

@tolik-u
Copy link

tolik-u commented Dec 3, 2018

Since 0.5.3, password parsing has changed and now passwords with "@" in them don't work.
Wild guess:

Use 3rd party url for better decoding of username/password ([PR 395])(#395), as discussed in issue 385)

@GrayStork
Copy link

If url is provided in form of amqp://username:pass@word@host.domain.com:5672/vhost starting from 0.5.3 version url parsing logic will split by '@' and return word@host.domain.com as host, to which it attempts to connect (and fail miserably). This was working properly in 0.5.2 and lower versions.

@daboytim
Copy link

daboytim commented Dec 3, 2018

FYI, this bug also affects usernames which contain an @. For example amqp://user@mail.com:password@host:port will attempt to connect to the host mail.com:password@host. This works properly in v0.5.2.

@cressie176
Copy link
Collaborator

cressie176 commented Dec 3, 2018

Sorry about the breaking behaviour. It wasn't expected. Prior to 0.5.3 URL parsing was bugged in that usernames with a : didn't work properly (see #385).

Introducing url-parse fixes that, but appears to have highlighted another bug in the pre 0.5.3 versions, which illegally allowed @ characters in the userinfo section of the URLs without percent encoding (at least if I'm reading the spec right).

   authority     = [ userinfo "@" ] host [ ":" port ]
   userinfo      = *( unreserved / pct-encoded / sub-delims / ":" )
   unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
   reserved      = gen-delims / sub-delims
   gen-delims    = ":" / "/" / "?" / "#" / "[" / "]" / "@"
   sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
                 / "*" / "+" / "," / ";" / "="

@ is a gen-delims and therefore not allowed in the userinfo without percent encoding. If you replace the @ in the userinfo section with %40 as per unshiftio/url-parse#154, I suspect the connection will work.

I don't think it's appropriate to "fix" this issue by reverting to the previous invalid and broken behaviour, but one thing we might be able to do is deprecate 0.5.3 and re-release as 0.6.0. What do you think @squaremo?

@daboytim
Copy link

daboytim commented Dec 3, 2018

Thanks for the suggestion using %40 in place of @ but unfortunately this doesn't work for me as I get an authentication error back from the broker (rabbitmq 3.6). I'll stick with 0.5.2 for now.

@squaremo
Copy link
Collaborator

squaremo commented Dec 4, 2018

Thanks for the suggestion using %40 in place of @ but unfortunately this doesn't work for me as I get an authentication error back from the broker (rabbitmq 3.6)

I created a user that happens to have an '@' in the username, user@name, with password pass. Then,

  • when I attempt to connect with amqplib using the default amqp://guest:guest@localhost, it works fine;
  • when I attempt to connect with amqplib using the new user, amqp://user@name:pass@localhost it complains about the hostname being unresolvable (which is correct, according to the specification or URLs)
  • when I attempt to connect with amqplib using the correct encoding of amqp://user%40name:pass@localhost it complains that it got connection close (which is unexpected!);
  • when I try to connect to amqp://guest:notthepassword@localhost I get a clear authentication error.

It seems like using the correct encoding does cause problems, and that's a bug most certainly. But reverting it would mean going back to : in usernames causing a very similar problem, as well as being contrary to the specification or URLs, and I'm not keen to move backwards two steps.

So for now, if this causes problems for you, consider either sticking with the prior version, or using an object value rather than a URL when connecting. Meanwhile we'll try to track down where the problem lies.

@squaremo
Copy link
Collaborator

squaremo commented Dec 4, 2018

  • when I attempt to connect with amqplib using the correct encoding of 'amqp://user%40name:pass@localhost' it complains that it got connection close (which is unexpected!);

Mystery solved -- I had forgotten to give my new user access to a vhost. Once I did that, it was able to connect, with a percent-encoded username or (after I'd updated it in rabbitmq) password.

Can someone post exact instructions, including creating the user, for getting this to fail, please?

@cressie176
Copy link
Collaborator

@daboytim @tolik-u @GrayStork still not able to reproduce after changing to percent encoded usernames. Are any of you able to provide exact instructions and ideally a code sample please?

@cressie176
Copy link
Collaborator

Closing due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants