From 224e70f709a449138b1f99e23a3f699cff950052 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 10 Apr 2014 18:57:38 +0200 Subject: [PATCH] [Validator] Fixed and simplified IsbnValidator --- .../Component/Validator/Constraints/Isbn.php | 3 + .../Validator/Constraints/IsbnValidator.php | 105 +++++++++++------- .../Tests/Constraints/IsbnValidatorTest.php | 8 ++ 3 files changed, 73 insertions(+), 43 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/Isbn.php b/src/Symfony/Component/Validator/Constraints/Isbn.php index 6c978cf3b8..bd61f4b539 100644 --- a/src/Symfony/Component/Validator/Constraints/Isbn.php +++ b/src/Symfony/Component/Validator/Constraints/Isbn.php @@ -42,5 +42,8 @@ class Isbn extends Constraint if (null === $this->isbn10 && null === $this->isbn13) { throw new MissingOptionsException(sprintf('Either option "isbn10" or "isbn13" must be given for constraint "%s".', __CLASS__), array('isbn10', 'isbn13')); } + + $this->isbn10 = (bool) $this->isbn10; + $this->isbn13 = (bool) $this->isbn13; } } diff --git a/src/Symfony/Component/Validator/Constraints/IsbnValidator.php b/src/Symfony/Component/Validator/Constraints/IsbnValidator.php index a000e382ed..0e79c3289a 100644 --- a/src/Symfony/Component/Validator/Constraints/IsbnValidator.php +++ b/src/Symfony/Component/Validator/Constraints/IsbnValidator.php @@ -19,6 +19,7 @@ use Symfony\Component\Validator\Exception\UnexpectedTypeException; * Validates whether the value is a valid ISBN-10 or ISBN-13. * * @author The Whole Life To Learn + * @author Bernhard Schussek * * @see https://en.wikipedia.org/wiki/Isbn */ @@ -37,53 +38,71 @@ class IsbnValidator extends ConstraintValidator throw new UnexpectedTypeException($value, 'string'); } - if (!is_numeric($value)) { - $value = str_replace('-', '', $value); + $value = (string) $value; + $canonical = strtoupper(str_replace('-', '', $value)); + + if ($constraint->isbn10 && $this->isValidIsbn10($canonical)) { + return; } - $validation = 0; - $value = strtoupper($value); - $valueLength = strlen($value); + if ($constraint->isbn13 && $this->isValidIsbn13($canonical)) { + return; + } - if (10 === $valueLength && null !== $constraint->isbn10) { - for ($i = 0; $i < 10; $i++) { - if ($value[$i] == 'X') { - $validation += 10 * intval(10 - $i); - } else { - $validation += intval($value[$i]) * intval(10 - $i); - } - } - - if ($validation % 11 != 0) { - if (null !== $constraint->isbn13) { - $this->context->addViolation($constraint->bothIsbnMessage); - } else { - $this->context->addViolation($constraint->isbn10Message); - } - } - } elseif (13 === $valueLength && null !== $constraint->isbn13) { - for ($i = 0; $i < 13; $i += 2) { - $validation += intval($value[$i]); - } - for ($i = 1; $i < 12; $i += 2) { - $validation += intval($value[$i]) * 3; - } - - if ($validation % 10 != 0) { - if (null !== $constraint->isbn10) { - $this->context->addViolation($constraint->bothIsbnMessage); - } else { - $this->context->addViolation($constraint->isbn13Message); - } - } + if ($constraint->isbn10 && $constraint->isbn13) { + $this->context->addViolation($constraint->bothIsbnMessage, array( + '{{ value }}' => $value, + )); + } elseif ($constraint->isbn10) { + $this->context->addViolation($constraint->isbn10Message, array( + '{{ value }}' => $value, + )); } else { - if (null !== $constraint->isbn10 && null !== $constraint->isbn13) { - $this->context->addViolation($constraint->bothIsbnMessage); - } elseif (null !== $constraint->isbn10) { - $this->context->addViolation($constraint->isbn10Message); - } else { - $this->context->addViolation($constraint->isbn13Message); - } + $this->context->addViolation($constraint->isbn13Message, array( + '{{ value }}' => $value, + )); } } + + private function isValidIsbn10($isbn) + { + if (10 !== strlen($isbn)) { + return false; + } + + $checkSum = 0; + + for ($i = 0; $i < 10; ++$i) { + if ('X' === $isbn{$i}) { + $digit = 10; + } elseif (ctype_digit($isbn{$i})) { + $digit = $isbn{$i}; + } else { + return false; + } + + $checkSum += $digit * intval(10 - $i); + } + + return 0 === $checkSum % 11; + } + + private function isValidIsbn13($isbn) + { + if (13 !== strlen($isbn) || !ctype_digit($isbn)) { + return false; + } + + $checkSum = 0; + + for ($i = 0; $i < 13; $i += 2) { + $checkSum += $isbn{$i}; + } + + for ($i = 1; $i < 12; $i += 2) { + $checkSum += $isbn{$i} * 3; + } + + return 0 === $checkSum % 10; + } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php index 7f0859b999..049a9ac3ee 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php @@ -58,6 +58,10 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase array('0-4X19-92611'), array('0_45122_5244'), array('2870#971#648'), + array('1A34567890'), + // chr(1) evaluates to 0 + // 2070546810 is valid + array('2'.chr(1).'70546810'), ); } @@ -92,6 +96,10 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase array('980-0474292319'), array('978_0451225245'), array('978#0471292319'), + array('978-272C442282'), + // chr(1) evaluates to 0 + // 978-2070546817 is valid + array('978-2'.chr(1).'70546817'), ); }