node-mongo-queue icon indicating copy to clipboard operation
node-mongo-queue copied to clipboard

Fix for colon between username and password being encoded

Open seancunningham opened this issue 9 years ago • 11 comments

At very least, Mongo v. 2.4.6 does not like the colon between the username and password encoded. This update simply encodes them separately to avoid such issues.

seancunningham avatar Mar 16 '16 17:03 seancunningham

A more solid solution would probably be to use node's native url.format to build that thing.

Marsup avatar Mar 16 '16 21:03 Marsup

Yes, I agree. Truthfully it was a quick fix at first, as it was blocking me from something deemed more important. Circled back and fixed based on the suggestion.

seancunningham avatar Mar 16 '16 22:03 seancunningham

You shouldn't have forced the add on lib/queue. Also is it really solving your problem now ? Because it does a global encode on the auth as well.

Marsup avatar Mar 17 '16 09:03 Marsup

Yes, it is. I'm assuming it has to do with the assumption the colon needs to be encoded when using encodeURIComponent, where it is expected in the auth field.

seancunningham avatar Mar 17 '16 14:03 seancunningham

Also, I can remove lib/queue, if you want to merge it. I didn't want to publish my branch onto npm, but still needed it for a project - so I added lib/queue so it could be installed directly from github using npm.

seancunningham avatar Mar 17 '16 14:03 seancunningham

It already can, that's the whole point of the prepublish script. I'm doubtful it fixes your problem but ok.

Marsup avatar Mar 17 '16 15:03 Marsup

URL.format, and just not encoding the colon in the auth field, both work because the colon gets encoded with encodeURIComponent. Does your method work for you? I would be surprised to hear that user%3Apassword works as an auth field with mongo, but maybe I'm just running an older version. If you dig into URL format you'll find they ended up doing the same thing as my original fix (encoding each separately and just concating the string. (see here for ref).

seancunningham avatar Mar 17 '16 15:03 seancunningham

I mean in the current state of this PR, it doesn't seem any different from before on this encoding part.

Marsup avatar Mar 17 '16 16:03 Marsup

Have you tested it? Because what I am saying is that your version yields username**%3Apassword, whereas my first commit and this current commit both give username:**password, and I am having a hard time telling whether you are saying that you can't see the difference while thinking about it or that you have tested it and the encoded colon in the auth field works for the version of mongodb you are running (because it doesn't work for mine, mongo sees it as a single username string with no password).

seancunningham avatar Mar 17 '16 17:03 seancunningham

I haven't tested it but conceptually both versions encode user:password, the whole string, node also does it in a single pass (https://github.com/nodejs/node/blob/master/lib/url.js#L543).

Marsup avatar Mar 17 '16 17:03 Marsup

They don't escape the colon though - (https://github.com/nodejs/node/blob/master/lib/url.js#L933)

seancunningham avatar Mar 17 '16 17:03 seancunningham