Skip to content

Commit

Permalink
bug #12235 [Validator] Fixed Regex::getHtmlPattern() to work with com…
Browse files Browse the repository at this point in the history 
…plex and negated patterns (webmozart)

This PR was merged into the 2.3 branch.

Discussion
----------

[Validator] Fixed Regex::getHtmlPattern() to work with complex and negated patterns

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #5307
| License       | MIT
| Doc PR        | -

According to my own testing, this should fix the generation of HTML patterns when `Regex::$match` is set to false. Additionally, patterns containing pipes (or statements) are fixed. See the test cases for examples.

Commits
-------

bf006f5 [Validator] Fixed Regex::getHtmlPattern() to work with complex and negated patterns
  • Loading branch information
@fabpot
fabpot committed Nov 2, 2014
2 parents eb4b20f + bf006f5 commit 9ea4296
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 96 deletions.
61 changes: 31 additions & 30 deletions src/Symfony/Component/Validator/Constraints/Regex.php
View file
Expand Up @@ -44,23 +44,6 @@ public function getRequiredOptions()
return array('pattern');
}

/**
* Returns htmlPattern if exists or pattern is convertible.
*
* @return string|null
*/
public function getHtmlPattern()
{
// If htmlPattern is specified, use it
if (null !== $this->htmlPattern) {
return empty($this->htmlPattern)
? null
: $this->htmlPattern;
}

return $this->getNonDelimitedPattern();
}

/**
* Converts the htmlPattern to a suitable format for HTML5 pattern.
* Example: /^[a-z]+$/ would be converted to [a-z]+
Expand All @@ -69,29 +52,47 @@ public function getHtmlPattern()
* Pattern is also ignored if match=false since the pattern should
* then be reversed before application.
*
* @todo reverse pattern in case match=false as per issue #5307
*
* @link http://dev.w3.org/html5/spec/single-page.html#the-pattern-attribute
*
* @return string|null
*/
private function getNonDelimitedPattern()
public function getHtmlPattern()
{
// If match = false, pattern should not be added to HTML5 validation
if (!$this->match) {
// If htmlPattern is specified, use it
if (null !== $this->htmlPattern) {
return empty($this->htmlPattern)
? null
: $this->htmlPattern;
}

// Quit if delimiters not at very beginning/end (e.g. when options are passed)
if ($this->pattern[0] !== $this->pattern[strlen($this->pattern) - 1]) {
return;
}

if (preg_match('/^(.)(\^?)(.*?)(\$?)\1$/', $this->pattern, $matches)) {
$delimiter = $matches[1];
$start = empty($matches[2]) ? '.*' : '';
$pattern = $matches[3];
$end = empty($matches[4]) ? '.*' : '';
$delimiter = $this->pattern[0];

// Unescape the delimiter
$pattern = str_replace('\\'.$delimiter, $delimiter, substr($this->pattern, 1, -1));

// Unescape the delimiter in pattern
$pattern = str_replace('\\'.$delimiter, $delimiter, $pattern);
// If the pattern is inverted, we can simply wrap it in
// ((?!pattern).)*
if (!$this->match) {
return '((?!'.$pattern.').)*';
}

return $start.$pattern.$end;
// If the pattern contains an or statement, wrap the pattern in
// .*(pattern).* and quit. Otherwise we'd need to parse the pattern
if (false !== strpos($pattern, '|')) {
return '.*('.$pattern.').*';
}

// Trim leading ^, otherwise prepend .*
$pattern = '^' === $pattern[0] ? substr($pattern, 1) : '.*'.$pattern;

// Trim trailing $, otherwise append .*
$pattern = '$' === $pattern[strlen($pattern) - 1] ? substr($pattern, 0, -1) : $pattern.'.*';

return $pattern;
}
}
87 changes: 87 additions & 0 deletions src/Symfony/Component/Validator/Tests/Constraints/RegexTest.php
View file
@@ -0,0 +1,87 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Constraints;

use Symfony\Component\Validator\Constraints\Regex;

/**
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class RegexTest extends \PHPUnit_Framework_TestCase
{
public function testConstraintGetDefaultOption()
{
$constraint = new Regex('/^[0-9]+$/');

$this->assertSame('/^[0-9]+$/', $constraint->pattern);
}

public function provideHtmlPatterns()
{
return array(
// HTML5 wraps the pattern in ^(?:pattern)$
array('/^[0-9]+$/', '[0-9]+'),
array('/[0-9]+$/', '.*[0-9]+'),
array('/^[0-9]+/', '[0-9]+.*'),
array('/[0-9]+/', '.*[0-9]+.*'),
// We need a smart way to allow matching of patterns that contain
// ^ and $ at various sub-clauses of an or-clause
// .*(pattern).* seems to work correctly
array('/[0-9]$|[a-z]+/', '.*([0-9]$|[a-z]+).*'),
array('/[0-9]$|^[a-z]+/', '.*([0-9]$|^[a-z]+).*'),
array('/^[0-9]|[a-z]+$/', '.*(^[0-9]|[a-z]+$).*'),
// Unescape escaped delimiters
array('/^[0-9]+\/$/', '[0-9]+/'),
array('#^[0-9]+\#$#', '[0-9]+#'),
// Cannot be converted
array('/^[0-9]+$/i', null),

// Inverse matches are simple, just wrap in
// ((?!pattern).)*
array('/^[0-9]+$/', '((?!^[0-9]+$).)*', false),
array('/[0-9]+$/', '((?![0-9]+$).)*', false),
array('/^[0-9]+/', '((?!^[0-9]+).)*', false),
array('/[0-9]+/', '((?![0-9]+).)*', false),
array('/[0-9]$|[a-z]+/', '((?![0-9]$|[a-z]+).)*', false),
array('/[0-9]$|^[a-z]+/', '((?![0-9]$|^[a-z]+).)*', false),
array('/^[0-9]|[a-z]+$/', '((?!^[0-9]|[a-z]+$).)*', false),
array('/^[0-9]+\/$/', '((?!^[0-9]+/$).)*', false),
array('#^[0-9]+\#$#', '((?!^[0-9]+#$).)*', false),
array('/^[0-9]+$/i', null, false),
);
}

/**
* @dataProvider provideHtmlPatterns
*/
public function testGetHtmlPattern($pattern, $htmlPattern, $match = true)
{
$constraint = new Regex(array(
'pattern' => $pattern,
'match' => $match,
));

$this->assertSame($pattern, $constraint->pattern);
$this->assertSame($htmlPattern, $constraint->getHtmlPattern());
}

public function testGetCustomHtmlPattern()
{
$constraint = new Regex(array(
'pattern' => '((?![0-9]$|[a-z]+).)*',
'htmlPattern' => 'foobar',
));

$this->assertSame('((?![0-9]$|[a-z]+).)*', $constraint->pattern);
$this->assertSame('foobar', $constraint->getHtmlPattern());
}
}
66 changes: 0 additions & 66 deletions src/Symfony/Component/Validator/Tests/Constraints/RegexValidatorTest.php
View file
Expand Up @@ -88,70 +88,4 @@ public function getInvalidValues()
array('090foo'),
);
}

public function testConstraintGetDefaultOption()
{
$constraint = new Regex(array(
'pattern' => '/^[0-9]+$/',
));

$this->assertEquals('pattern', $constraint->getDefaultOption());
}

public function testHtmlPatternEscaping()
{
$constraint = new Regex(array(
'pattern' => '/^[0-9]+\/$/',
));

$this->assertEquals('[0-9]+/', $constraint->getHtmlPattern());

$constraint = new Regex(array(
'pattern' => '#^[0-9]+\#$#',
));

$this->assertEquals('[0-9]+#', $constraint->getHtmlPattern());
}

public function testHtmlPattern()
{
// Specified htmlPattern
$constraint = new Regex(array(
'pattern' => '/^[a-z]+$/i',
'htmlPattern' => '[a-zA-Z]+',
));
$this->assertEquals('[a-zA-Z]+', $constraint->getHtmlPattern());

// Disabled htmlPattern
$constraint = new Regex(array(
'pattern' => '/^[a-z]+$/i',
'htmlPattern' => false,
));
$this->assertNull($constraint->getHtmlPattern());

// Cannot be converted
$constraint = new Regex(array(
'pattern' => '/^[a-z]+$/i',
));
$this->assertNull($constraint->getHtmlPattern());

// Automatically converted
$constraint = new Regex(array(
'pattern' => '/^[a-z]+$/',
));
$this->assertEquals('[a-z]+', $constraint->getHtmlPattern());

// Automatically converted, adds .*
$constraint = new Regex(array(
'pattern' => '/[a-z]+/',
));
$this->assertEquals('.*[a-z]+.*', $constraint->getHtmlPattern());

// Dropped because of match=false
$constraint = new Regex(array(
'pattern' => '/[a-z]+/',
'match' => false,
));
$this->assertNull($constraint->getHtmlPattern());
}
}

0 comments on commit 9ea4296

Please sign in to comment.