search

noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

Problem with Policy.prototype.parseAddress and auth #363

Open gbeister opened 6 years ago

gbeister commented 6 years ago

Within the policy.js in amqp10/lib/policies the Policy.prototype.parseAddress is wrongly retrieving the authentification information. Currently its done via: var auth = matchAuth.exec(address)[1]; but should be: var auth = parsedAddress.auth; Background: an amqp-endpoint like: amqp://<user-name containing an @>:<password>@<end point>:<port> will lead into an empty password and invalid user-name for client connection. Those endpoints do not work currently. With the above fix proposed everything works fine.

ps2goat commented 6 years ago

I had the same issue with my password. use encodeURIComponent() on your user name and password before putting it into your url. I haven't specifically tested with an @, but it was working for other special chars.

moebigit commented 6 years ago

I also wonder what the purpose of this code section is?

The code checks whether the url.parse(address) has set a valid auth information, but then ignores this and reparses the entire url string applying the decodeURIComponent(authSplit[0]) on the username.

As reported by @gbeister - @ is not an encoded character and the regex does not properly match a valid URI which allows the username to contain a @-symbol.

A fix for this is adjusting the regex to the following (and maintaining the current code logic): var matchAuth = /amqps?:\/\/([^:]+:[^@]+).+/g;

But @gbeister approach is actually more straight forward: apply the decodeURIComponent to the parsedAdress.auth instead.

It would be great to understand the motivation behind the current approach.