Skip to content

restrict ISBN group separators to a dash or space - #439

Open
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:isbn-separator-strict
Open

sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:isbn-separator-strict

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

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 \s in 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.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude Code (Anthropic) was used to survey the validators, draft the change and this description; I reviewed the fix and the fixtures and ran the build locally.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant