jenkins-bot has submitted this change and it was merged. Change subject: Special:BookSources: Correct validation of ISBNs containing X ......................................................................
Special:BookSources: Correct validation of ISBNs containing X PHP's "equal" (==) operator considers the integer 0 to be equal to the string 'X', and when 'X' is converted to a number, it becomes 0. Neither is desired here. * Fail when an X is encountered while calculating the check digit. (X can only occur as the check digit of an ISBN-10.) * Fixed the check digit comparisons by adding an explicit string cast. * Used the "identical" operator to make it more obvious that no type juggling should take place during the comparisons. * Added some test cases. * Removed an outdated TODO. Bug: 67021 Change-Id: I85f53c41f403a60340e9441757fe66b9764e623c --- M RELEASE-NOTES-1.25 M includes/specials/SpecialBooksources.php A tests/phpunit/includes/specials/SpecialBooksourcesTest.php 3 files changed, 46 insertions(+), 4 deletions(-) Approvals: Umherirrender: Looks good to me, approved jenkins-bot: Verified diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25 index e5a6cd1..e322007 100644 --- a/RELEASE-NOTES-1.25 +++ b/RELEASE-NOTES-1.25 @@ -22,6 +22,8 @@ === Bug fixes in 1.25 === * (bug 71003) No additional code will be generated to try to load CSS-embedded SVG images in Internet Explorer 6 and 7, as they don't support them anyway. +* (bug 67021) On Special:BookSources, corrected validation of ISBNs (both + 10- and 13-digit forms) containing "X". === Action API changes in 1.25 === * (bug 65403) XML tag highlighting is now only performed for formats diff --git a/includes/specials/SpecialBooksources.php b/includes/specials/SpecialBooksources.php index e6750e1..f6b9d33 100644 --- a/includes/specials/SpecialBooksources.php +++ b/includes/specials/SpecialBooksources.php @@ -26,7 +26,6 @@ * The parser creates links to this page when dealing with ISBNs in wikitext * * @author Rob Church <[email protected]> - * @todo Validate ISBNs using the standard check-digit method * @ingroup SpecialPage */ class SpecialBookSources extends SpecialPage { @@ -73,7 +72,9 @@ $sum = 0; if ( strlen( $isbn ) == 13 ) { for ( $i = 0; $i < 12; $i++ ) { - if ( $i % 2 == 0 ) { + if ( $isbn[$i] === 'X' ) { + return false; + } elseif ( $i % 2 == 0 ) { $sum += $isbn[$i]; } else { $sum += 3 * $isbn[$i]; @@ -81,11 +82,14 @@ } $check = ( 10 - ( $sum % 10 ) ) % 10; - if ( $check == $isbn[12] ) { + if ( (string)$check === $isbn[12] ) { return true; } } elseif ( strlen( $isbn ) == 10 ) { for ( $i = 0; $i < 9; $i++ ) { + if ( $isbn[$i] === 'X' ) { + return false; + } $sum += $isbn[$i] * ( $i + 1 ); } @@ -93,7 +97,7 @@ if ( $check == 10 ) { $check = "X"; } - if ( $check == $isbn[9] ) { + if ( (string)$check === $isbn[9] ) { return true; } } diff --git a/tests/phpunit/includes/specials/SpecialBooksourcesTest.php b/tests/phpunit/includes/specials/SpecialBooksourcesTest.php new file mode 100644 index 0000000..d341ccf --- /dev/null +++ b/tests/phpunit/includes/specials/SpecialBooksourcesTest.php @@ -0,0 +1,36 @@ +<?php +class SpecialBooksourcesTest extends MediaWikiTestCase { + public static function provideISBNs() { + return array( + array( '978-0-300-14424-6', true ), + array( '0-14-020652-3', true ), + array( '020652-3', false ), + array( '9781234567897', true ), + array( '1-4133-0454-0', true ), + array( '978-1413304541', true ), + array( '0136091814', true ), + array( '0136091812', false ), + array( '9780136091813', true ), + array( '9780136091817', false ), + array( '123456789X', true ), + + // Bug 67021 + array( '1413304541', false ), + array( '141330454X', false ), + array( '1413304540', true ), + array( '14133X4540', false ), + array( '97814133X4541', false ), + array( '978035642615X', false ), + array( '9781413304541', true ), + array( '9780356426150', true ), + ); + } + + /** + * @covers SpecialBooksources::isValidISBN + * @dataProvider provideISBNs + */ + public function testIsValidISBN( $isbn, $isValid ) { + $this->assertSame( $isValid, SpecialBooksources::isValidISBN( $isbn ) ); + } +} -- To view, visit https://gerrit.wikimedia.org/r/141654 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I85f53c41f403a60340e9441757fe66b9764e623c Gerrit-PatchSet: 7 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: PleaseStand <[email protected]> Gerrit-Reviewer: Addshore <[email protected]> Gerrit-Reviewer: Brion VIBBER <[email protected]> Gerrit-Reviewer: Legoktm <[email protected]> Gerrit-Reviewer: PleaseStand <[email protected]> Gerrit-Reviewer: Umherirrender <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
