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

SRS return path issues - causes whole email to fail to process #90

Closed
eileenmcnaughton opened this issue Sep 12, 2023 · 4 comments
Closed

Comments

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Sep 12, 2023

We have a new issue with mail components - the returnPath is capabil of being an invalid email in line with the Sender Rewriting scheme

e.g

Return-Path:  <SRS0=z9mA=EY=donate-long-name.example.org=b.51907682.51575594.57daed525aaca713@example.org>

However, this doesn't pass !filter_var( $value->email, FILTER_VALIDATE_EMAIL ) )

image

I'm playing catch up here a bit because I don't know why this wasn't raised in the context of #87 (our ref is https://lab.civicrm.org/dev/core/-/issues/3586)

However, the problem is that an SRS path - causes the whole email to fail to load - which is the problem

We have a proposed fix which is just to set NULL rather than throw an exception above in these cases - perhaps we can detect SRS in the string and just not return anything in that case?

@eileenmcnaughton eileenmcnaughton changed the title Over-long return path issues SRS return path issues - causes whole email to fail to process Sep 12, 2023
@derickr
Copy link
Member

derickr commented Sep 13, 2023

Hi,

I'll have a look. I don't think there is anything wrong with this line, so moving to filter_var() with FILTER_VALIDATE_EMAIL was probably the wrong choice.

Do you have a set of emails that are now broken, and also your original broken one from the issue in civic that you linked to? We can then add these as test cases.

cheers,
Derick

@derickr
Copy link
Member

derickr commented Sep 13, 2023

I had a little dive into RFCs, and filter_var() does do this correctly. Local parts are limited to 64 octets, and your example "email address" has more.

I do think it is important that we validate this, and that we don't just return null when it isn't valid. I also believe that when parsing we should be more lenient in what we accept.

My suggestion would be to use the regex from PHP's filter_var() and extend the length limits. The PR at #91 does this, and extends it from 64 to 124.

@derickr
Copy link
Member

derickr commented Sep 14, 2023

I've merged this, and also released 1.9.6.

@eileenmcnaughton
Copy link
Contributor Author

Thanks for being so helpful @derickr

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

No branches or pull requests

2 participants