Skip to content

Fix email validation to prevent silent truncation with newline characters#107

Merged
afair merged 1 commit intoafair:masterfrom
koshilife:fix/reject-newline-characters
Oct 20, 2025
Merged

Fix email validation to prevent silent truncation with newline characters#107
afair merged 1 commit intoafair:masterfrom
koshilife:fix/reject-newline-characters

Conversation

@koshilife
Copy link
Copy Markdown
Contributor

@koshilife koshilife commented Oct 17, 2025

Summary

Fixed a data integrity issue where email addresses containing newline characters (\n, \r\n) were incorrectly validated as true due to silent truncation in the split_local_host method.

Problem

The original regex /(.+)@(.+)/ would match only up to the first \n character, causing silent data loss:

# Before fix
"user@foo.com\nother@bar.com"  Local: "user", Host: "foo.com"  Valid 
"a@example.com\nb@example.com"  Local: "a", Host: "example.com"  Valid 

This created a data integrity risk where invalid input appeared as valid email addresses.

Solution

Changed the regex in split_local_host method to use anchors:

# Before
if (lh = email.match(/(.+)@(.+)/))

# After  
if (lh = email.match(/\A(.+)@(.+)\z/))

The \A and \z anchors ensure the entire string must match the email pattern. If newline characters are present, the regex fails to match, and the method returns [email, ""], causing validation to fail appropriately.

Changes

  • Core fix: Modified regex in Address.split_local_host method (1 line change)
  • Test coverage: Added comprehensive test cases for newline character scenarios

Test Cases Added

def test_newline_characters
  assert !EmailAddress.valid?("user@foo.com\nother@bar.com") # LF truncation
  assert !EmailAddress.valid?("user@foo.com\r\nother@bar.com") # CRLF truncation  
  assert EmailAddress.valid?("\nuser@foo.com") # valid because strip processing removes \n
  assert EmailAddress.valid?("user@foo.com\r\n") # valid because strip processing removes \n
end

Verification

Before fix:

EmailAddress.valid?("user@foo.com\nother@bar.com") # => true ❌

After fix:

EmailAddress.valid?("user@foo.com\nother@bar.com") # => false ✅

Backward Compatibility

  • ✅ All existing functionality preserved
  • ✅ No breaking changes to valid email address handling
  • ✅ Only affects invalid input that was previously incorrectly accepted

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Note: This fix addresses a data integrity issue where malformed input was silently truncated and incorrectly validated. The change ensures that only complete, well-formed email addresses are accepted as valid.

@afair
Copy link
Copy Markdown
Owner

afair commented Oct 20, 2025

Thank you! The failing build is my fault, and I have the fix for that I will apply first. I'll merge this when it is ready

@koshilife
Copy link
Copy Markdown
Contributor Author

@afair Thanks for reviewing quickly!

@koshilife
Copy link
Copy Markdown
Contributor Author

I re-checked it myself, and the test failed. This PR may not be good enough yet.

- Add newline character validation in valid? method to reject emails containing \r or \n
- Update test_newline_characters with comprehensive test cases
- Improve test comments to clarify strip processing behavior for edge cases
- Leading/trailing newlines are valid due to strip processing during initialization
- Embedded newlines remain invalid as expected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@koshilife koshilife force-pushed the fix/reject-newline-characters branch from 6cd923b to 9bb2d80 Compare October 20, 2025 15:04
@koshilife
Copy link
Copy Markdown
Contributor Author

I've pulled the changes from ffb5a63, re-run the tests locally, and confirmed the fix. Please review it whenever you have a moment.

@afair afair merged commit fe0c636 into afair:master Oct 20, 2025
3 checks passed
@koshilife koshilife deleted the fix/reject-newline-characters branch October 20, 2025 23:43
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.

2 participants