Skip to content

Reject non-ASCII whitespace in idn-email and strengthen email format tests#1268

Open
el-psy-kongroo-d wants to merge 1 commit into
networknt:masterfrom
el-psy-kongroo-d:followup/1267-email-idn-whitespace
Open

Reject non-ASCII whitespace in idn-email and strengthen email format tests#1268
el-psy-kongroo-d wants to merge 1 commit into
networknt:masterfrom
el-psy-kongroo-d:followup/1267-email-idn-whitespace

Conversation

@el-psy-kongroo-d

Copy link
Copy Markdown
Contributor

Follow-up to #1267, applying the review suggestions from that PR.

Changes

  • Extract the non-ASCII-whitespace check into Strings.containsNonAsciiWhitespace
    and reuse it from both EmailFormat and IdnEmailFormat (instead of a private
    helper in a single class).
  • idn-email: IdnEmailFormat shares EmailFormat's delegation and had the
    same bug — a leading U+00A0 was accepted. It now rejects non-ASCII whitespace
    too. Adds a new IdnEmailFormatTest (which also asserts a non-ASCII letter
    is still accepted, since idn-email allows those) and a license header to
    IdnEmailFormat, which previously had none.
  • Strengthen EmailFormatTest: assert a quoted local part with an ASCII
    space ("joe bloggs"@example.com) stays valid, and generalize the negative
    case over U+00A0, U+2003 and U+3000.

Scope

Deliberately narrow: only non-ASCII whitespace is rejected. Non-ASCII
letters remain valid (important for idn-email). Zero-width format characters
such as U+200B and U+FEFF (Unicode category Cf) are out of scope and unchanged
from previous behavior.

Notes on license headers

  • IdnEmailFormat had no header; the added one matches its sibling
    EmailFormat.java (Copyright (c) 2016 Network New Technologies Inc.).
  • EmailFormatTest / IdnEmailFormatTest use Copyright (c) 2025 the original author or authors, matching every other test file in the format package
    (TimeFormatTest, UriFormatTest, ...). That is why the Reject non-ASCII whitespace in email format #1267 header nit was
    not switched to the main-source form — it would diverge from the test siblings.

Testing

Full test suite passes (8477 tests, 0 failures).

…tests

Follow-up to networknt#1267 applying the review suggestions:

- Extract the non-ASCII-whitespace check to Strings.containsNonAsciiWhitespace
  and reuse it from both EmailFormat and IdnEmailFormat.
- idn-email shares EmailFormat's delegation and had the same bug (a leading
  U+00A0 was accepted); it now rejects non-ASCII whitespace too. Adds a license
  header to IdnEmailFormat (it had none) and a new IdnEmailFormatTest that also
  verifies a non-ASCII letter is still accepted.
- Strengthen EmailFormatTest: assert a quoted local part with an ASCII space
  ("joe bloggs"@example.com) stays valid, and generalize the negative case over
  U+00A0, U+2003 and U+3000.

The change stays deliberately narrow (whitespace only); non-ASCII letters are
still accepted in idn-email. Zero-width format characters (e.g. U+200B, U+FEFF)
are out of scope and unchanged.
@el-psy-kongroo-d

Copy link
Copy Markdown
Contributor Author

Thanks for the quick review and merge on #1267, @stevehu 🙏

This follow-up applies the suggestions from that review:

  • Extracted the check into Strings.containsNonAsciiWhitespace and applied it to idn-email as well (with a test that a non-ASCII letter still validates).
  • Added the quoted-local-part positive test and generalized the negative case to U+2003 / U+3000.

Two judgment calls I flagged in the description: I kept EmailFormatTest's the original author or authors header (all format test files use it, so changing only this one would diverge from its siblings), and kept the scope to whitespace only. Happy to adjust either if you'd prefer.

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