8000 bug #37218 [Lock] MongoDbStore skim non-standard options from uri (kr… · symfony/symfony@5f074cd · GitHub
[go: up one dir, main page]
Skip to content

Commit 5f074cd

Browse files
fabpotfabpot
committed
bug #37218 [Lock] MongoDbStore skim non-standard options from uri (kralos)
This PR was squashed before being merged into the 5.1 branch. Discussion ---------- [Lock] MongoDbStore skim non-standard options from uri | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #37180 | License | MIT | Doc PR | Don't allow non-standard connection uri options reach `MongoDB\Client` Commits ------- 67912fa [Lock] MongoDbStore skim non-standard options from uri
2 parents 5ef1679 + 67912fa commit 5f074cd

File tree

2 files changed

+91
-32
lines changed

2 files changed

+91
-32
lines changed

‎src/Symfony/Component/Lock/Store/MongoDbStore.php

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ class MongoDbStore implements BlockingStoreInterface
7272
* driverOptions: Array of driver options. [used when $mongo is a URI]
7373
*
7474
* When using a URI string:
75-
* the database is determined from the "database" option, otherwise the uri's path is used.
76-
* the collection is determined from the "collection" option, otherwise the uri's "collection" querystring parameter is used.
75+
* The database is determined from the uri's path, otherwise the "database" option is used. To specify an alternate authentication database; "authSource" uriOption or querystring parameter must be used.
76+
* The collection is determined from the uri's "collection" querystring parameter, otherwise the "collection" option is used.
7777
*
7878
* For example: mongodb://myuser:mypass@myhost/mydatabase?collection=mycollection
7979
*
@@ -104,34 +104,20 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
104104
if ($mongo instanceof Collection) {
105105
$this->collection = $mongo;
106106
} elseif ($mongo instanceof Client) {
107-
if (null === $this->options['database']) {
108-
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" option when constructing with a "%s".', __METHOD__, Client::class));
109-
}
110-
if (null === $this->options['collection']) {
111-
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" option when constructing with a "%s".', __METHOD__, Client::class));
112-
}
113-
114107
$this->client = $mongo;
115108
} elseif (\is_string($mongo)) {
116-
if (false === $parsedUrl = parse_url($mongo)) {
117-
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $mongo));
118-
}
119-
$query = [];
120-
if (isset($parsedUrl['query'])) {
121-
parse_str($parsedUrl['query'], $query);
122-
}
123-
$this->options['collection'] = $query['collection'] ?? $this->options['collection'] ?? null;
124-
$this->options['database'] = ltrim($parsedUrl['path'] ?? '', '/') ?: $this->options['database'] ?? null;
109+
$this->uri = $this->skimUri($mongo);
110+
} else {
111+
throw new InvalidArgumentException(sprintf('"%s()" requires "%s" or "%s" or URI as first argument, "%s" given.', __METHOD__, Collection::class, Client::class, get_debug_type($mongo)));
112+
}
113+
114+
if (!($mongo instanceof Collection)) {
125115
if (null === $this->options['database']) {
126-
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option when constructing with a URI.', __METHOD__));
116+
throw new InvalidArgumentException(sprintf('"%s()" requires the "database" in the URI path or option.', __METHOD__));
127117
}
128118
if (null === $this->options['collection']) {
129-
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option when constructing with a URI.', __METHOD__));
119+
throw new InvalidArgumentException(sprintf('"%s()" requires the "collection" in the URI querystring or option.', __METHOD__));
130120
}
131-
132-
$this->uri = $mongo;
133-
} else {
134-
throw new InvalidArgumentException(sprintf('"%s()" requires "%s" or "%s" or URI as first argument, "%s" given.', __METHOD__, Collection::class, Client::class, get_debug_type($mongo)));
135121
}
136122

137123
if ($this->options['gcProbablity'] < 0.0 || $this->options['gcProbablity'] > 1.0) {
@@ -143,6 +129,45 @@ public function __construct($mongo, array $options = [], float $initialTtl = 300
143129
}
144130
}
145131

132+
/**
133+
* Extract default database and collection from given connection URI and remove collection querystring.
134+
*
135+
* Non-standard parameters are removed from the URI to improve libmongoc's re-use of connections.
136+
*
137+
* @see https://www.php.net/manual/en/mongodb.connection-handling.php
138+
*/
139+
private function skimUri(string $uri): string
140+
{
141+
if (false === $parsedUrl = parse_url($uri)) {
142+
throw new InvalidArgumentException(sprintf('The given MongoDB Connection URI "%s" is invalid.', $uri));
143+
}
144+
145+
$query = [];
146+
if (isset($parsedUrl['query'])) {
147+
parse_str($parsedUrl['query'], $query);
148+
}
149+
150+
if (isset($query['collection'])) {
151+
$this->options['collection'] = $query['collection'];
152+
$queryStringPos = strrpos($uri, $parsedUrl['query']);
153+
unset($query['collection']);
154+
$prefix = substr($uri, 0, $queryStringPos);
155+
$newQuery = http_build_query($query, '', '&', PHP_QUERY_RFC3986);
156+
if (empty($newQuery)) {
157+
$prefix = rtrim($prefix, '?');
158+
}
159+
$suffix = substr($uri, $queryStringPos + \strlen($parsedUrl['query']));
160+
$uri = $prefix.$newQuery.$suffix;
161+
}
162+
163+
$pathDb = ltrim($parsedUrl['path'] ?? '', '/') ?: null;
164+
if (null !== $pathDb) {
165+
$this->options['database'] = $pathDb;
166+
}
167+
168+
return $uri;
169+
}
170+
146171
/**
147172
* Creates a TTL index to automatically remove expired locks.
148173
*

‎src/Symfony/Component/Lock/Tests/Store/MongoDbStoreTest.php

Lines changed: 42 additions & 8 deletions
160
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,22 @@ public function provideConstructorArgs()
121121
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock']];
122122
}
123123

124-
public function testDsnPrecedence()
124+
public function testUriPrecedence()
125125
{
126126
$client = self::getMongoClient();
127127

128-
$store = new MongoDbStore('mongodb://localhost/test_dsn?collection=lock_dns', ['collection' => 'lock_option', 'database' => 'test_option']);
129-
$r = new \ReflectionObject($store);
130-
$p = $r->getProperty('options');
131-
$p->setAccessible(true);
132-
$options = $p->getValue($store);
133-
$this->assertSame('lock_dns', $options['collection']);
134-
$this->assertSame('test_dsn', $options['database']);
128+
$store = new MongoDbStore('mongodb://localhost/test_uri?collection=lock_uri', [
129+
'database' => 'test_option',
130+
'collection' => 'lock_option',
131+
]);
132+
$storeReflection = new \ReflectionObject($store);
133+
134+
$optionsProperty = $storeReflection->getProperty('options');
135+
$optionsProperty->setAccessible(true);
136+
$options = $optionsProperty->getValue($store);
137+
138+
$this->assertSame('test_uri', $options['database']);
139+
$this->assertSame('lock_uri', $options['collection']);
135140
}
136141

137142
/**
@@ -154,4 +159,33 @@ public function provideInvalidConstructorArgs()
154159
yield ['mongodb://localhost/test', []];
155
yield ['mongodb://localhost/', []];
156161
}
162+
163+
/**
164+
* @dataProvider provideUriCollectionStripArgs
165+
*/
166+
public function testUriCollectionStrip(string $uri, array $options, string $driverUri)
167+
{
168+
$client = self::getMongoClient();
169+
170+
$store = new MongoDbStore($uri, $options);
171+
$storeReflection = new \ReflectionObject($store);
172+
173+
$uriProperty = $storeReflection->getProperty('uri');
174+
$uriProperty->setAccessible(true);
175+
$uri = $uriProperty->getValue($store);
176+
$this->assertSame($driverUri, $uri);
177+
}
178+
179+
public function provideUriCollectionStripArgs()
180+
{
181+
yield ['mongodb://localhost/?collection=lock', ['database' => 'test'], 'mongodb://localhost/'];
182+
yield ['mongodb://localhost/', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/'];
183+
yield ['mongodb://localhost/test?collection=lock', [], 'mongodb://localhost/test'];
184+
yield ['mongodb://localhost/test', ['collection' => 'lock'], 'mongodb://localhost/test'];
185+
186+
yield ['mongodb://localhost/?collection=lock&replicaSet=repl', ['database' => 'test'], 'mongodb://localhost/?replicaSet=repl'];
187+
yield ['mongodb://localhost/?replicaSet=repl', ['database' => 'test', 'collection' => 'lock'], 'mongodb://localhost/?replicaSet=repl'];
188+
yield ['mongodb://localhost/test?collection=lock&replicaSet=repl', [], 'mongodb://localhost/test?replicaSet=repl'];
189+
yield ['mongodb://localhost/test?replicaSet=repl', ['collection' => 'lock'], 'mongodb://localhost/test?replicaSet=repl'];
190+
}
157191
}

0 commit comments

Comments
 (0)
0