Skip to content

Commit

Permalink
bug #20342 [Form] Fix UrlType transforms valid protocols (ogizanagi)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fabpot committed Oct 30, 2016
2 parents 16b29a1 + 46dd3b9 commit 620ea20
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

0 comments on commit 620ea20

Please sign in to comment.