Skip to content

test(email): expand is_idn_email coverage for the address-literal branch - #2698

Open
vtushar06 wants to merge 1 commit into
sourcemeta:mainfrom
vtushar06:idn-email-address-literal-coverage
Open

test(email): expand is_idn_email coverage for the address-literal branch#2698
vtushar06 wants to merge 1 commit into
sourcemeta:mainfrom
vtushar06:idn-email-address-literal-coverage

Conversation

@vtushar06

@vtushar06 vtushar06 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

I added 39 tests to idn_email_test.cc covering the address-literal branch of is_idn_email. Core already passes all of them, so this is coverage, not a fix.

What is new: the branch had almost nothing behind it. is_general_address_literal had no tests at all, and is_snum's leading-zero behaviour - the thing that separates it from the RFC 3986 dec-octet behind is_ipv4 - was not pinned anywhere, despite being called out in a comment.

  • Snum (13 tests): leading zeros in each position, padded zero octets, the 255 maximum, one over, four digits, wrong octet counts, and the dot-placement errors.
  • General-address-literal (9 tests): the tag:content form, the shortest valid form, a leading-hyphen tag, a trailing-hyphen tag, empty tag and empty content, and the dcontent exclusions for SP and [.
  • IPv6 literals (5 tests): the fully compressed form, an uppercase IPV6: tag, IPv6v4-comp, IPv6v4-full, and the empty payload.
  • Literal shape (5 tests): empty, unterminated, doubled brackets, a label after the closing bracket, and a trailing space inside.
  • The @ delimiter (2 tests): U+FF20 and U+FE6B, neither of which is the delimiter.
  • Local-part code points (4 tests): a C1 control, two noncharacters, and a supplementary private-use code point - all admitted by UTF8-non-ascii with no property filter.
  • Quoted-string structure (3 tests): unterminated, text after the closing DQUOTE, and a quoted-pair escaping the DQUOTE.

How I checked the expected values: every input was run against the built is_idn_email and is_email before being written, and each expectation is the RFC verdict rather than the observed one - the two agreed on all 43 candidates I probed. sourcemeta_core_email_unit is 393 passed / 0 failed with these added.

Two of my initial expectations were wrong and Core was right, which is worth recording. user@[-tag:abc] is valid, because Ldh-str = *( ALPHA / DIGIT / "-" ) Let-dig only constrains the last character, unlike sub-domain = Let-dig [Ldh-str]. And user@[IPv4:192.0.2.1] is a syntactically valid General-address-literal, since IPv4 is not a tag RFC 5321 defines. Both are now tests with the reasoning in the comment.

RFC references:

Review in cubic

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 01:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@augmentcode

augmentcode Bot commented Jul 31, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Expands test coverage for the RFC 5321 address-literal branch in sourcemeta::core::is_idn_email, with matching assertions for is_email where applicable.

Changes:

  • Adds a focused Snum (IPv4 address-literal) suite, including leading-zero behavior, range bounds (0–255), digit-count limits, and dot-placement errors.
  • Adds General-address-literal tests covering minimal forms, hyphen rules in Standardized-tag, and invalid empty tag/content cases.
  • Adds dcontent exclusion tests (e.g., SP and [) within general address literal content.
  • Adds IPv6 address-literal tests for fully compressed form, case-insensitive IPv6: tag, and IPv6v4 (comp/full) forms, plus an empty-payload invalid case.
  • Adds “shape” negative tests for malformed literals (empty, unterminated, doubled brackets, trailing text/space).
  • Adds tests ensuring only ASCII U+0040 acts as the mailbox delimiter (rejects fullwidth/small commercial at code points).
  • Adds IDN-local-part UTF-8 edge cases (C1 control, noncharacters, supplementary private-use), valid for is_idn_email but invalid for is_email.
  • Adds quoted-string structure tests (unterminated, trailing local-part text, and escaped DQUOTE via quoted-pair).

Notes: This PR is test-only (coverage expansion); expectations are pinned to RFC-defined behavior and were cross-checked against current Core behavior.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

EXPECT_FALSE(sourcemeta::core::is_email("\xf3\xb0\x80\x80@example.com"));
}

// --- Quoted-string structure -----------------------------------------------

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove section comments?

@jviotti jviotti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but there are still lots of section comments!

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.

3 participants