globalbrain / sefirot

Global Brain Design System.
https://sefirot.globalbrains.com
MIT License
151 stars 12 forks source link

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

Closed kiaking closed 4 weeks ago

kiaking commented 4 weeks ago

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.

netlify[bot] commented 4 weeks ago

Deploy Preview for sefirot-story ready!

Name Link
Latest commit 66aa29ba8f85b91d992410d76de53e23b1207d30
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.

netlify[bot] commented 4 weeks ago

Deploy Preview for sefirot-docs ready!

Name Link
Latest commit 66aa29ba8f85b91d992410d76de53e23b1207d30
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 commented 4 weeks ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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

kiaking commented 4 weeks ago

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

brc-dd commented 4 weeks ago

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 commented 4 weeks ago

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

brc-dd commented 4 weeks ago

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

brc-dd commented 4 weeks ago

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

kiaking commented 4 weeks ago

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 🫠