From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Markus Frank <m.frank@proxmox.com>, pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH pmg-api] fix #3924: ldap: skip non-smtp proxyAddresses
Date: Wed, 23 Mar 2022 13:03:23 +0100 [thread overview]
Message-ID: <78e6b927-9d4d-3e91-810c-fe50df5e4770@proxmox.com> (raw)
In-Reply-To: <20220318123706.6160-1-m.frank@proxmox.com>
On 18.03.22 13:37, Markus Frank wrote:
> When an entry in proxyAddresses starts with SIP (or everthing
> that is not smtp) no e-mail gets listed on the webpage.
> Therefore no quarantine notification messages are sent.
>
1. that statement doesn't really tells me any rationale or the actual
problem, also, which webpage? do you mean the web-interface (GUI)?
2. this loops not only over proxyAddresses but *all* mail attributes
(user configurable, default: mail, userPrincipalName, proxyAddresses,
othermailbox, mailAlternativeAddress) so unconditionally skipping all
that doesn't starts with smtp seems odd
3. the bug report shows that there is a smtp address, it just isn't on
the first line, should this rather cope with such multi-line stuff
and better extract the (any?) mail addresses there?
> To change this, every entry with another protocol than smtp,
> should be skipped.
>
> removed "SMTP" because of lowercase function is called before.
nit: could be a separate patch as its rather unrelated with the
skipping.
>
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> src/PMG/LDAPCache.pm | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/PMG/LDAPCache.pm b/src/PMG/LDAPCache.pm
> index df61454..8172329 100755
> --- a/src/PMG/LDAPCache.pm
> +++ b/src/PMG/LDAPCache.pm
> @@ -159,12 +159,17 @@ sub queryusers {
> next if !$user->{attributes}->{$attr};
> foreach my $mail (@{$user->{attributes}->{$attr}}) {
> $mail = lc($mail);
> +
> + # Check if proxyAddresses entry starts with anything
but it's not only 'proxyAddresses' that's accessed here (see 2. above).
> + # other than smtp and skip if true
> + next if ($mail =~ m/^(?!.*smtp).*[\:\$]/gs);
iff we'd do it this way a negated match (`!~` vs. `=~`) could be favorable over a
negative-lookahead readability wise.
> +
> # Test if the Line starts with one of the following lines:
> - # proxyAddresses: [smtp|SMTP]:
> + # proxyAddresses: [smtp]:
> # and also discard this starting string, so that $mail is only the
> # address without any other characters...
>
> - $mail =~ s/^(smtp|SMTP)[\:\$]//gs;
> + $mail =~ s/^smtp[\:\$]//gs;
>
> if ($mail !~ m/[\{\}\\\/]/ && $mail =~ m/^\S+\@\S+$/) {
> $umails->{$mail} = 1;
prev parent reply other threads:[~2022-03-23 12:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-18 12:37 Markus Frank
2022-03-23 12:03 ` Thomas Lamprecht [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=78e6b927-9d4d-3e91-810c-fe50df5e4770@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=m.frank@proxmox.com \
--cc=pmg-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox