bug #39531 [Mailer] Fix parsing Dsn with empty user/password (OskarStark)

fabpot · fabpot · commit 72fb034c772a · 2020-12-17T17:39:20.000+01:00
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [Mailer] Fix parsing Dsn with empty user/password | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | no | License | MIT | Doc PR | no While working on a PR for Notifier that user and password would be parsed as an empty string, which is not wrong, but not expected IMO. Thi `scheme://@symfony.com` and `scheme://:@symfony.com` should be a valid scheme with user and pass `null` Another fix would be to check for `://@` and `://:@` and throw an `InvalidArgumentException` WDYT? The final solution will then be applied to the Notifier DSN in `5.1` Commits ------- 041cb46 [Mailer] Fix parsing Dsn with empty user/password
diff --git a/src/Symfony/Component/Mailer/Tests/Transport/DsnTest.php b/src/Symfony/Component/Mailer/Tests/Transport/DsnTest.php
@@ -52,6 +52,26 @@ public function fromStringProvider(): iterable
             new Dsn('smtp', 'example.com'),
         ];
 
+        yield 'simple dsn including @ sign, but no user/password/token' => [
+            'scheme://@localhost',
+            new Dsn('scheme', 'localhost', null, null),
+        ];
+
+        yield 'simple dsn including : sign and @ sign, but no user/password/token' => [
+            'scheme://:@localhost',
+            new Dsn('scheme', 'localhost', null, null),
+        ];
+
+        yield 'simple dsn including user, : sign and @ sign, but no password' => [
+            'scheme://user1:@localhost',
+            new Dsn('scheme', 'localhost', 'user1', null),
+        ];
+
+        yield 'simple dsn including : sign, password, and @ sign, but no user' => [
+            'scheme://:pass@localhost',
+            new Dsn('scheme', 'localhost', null, 'pass'),
+        ];
+
         yield 'simple smtp with custom port' => [
             'smtp://user1:pass2@example.com:99',
             new Dsn('smtp', 'example.com', 'user1', 'pass2', 99),
diff --git a/src/Symfony/Component/Mailer/Transport/Dsn.php b/src/Symfony/Component/Mailer/Transport/Dsn.php
@@ -49,8 +49,8 @@ public static function fromString(string $dsn): self
             throw new InvalidArgumentException(sprintf('The "%s" mailer DSN must contain a host (use "default" by default).', $dsn));
         }
 
-        $user = isset($parsedDsn['user']) ? urldecode($parsedDsn['user']) : null;
-        $password = isset($parsedDsn['pass']) ? urldecode($parsedDsn['pass']) : null;
+        $user = '' !== ($parsedDsn['user'] ?? '') ? urldecode($parsedDsn['user']) : null;
+        $password = '' !== ($parsedDsn['pass'] ?? '') ? urldecode($parsedDsn['pass']) : null;
         $port = $parsedDsn['port'] ?? null;
         parse_str($parsedDsn['query'] ?? '', $query);
 

Original file line number	Diff line number	Diff line change
`@@ -49,8 +49,8 @@ public static function fromString(string $dsn): self`
`49`	`49`	`throw new InvalidArgumentException(sprintf('The "%s" mailer DSN must contain a host (use "default" by default).', $dsn));`
`50`	`50`	`}`
`51`	`51`
`52`		`- $user = isset($parsedDsn['user']) ? urldecode($parsedDsn['user']) : null;`
`53`		`- $password = isset($parsedDsn['pass']) ? urldecode($parsedDsn['pass']) : null;`
	`52`	`+ $user = '' !== ($parsedDsn['user'] ?? '') ? urldecode($parsedDsn['user']) : null;`
	`53`	`+ $password = '' !== ($parsedDsn['pass'] ?? '') ? urldecode($parsedDsn['pass']) : null;`
`54`	`54`	`$port = $parsedDsn['port'] ?? null;`
`55`	`55`	`parse_str($parsedDsn['query'] ?? '', $query);`
`56`	`56`