Skip to content

identity_select: match received header in reverse order (#10158)#10159

Open
johndoh wants to merge 4 commits into
roundcube:masterfrom
johndoh:identity_select
Open

identity_select: match received header in reverse order (#10158)#10159
johndoh wants to merge 4 commits into
roundcube:masterfrom
johndoh:identity_select

Conversation

@johndoh

@johndoh johndoh commented May 10, 2026

Copy link
Copy Markdown
Contributor

RFC 5321 says that additional Received headers should be prepended:

When forwarding a message into or out of the Internet environment, a
gateway MUST prepend a Received: line, but it MUST NOT alter in any
way a Received: line that is already in the header section.
I'm not sure adding an option to reverse the matching is needed, I think making it the behaviour "find the first address this message was sent to which matches an identity" makes sense.

I also adjusted the regex because having <> around the email address is common but not required.

closes #10158

@johndoh johndoh force-pushed the identity_select branch from c37717c to 8191d72 Compare May 10, 2026 12:31
@kjj2

kjj2 commented May 13, 2026

Copy link
Copy Markdown

After looking at this some more, and testing it on more emails in my inbox, I think there is a problem.

The get_email_from_header() function claims to return an array, and then the select() function checks each identity in turn until it finds an in_array() match. But get_email_from_header() always returns an array with one element - the first email address that it finds. (Or, it can fail to find an email address, in which case it seems to return either a null array or the array of raw headers matching the search term, both of which are even wrong-er.)

The problem is that a mailing list might preserve headers from before the email gets listified. In that case, the email addresses in the headers will be:

mailbox address
alias address <- the one we want to find
the address of the mailing list itself

In one email that I'm looking at, the bottom (oldest) received: header is:

Received: from blah blah
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by fakemailserver.org (Postfix) with ESMTPSA id 4gFvcB6wFyz4vyH
for mailing-list@distant-mail-server.org; Wed, 13 May 2026 13:35:54 +0000 (UTC)

Searching from either the top or the bottom gives the wrong result for these emails. Maybe something like this:

/**
 * Extract email addresses from specified message header
 */
protected function get_email_from_header($message, $header)
{
    $value = $message->headers->get($header, false);

    // received: embeds an email address among other stuff, so we have to regex it out
    // other headers would just be a raw email address that we can return

    if (strtolower($header) != 'received') return $value;

    $emails_found=array();

    foreach ( $value as $entry) {
        if (preg_match('/[\s\t]+for\s+<?([^\s>@]+@[^\s]+\.[^\s>;]+)>?/u', $entry, $matches)) {
            $emails_found[] = $matches[1];
        }
    }

    return (array) $emails_found;
}

@kjj2

kjj2 commented May 13, 2026

Copy link
Copy Markdown

With my version, I don't think there is any point in reversing the headers. The identity that will be selected is the first one in the $p['identities'] array, in whatever order foreach() presents them, that matches any email address in any of the user-selected headers.

On my system, the first identity on the Settings/Identities page is my real mailbox name, and then the aliases are listed alphabetically. Maybe we should reverse that list in the select() function before giving it to foreach.

public function select($p)
{
    if ($p['selected'] !== null || empty($p['message']->headers)) {
        return $p;
    }

    $rcmail = rcmail::get_instance();

    foreach ((array) $rcmail->config->get('identity_select_headers', []) as $header) {
        if ($emails = $this->get_email_from_header($p['message'], $header)) {
            $identity_list = array_reverse($p['identities'],TRUE); // TRUE here preserves the $idx to $ident mapping
            foreach ($identity_list as $idx => $ident) {
                if (in_array($ident['email_ascii'], $emails)) {
                    $p['selected'] = $idx;
                    break 2;
                }
            }
        }
    }

    return $p;
}

@johndoh

johndoh commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

I take your point about checking all the Received headers not just one but I think relying on the order of the identities is a mistake so instead I made it look for a matching identity in the (reverse) order of the Received headers.

@kjj2

kjj2 commented May 13, 2026

Copy link
Copy Markdown

Looks good. Tested and working on my system.

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.

identity_select option to reverse search order

2 participants