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

fix: honor report size in dmarc URI #8

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

shaunwarman
Copy link

Taking a stab at #2

Honor the report size in dmarc URI (e.g. ... rua=some@email.com!30m) before validating the email.

closes #2

@coveralls
Copy link

coveralls commented Apr 7, 2020

Coverage Status

Coverage increased (+0.6%) to 95.181% when pulling 52036d7 on shaunwarman:honor-report-size into 14f4789 on softvu:master.

@niftylettuce
Copy link
Contributor

I think it may need more than just splitting by ! as ! is a valid character in email addresses.

> require('validator').isEmail('word!up@wordup.co')
true

@shaunwarman
Copy link
Author

Oh duh, good catch

@c0bra
Copy link
Contributor

c0bra commented Apr 7, 2020

I no longer work at the company that needed this library, so my brain is probably missing vital relevant bits, but maybe we could do matching like:

const limitRE = /!(?<limit>.+)$/
const limitMatch = email.match(limitRE)

if (limitMatch) {
  const sizeLimit = limitMatch.groups.limit
  email = email.replace(limitRE, '')
}

@niftylettuce
Copy link
Contributor

Thank you @c0bra - @shaunwarman will get this resolved soon and push up so you can release new version to npm once merged. We are using this for @forwardemail

@shaunwarman
Copy link
Author

Sorry for the hold up @c0bra - just got back to this. Thanks for that match!

@c0bra
Copy link
Contributor

c0bra commented Apr 30, 2020

👍

@c0bra c0bra merged commit e8ef5d5 into softvu:master Apr 30, 2020
@c0bra
Copy link
Contributor

c0bra commented Apr 30, 2020

Published just now in v1.2.0

@niftylettuce
Copy link
Contributor

This does not support email addresses with exclamation marks. I've opened PR #10 to fix this @c0bra. Could you please merge it and publish a new version then? Also note that I've filed issue #9 due to size and numbers still not being validated in the !10m portion. We need to add validation to that so it's true to spec as well.

Thank you! 🙏

Ref:

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.

Emails can include /\![\d]+[\s]/
4 participants