bug #20342 [Form] Fix UrlType transforms valid protocols (ogizanagi)

fabpot · fabpot · commit 620ea20f4967 · 2016-10-30T08:33:18.000-07:00
This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #20342). Discussion ---------- [Form] Fix UrlType transforms valid protocols | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A According to https://tools.ietf.org/html/rfc3986#section-3.1: <details> <summary>`scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )`</summary> > Each URI begins with a scheme name that refers to a specification for assigning identifiers within that scheme. As such, the URI syntax is a federated and extensible naming system wherein each scheme's specification may further restrict the syntax and semantics of identifiers using that scheme. > Scheme names consist of a sequence of characters beginning with a letter and followed by any combination of letters, digits, plus ("+"), period ("."), or hyphen ("-"). Although schemes are case- insensitive, the canonical form is lowercase and documents that specify schemes must do so with lowercase letters. An implementation should accept uppercase letters as equivalent to lowercase in scheme names (e.g., allow "HTTP" as well as "http") for the sake of robustness but should only produce lowercase scheme names for consistency. </details> ~~Fixing the regex to add missing chars could solve the issue. However according to the RFC, we should not allow underscores. But `\w+` permits it (removing it would be a minor BC break anyway).~~ ~~IMHO, we should not care in this listener if the scheme is valid or not (a validator should be used instead), so I'd suggest to simply check if a scheme is provided or not.~~ I'm not using `parse_url($string, PHP_URL_SCHEME)` because `http:/symfony.com` or `http:symfony.com` is considered valid as containing a scheme. Actually, I changed my mind about previous fix (28f816a) and went back to the regex, in order to have strings like `symfony.com?uri=http://example.com` properly transformed to `symfony.com?uri=http://example.com` Commits ------- 46dd3b9 [Form] Fix UrlType transforms valid protocols
diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/FixUrlProtocolListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/FixUrlProtocolListener.php
@@ -38,7 +38,7 @@ public function onSubmit(FormEvent $event)
     {
         $data = $event->getData();
 
-        if ($this->defaultProtocol && $data && !preg_match('~^\w+://~', $data)) {
+        if ($this->defaultProtocol && $data && !preg_match('~^[\w+.-]+://~', $data)) {
             $event->setData($this->defaultProtocol.'://'.$data);
         }
     }
diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/FixUrlProtocolListenerTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/FixUrlProtocolListenerTest.php
@@ -40,15 +40,28 @@ public function testSkipKnownUrl()
         $this->assertEquals('http://www.symfony.com', $event->getData());
     }
 
-    public function testSkipOtherProtocol()
+    public function provideUrlsWithSupportedProtocols()
+    {
+        return array(
+            array('ftp://www.symfony.com'),
+            array('chrome-extension://foo'),
+            array('h323://foo'),
+            array('iris.beep://foo'),
+            array('foo+bar://foo'),
+        );
+    }
+
+    /**
+     * @dataProvider provideUrlsWithSupportedProtocols
+     */
+    public function testSkipOtherProtocol($url)
     {
-        $data = 'ftp://www.symfony.com';
         $form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
-        $event = new FormEvent($form, $data);
+        $event = new FormEvent($form, $url);
 
         $filter = new FixUrlProtocolListener('http');
         $filter->onSubmit($event);
 
-        $this->assertEquals('ftp://www.symfony.com', $event->getData());
+        $this->assertEquals($url, $event->getData());
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public function onSubmit(FormEvent $event)`
`38`	`38`	`{`
`39`	`39`	`$data = $event->getData();`
`40`	`40`
`41`		`- if ($this->defaultProtocol && $data && !preg_match('~^\w+://~', $data)) {`
	`41`	`+ if ($this->defaultProtocol && $data && !preg_match('~^[\w+.-]+://~', $data)) {`
`42`	`42`	`$event->setData($this->defaultProtocol.'://'.$data);`
`43`	`43`	`}`
`44`	`44`	`}`