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

Allow RCPT TO command to fail for some recipients #4816

Closed
wants to merge 2 commits into from

Conversation

@volpav
Copy link
Contributor

@volpav volpav commented Jan 14, 2020

This PR resolves #4792 adding a new option to control whether to allow RCPT TO command to fail for some recipients (and proceed with the remaining valid ones).

In case when all recipients cause failures, curl will abort SMTP conversation and return the error received from to the last RCPT TO command.

@jay jay added the SMTP label Jan 15, 2020
@jay
Copy link
Member

@jay jay commented Jan 15, 2020

Please look into fixing your code for the CI errors.

@bagder
Copy link
Member

@bagder bagder commented Jan 15, 2020

Don't be alarmed by the red CI build(s), that's just due to flaky/bad environments and not because of any flaw in your PR. Ignore them.

@bagder
Copy link
Member

@bagder bagder commented Jan 15, 2020

It would be nice if you now could squash all your commits into a single one (and force-push that) to improve the review experience.

docs/cmdline-opts/mail-rcpt-ignore-invalid.d Outdated Show resolved Hide resolved
docs/cmdline-opts/mail-rcpt-ignore-invalid.d Outdated Show resolved Hide resolved
docs/libcurl/curl_easy_setopt.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_MAIL_RCPT_IGNORE_INVALID.3 Outdated Show resolved Hide resolved
@volpav
Copy link
Contributor Author

@volpav volpav commented Jan 15, 2020

Folks, thanks for the early comments but this PR is not ready for review yet as I'm still dealing with the couple of things. Will send out for review once this is presentable.

@volpav volpav force-pushed the volpav:volpav/smtp-multiple-recipients branch from 089704f to f76cd1e Jan 15, 2020
@volpav volpav changed the title Add an option to ignore invalid SMTP email recipients (and proceed with the valid ones) Allow RCPT TO command to fail for some recipients Jan 15, 2020
@volpav volpav force-pushed the volpav:volpav/smtp-multiple-recipients branch from 8ab7934 to 1c1b7b9 Jan 15, 2020
@volpav volpav requested a review from jay Jan 15, 2020
@volpav volpav force-pushed the volpav:volpav/smtp-multiple-recipients branch from dc7f0b0 to 5450c70 Jan 15, 2020
@volpav volpav marked this pull request as ready for review Jan 15, 2020
@volpav volpav requested a review from bagder Jan 15, 2020
@volpav
Copy link
Contributor Author

@volpav volpav commented Jan 15, 2020

Addressed initial feedback, made a couple minor changes. Some checks are still failing but I'm assuming it's due to the flakiness (as @bagder mentioned). New and existing tests for SMTP seem to be passing.

@bagder
bagder approved these changes Jan 20, 2020
docs/KNOWN_BUGS Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_MAIL_RCPT_ALLLOWFAILS.3 Outdated Show resolved Hide resolved
…n the documentation.
@bagder bagder closed this in 4a4609b Jan 21, 2020
@bagder
Copy link
Member

@bagder bagder commented Jan 21, 2020

Thanks!

@volpav volpav deleted the volpav:volpav/smtp-multiple-recipients branch Jan 23, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.