Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(validation): allow undersocre in domain name when validating email #542

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

kiaking
Copy link
Member

@kiaking kiaking commented Jun 24, 2024

There was a client who had email address that contains underscore in domain name. And seems like it is valid. So, updating email validation to allow unerscore.

@kiaking kiaking added the enhancement New feature or request label Jun 24, 2024
@kiaking kiaking self-assigned this Jun 24, 2024
@kiaking kiaking requested a review from brc-dd as a code owner June 24, 2024 00:29
Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for sefirot-story ready!

Name Link
🔨 Latest commit 66aa29b
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-story/deploys/6678bde3481fe7000813ff88
😎 Deploy Preview https://deploy-preview-542--sefirot-story.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for sefirot-docs ready!

Name Link
🔨 Latest commit 66aa29b
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-docs/deploys/6678bde3347538000861e337
😎 Deploy Preview https://deploy-preview-542--sefirot-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cuebit
Copy link
Member

cuebit commented Jun 24, 2024

AFAIK underscore does not exist in a valid FQDN. Does the underscore in the client email reside within a different label i.e. foo@_bar.example.com?

@kiaking
Copy link
Member Author

kiaking commented Jun 24, 2024

Seems like underscore is valid as domain, and invalid as a host name. The client address was more like john@abc_d.com

@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2024

Are they even able to use their email? Because according to RFC 5321 (SMTP) it's invalid.

@kiaking
Copy link
Member Author

kiaking commented Jun 24, 2024

Seems like it. Our capitalist is actually communicating through that email address 🫠 So, we kinda have no choice 😆

@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2024

ah ok, seems like there are 6 email standards 🫠 I think we can merge this for now, or just remove this (return value.includes('@')) and let laravel handle it.

@cuebit
Copy link
Member

cuebit commented Jun 24, 2024

@brc-dd looks like you’ll have to fork tldts 😂

@kiaking kiaking merged commit 9b37326 into main Jun 24, 2024
9 checks passed
@kiaking kiaking deleted the validation-allow-underscore branch June 24, 2024 01:25
@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2024

I'll port https://github.com/egulias/EmailValidator to JS some day 😅

@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2024

Ok wait, but that package is also rejecting this 🤣 @kiaking Laravel will throw for this email.

@kiaking
Copy link
Member Author

kiaking commented Jun 24, 2024

Ahhh... @brc-dd Well this fix will apply to non Laravel app so it's OK for now, but damn. We might have to create custom validation for Laravel then 🫠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants