Skip to content

allow suffix after a mail address for RFC 3461#16643

Closed
my-name-is-already-taken wants to merge 8 commits intocurl:masterfrom
my-name-is-already-taken:rfc3461
Closed

allow suffix after a mail address for RFC 3461#16643
my-name-is-already-taken wants to merge 8 commits intocurl:masterfrom
my-name-is-already-taken:rfc3461

Conversation

@my-name-is-already-taken
Copy link
Contributor

This change makes it possible to Append a DSN specification behind the MAIL and RCPT mail addresses in the smtp protocol.

Currently, curl checks if these addresses start with a '<' or end with a '>' and remove these characters, before adding them when issuing the actual command. So when setting CURLOPT_MAIL_RCPT to <foo@bar.com> NOTIFY=FAILURE, the actual address sent to the server becomes <foo@bar.com> NOTIFY=FAILURE>.

This proposal changes the behavior in the following way: If the address starts with a '<' character, we search the address for the last '>' instead of only checking the last character. Everything that comes behind this bracket is then kept as a suffix in the address.

See also #8232.

@github-actions github-actions bot added the SMTP label Mar 10, 2025
@testclutch
Copy link

Analysis of PR #16643 at fafb6ae0:

Test 1515 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

@bagder
Copy link
Member

bagder commented Mar 10, 2025

This looks like a good idea to support. I would just like to ask for a little more here:

  • documentation - this needs to be described
  • a test case or two that verify that this behavior works as intended and documented

@my-name-is-already-taken
Copy link
Contributor Author

I have fixed an error and added a testcase as well as some documentation.

I had difficulties writing the test, so I would appreciate it if someone checked whether I've done it correcty.

Please let me know if something else is missing.

@my-name-is-already-taken my-name-is-already-taken changed the title allow suffix behind a Mail Address for RFC 3461 allow suffix after a mail address for RFC 3461 Mar 10, 2025
@vszakats
Copy link
Member

vszakats commented Mar 10, 2025

Spellcheck says:

Misspelled words:
<htmlcontent> docs/libcurl/opts/CURLOPT_MAIL_RCPT.md: html>body>p
--------------------------------------------------------------------------------
DSN
--------------------------------------------------------------------------------

This should silence it:

--- i/.github/scripts/spellcheck.words
+++ w/.github/scripts/spellcheck.words
@@ -203,6 +203,7 @@ DoT
 doxygen
 drftpd
 dsa
+DSN
 Dudka
 Dymond
 dynbuf

@github-actions github-actions bot added the CI Continuous Integration label Mar 10, 2025
@vszakats vszakats dismissed their stale review March 12, 2025 10:37

Thanks, all fixed from my end. Can't comment much on the protocol side.

@bagder bagder added needs-info feature-window A merge of this requires an open feature window labels Jul 9, 2025
@bagder
Copy link
Member

bagder commented Jul 9, 2025

This branch has conflicts that must be resolved

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs merge conflicts resolved

@my-name-is-already-taken
Copy link
Contributor Author

I have resolved the conflict and renamed test3211 to test3215.

Copy link
Member

@vszakats vszakats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bagder bagder closed this in 450c00f Jul 30, 2025
@bagder
Copy link
Member

bagder commented Jul 30, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration feature-window A merge of this requires an open feature window needs-info SMTP tests

Development

Successfully merging this pull request may close these issues.

4 participants