Added encoding to user name and password #148
Conversation
| var lastAtIndex = this.mongoURI.lastIndexOf('@'); | ||
| var encodedMongoURI = this.mongoURI; | ||
| var username = null; | ||
| var password = null; |
There was a problem hiding this comment.
It does not look like username or password are used... and it looks like the database uri is being somewhat validated here, so how about we throw an error if it's not?
|
@gfosco sorry for that, I've removed old references. I've added a regex validation for the mongo URI. It won't capture all invalid URIs, but it will at least validate the monogURI to be in the form we expect, as specified here. Also, not sure what your policy is on Regex, but I usually add a comment with a link to the tester. If this is against your standards, let me know and I'll remove it. |
Added encoding to user name and password
|
Thanks for adding the regxr link.. very cool. 👍 |
|
This causes MongoDB Replica Sets to no longer work. |
|
@themichael123 can you provide an example URI? |
|
@taylorstine Have a look here: https://docs.mongodb.org/manual/reference/connection-string/ I wouldn't use regex for this task. |
|
@taylorstine @gfosco here's an example of a paid MongoLabs URI that isn't working: mongodb://test:[email protected]:59325,ds059315-a1.mongolab.com:59315/testDBname?replicaSet=rs-ds059415 here's an example of a free sandbox MongoLabs URI that is working: |
|
I confirmed that the MongoLabs replica set URI worked fine in Parse Server 2.0.4, the breaking change was in 2.0.5 |
|
Let's get a fix PR sent in to parse-server to address this... @taylorstine are you interested in doing it? |
|
@gfosco sure, but I could use a second opinion on the regex. My aptitude for regex has kind of reached it's limit, maybe it should just be removed? |
|
@gfosco @taylorstine I'd vote for removing the regex until we can be very confident in it. this bug breaks most parse server deployments that aren't running in the sandbox. |
|
The approach we used on hosted parse was to remove one character at a time from the end of the connection string until it looked like a regular URI, and then we could use a regular URI parsing library on it, modify the contents as needed, and then rebuild the connection string that includes all the replica sets. That approach might work here as well. Make sure to add tests if you use that approach though. |
|
In my case it's moving from a sandbox to a production environment on MongoLabs that breaks it. It appears to be that the additional ?replicaSet argument added onto the URI fails the regex expression, I'd suggest taking it out as it's a little over-engineered. Testing the Mongo connection success/failure is probably worth while but it seems better to just trust people that they've entered a valid uri and will preserve future changes to the URI structure / arguments |
|
#297 I'm not confident in this approach, I don't want to break anything just because my password has a |
|
So could we split the full host on "@", then take the last part as the 'real' host and join the [0, n-1] parts together? Then from that, we split on ":" to get the username / password. |
Tests PSR4 autoloading
The mongodb driver requires auth values be URI encoded: mongodb/node-mongodb-native@0440630 This uses node's built-in url module to encode the auth portion, by parsing and re-formatting it, which causes special characters to get URI encoded properly: https://nodejs.org/api/url.html#url_escaped_characters This is all a bit silly since mongodb just takes our passed uri, and runs it through the same url parser again, but not before explicitly erroring on '@' characters in the uri. This is similiar to #148 (reverted by #297), but with much less code, and hopefully less breakage. Also, note that `uri_decode_auth` is no longer needed. That was removed in the above referenced node-mongodb-native commit. I've tested this on usernames and passwords with @, !, +, and a space. Presumably this would also work with usernames and passwords that are already URI encoded (since parseUrl will simply unescape it, and formatUrl will escape it again).
The mongodb driver requires auth values be URI encoded: mongodb/node-mongodb-native@0440630 This uses node's built-in url module to encode the auth portion, by parsing and re-formatting it, which causes special characters to get URI encoded properly: https://nodejs.org/api/url.html#url_escaped_characters This is all a bit silly since mongodb just takes our passed uri, and runs it through the same url parser again, but not before explicitly erroring on '@' characters in the uri. This is similiar to #148 (reverted by #297), but with much less code, and hopefully less breakage. Also, note that `uri_decode_auth` is no longer needed. That was removed in the above referenced node-mongodb-native commit. I've tested this on usernames and passwords with @, !, +, and a space. Presumably this would also work with usernames and passwords that are already URI encoded (since parseUrl will simply unescape it, and formatUrl will escape it again).
I have an @ symbol in my mongo password. I realize that I can specify a custom class for the DatabaseAdapter, but I believe the decoding of a username and password should be built in to the default implementation.