this regexp does not handle dsn like:

• ?foo_collection=x&collection=y
• #collection=x

Should we handle such situation?

Sinceparse_url() is called above, is there any reason not to restrict this matching to the already-parsed query component in$parsedUrl['query']? That would avoid the need to check for? and# in this pattern.

Anecdotally, MongoDB'sconnection string format does not use fragments (i.e.#) and I can't speak for how all drivers might interpret a fragment. I expect some drivers might capture it as a value for a query key (e.g.?foo=bar#fragment might parsebar#fragment as the value) -- but I think that's all outside the scope of this PR.

As for strippingcollection=<value> from the returned string, I think we can again leverage the result ofparse_url() to rebuild the original string. I understand simply replacingcollection=<value> anywhere in the$uri argument could pose a problem if it hypothetically appeared in a username/password component. One suggestion in myoriginal comment was to use thei modifier for case-insensitive match since connection string options are technically case-insensitive. I realize it may be intentionally to not do so here, sincecollection= is just being defined for this Symfony class. My second suggestion was to interpret the value as any sequence of characters other than&, since that would indicate the next connection string option. I think that's still reasonable if we do the matching on thequery field returned byparse_url().

I also testedparse_url() with two types of non-standard (outside of MongoDB) connection strings: replica sets and sharded clusters which have a comma-delimited list of host/port combinations, and the custom SRV scheme:

>>> parse_url('mongodb://user:password@mongodb0.example.com:27017,mongodb1.example.com:27017/?replicaSet=myRepl')=> [     "scheme" => "mongodb",     "host" => "mongodb0.example.com:27017,mongodb1.example.com",     "port" => 27017,     "user" => "user",     "pass" => "password",     "path" => "/",     "query" => "replicaSet=myRepl",   ]>>> parse_url('mongodb+srv://server.example.com/')=> [     "scheme" => "mongodb+srv",     "host" => "server.example.com",     "path" => "/",   ]

I don't believe any data is lost here (despite the odd wayhost is parsed in the first example). Reassembling can use the following concatenation:

• scheme +://
• Ifuser is present:user
• Ifpassword is present:: +password
• Ifuser was present:@
• host (potentially includes multiple hosts and ports for all but the last host)
• Ifport is present:: +port (port for the last host if there were multiple)
• Ifpath is present:path
• Ifquery is present:? +query
• Iffragment is present:# +fragment

Althoughparse_url() does not seem to require it, MongoDB drivers do tend to require a path separator/ between the hosts and query components; however, I don't think you need to care about that. Ifpath isn't picked up byparse_url, you can leave that component out of the returned string and let the driver raise an error that it normally would hadskimUri never been invoked.

@jderusse

I'll add that test case and support it. Although it's not a thing, if i can get the strip to be more robust it will better handle any new options introduced to mongodb uris in the future.

@jmikola

Since parse_url() is called above, is there any reason not to restrict this matching to the already-parsed query component in $parsedUrl['query']?

Because it would introduce a lot more complexity than it eliminates, reconstructing a uri from the output ofparse_url is quite involved as you pointed out in your concatenation list. I want to keep this method as simple and non-invasive as possible.

MongoDB's connection string format does not use fragments

I'm aware but I thought it was easy to support fragments in case one day mongodb uris used them... just to be more robust.

One suggestion in my original comment was to use the i modifier

As you pointed out, it could appear in the user/pass component. I'd rather be really strict aboutcollection being lower and therefore less likely to strip the wrong part of the uri. Since we've made it up I think it's our prerogative.

My second suggestion was to interpret the value as any sequence of characters other than&

Yeah I think that's good but since i'm interpreting the whole uri, i'm also looking for not#. This looks safe in accordance withRFC 3986.

MongoDB drivers do tend to require a path separator / between the hosts and query components; however, I don't think you need to care about that.

This is one of the reasons why I chose to replace in the uri instead of disassemble / reassemble the uri. The less I change the uri the better. If a developer configures a uri in their application asmongodb://localhost/ for application storage, then appends%mongodb.uri%?collection=lock forLock I want my stripped uri to go back to their original including the/ (and vice versa if/ was omitted). This will allow the driver to cache the connections based on the uri string as both the lock and application connection uris will hash to the same key.https://www.php.net/manual/en/mongodb.connection-handling.php

The less I touch uri the better. The simpler the implementation the better.