Repository navigation
restrict ISBN group separators to a dash or space - #439
Open
sahvx655-wq wants to merge 1 commit into
Open
sahvx655-wq wants to merge 1 commit into
sahvx655-wq wants to merge 1 commit into
Conversation
The SEP fragment shared by ISBN10_REGEX and ISBN13_REGEX matched any \s character, so tab, line feed, carriage return, vertical tab and form feed were accepted between groups although the Javadoc only allows a dash or a space. Narrow it to those two characters and add the control-character cases to the invalid-format fixtures for both lengths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ISBNValidator's Javadoc says the ISBN-10 and ISBN-13 groups are separated by a dash or a space, but the SEP fragment shared by ISBN10_REGEX and ISBN13_REGEX is
(?:\-|\s), and\sin a Java regex also covers tab, line feed, carriage return, vertical tab and form feed. I found it while sweeping the routines package for separator patterns: isValidISBN10("1\n930110\n99\n5") and isValidISBN13("978\t1\t930110\t99\t1") both return true, and isValid and validate agree with them, so a value with embedded line breaks passes as a correctly formatted ISBN. The check digit does not catch it because the separators are stripped before it runs. Any caller that stores, logs or echoes the raw value on the strength of isValid ends up with a multi-line token the validator vouched for.Narrow SEP to a character class of the two documented separators, which corrects both regexes in one place and lines up with the existing "Invalid Separator" fixtures for '.', '=' and '_'. The regex is the only layer that can do this: CodeValidator trims only the ends of the input and the check digit never sees the separators, so nothing further down the chain has the information. The invalid-format fixtures gain the five control-character separators for ISBN-10 and ISBN-13; they fail on master and pass with the change, and the default build with -Ddoclint=all is green.
mvn; that'smvnon the command line by itself.