~rjarry/aerc-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
14 9

[PATCH aerc] imap: fix panic in connect

Details
Message ID
<20240504200541.2414823-1-koni.marti@gmail.com>
DKIM signature
pass
Download raw message
Patch: +3 -2
Fix a nil pointer dereference panic in connect when trying to obtain the
folder delimiter. There is a List call to the imap server that in some
instances can return a nil *imap.MailboxInfo that is not checked before
it is dereferenced.

Link: https://lists.sr.ht/~rjarry/aerc-devel/%3CEDE672E5-3F6F-402D-B1A4-5477183FC13C@ukr.net%3E
Reported-by: Misha <ulianich_mihail@ukr.net>
Signed-off-by: Koni Marti <koni.marti@gmail.com>
---
 worker/imap/connect.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/worker/imap/connect.go b/worker/imap/connect.go
index 01ba7801..c109d3b7 100644
--- a/worker/imap/connect.go
+++ b/worker/imap/connect.go
@@ -101,8 +101,9 @@ func (w *IMAPWorker) connect() (*client.Client, error) {
	if err := c.List("", "", info); err != nil {
		return nil, fmt.Errorf("failed to retrieve delimiter: %w", err)
	}
	mailboxinfo := <-info
	w.delimiter = mailboxinfo.Delimiter
	if mailboxinfo := <-info; mailboxinfo != nil {
		w.delimiter = mailboxinfo.Delimiter
	}
	if w.delimiter == "" {
		// just in case some implementation does not follow standards
		w.delimiter = "/"
-- 
2.44.0

[aerc/patches] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<D114QY5JA3H8.3TOLL1FOXV35A@fra01>
In-Reply-To
<20240504200541.2414823-1-koni.marti@gmail.com> (view parent)
DKIM signature
missing
Download raw message
aerc/patches: SUCCESS in 1m57s

[imap: fix panic in connect][0] from [Koni Marti][1]

[0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/51599
[1]: koni.marti@gmail.com

✓ #1212363 SUCCESS aerc/patches/openbsd.yml     https://builds.sr.ht/~rjarry/job/1212363
✓ #1212362 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1212362
Details
Message ID
<41d15767-5851-47b0-b2eb-c91f991d4abb@sloti42n25>
In-Reply-To
<20240504200541.2414823-1-koni.marti@gmail.com> (view parent)
DKIM signature
pass
Download raw message
On Sat, 04 May 2024 15:05:50 CDT, Koni Marti wrote:
> Fix a nil pointer dereference panic in connect when trying to obtain the
> folder delimiter. There is a List call to the imap server that in some
> instances can return a nil *imap.MailboxInfo that is not checked before
> it is dereferenced.

Hey Koni -

Looks good. Just curious, which conditions cause the server to return a nil
here?

Reviewed-by: Tim Culverhouse <tim@tim.culverhouse.com>
Details
Message ID
<D116ZCMZIO7J.3AYDQCM6PY00Z@poldrack.dev>
In-Reply-To
<41d15767-5851-47b0-b2eb-c91f991d4abb@sloti42n25> (view parent)
DKIM signature
pass
Download raw message
On Sat 04 May 2024 17:41:35, Tim Culverhouse wrote:
> On Sat, 04 May 2024 15:05:50 CDT, Koni Marti wrote:
> Looks good. Just curious, which conditions cause the server to return a nil
> here?

My guess would be a non-standard one. I've tried connecting manually
through netcat and openssl and was kicked by the server both times.

-- 
Moritz Poldrack
https://moritz.sh

> List at least two alternate dates.
Details
Message ID
<D1191BRC4GPS.1ODCEE4VWJS07@gmail.com>
In-Reply-To
<41d15767-5851-47b0-b2eb-c91f991d4abb@sloti42n25> (view parent)
DKIM signature
pass
Download raw message
On Sat May 4, 2024 at 11:41 PM CEST, Tim Culverhouse wrote:
> On Sat, 04 May 2024 15:05:50 CDT, Koni Marti wrote:
> > Fix a nil pointer dereference panic in connect when trying to obtain the
> > folder delimiter. There is a List call to the imap server that in some
> > instances can return a nil *imap.MailboxInfo that is not checked before
> > it is dereferenced.
>
> Hey Koni -
>
> Looks good. Just curious, which conditions cause the server to return a nil
> here?

That imap server sends a valid OK response without any data back:

0o064Q LIST "" ""

0o064Q OK LIST completed


However, a proper (expected) response would look like this:

FPfrKQ LIST "" ""

* LIST (\Noselect) "/" "/"

FPfrKQ OK Success


So the call to LIST returns correctly and closes the channel from which
we try to read afterwards and this results in a nil *imap.MailboxInfo.
Details
Message ID
<a64e2264-4955-4439-8c7b-6fe7d6cec0ad@ferdinandy.com>
In-Reply-To
<D1191BRC4GPS.1ODCEE4VWJS07@gmail.com> (view parent)
DKIM signature
missing
Download raw message
2024. máj. 5. 1:29:47 Koni Marti <koni.marti@gmail.com>:

> On Sat May 4, 2024 at 11:41 PM CEST, Tim Culverhouse wrote:
>> On Sat, 04 May 2024 15:05:50 CDT, Koni Marti wrote:
>>> Fix a nil pointer dereference panic in connect when trying to obtain 
>>> the
>>> folder delimiter. There is a List call to the imap server that in 
>>> some
>>> instances can return a nil *imap.MailboxInfo that is not checked 
>>> before
>>> it is dereferenced.
>>
>> Hey Koni -
>>
>> Looks good. Just curious, which conditions cause the server to return 
>> a nil
>> here?
>
> That imap server sends a valid OK response without any data back:
>
> 0o064Q LIST "" ""
>
> 0o064Q OK LIST completed
>
>
> However, a proper (expected) response would look like this:
>
> FPfrKQ LIST "" ""
>
> * LIST (\Noselect) "/" "/"
>
> FPfrKQ OK Success
>
>
> So the call to LIST returns correctly and closes the channel from which
> we try to read afterwards and this results in a nil *imap.MailboxInfo.

Maybe in this case aerc should also open a prewritten email to the 
provider to let them know they are not doing it correctly :D
Details
Message ID
<4D325E25-B016-43A7-8303-6BB5717399FC@ukr.net>
In-Reply-To
<a64e2264-4955-4439-8c7b-6fe7d6cec0ad@ferdinandy.com> (view parent)
DKIM signature
pass
Download raw message
>Maybe in this case aerc should also open a prewritten email to the provider to let them know they are not doing it correctly :D

Explanatory message in stderr would be enough for me. It's likely that user is new (configuring aerc for the first time) and doesn't feel confident enough with the client to send mail anyway... Especially in a "blind mode" where they can only send (can they?), but not fetch mail.

From my recent experience with K9 mail, mail was stuck in "outbox" despite being [silently] sent — until I've configured special folders properly. Eventually I'm not sure how many times I've sent duplicated mails.
<https://forum.k9mail.app/t/messages-stuck-in-outbox/7784/6>

>> However, a proper (expected) response would look like this

I remember I was succesfully using sylpheed and claws mail with this mail provider many years ago (something could have changed since then though). Is it something like undefined/non-standartized behavior that other clients stick with?

BTW, should I use "reply to all" button or reply only to mail list (manually remove other addresses)? Do other recipients get duplicated emails if they are also subscribed to the mail list? I assume I should not press "reply", because mail list doesn't receive the message in that case

reply -L [Was: Re: [PATCH aerc] imap: fix panic in connect]

Details
Message ID
<D11Z0OA5M6GU.33P50IADP22DL@cepl.eu>
In-Reply-To
<4D325E25-B016-43A7-8303-6BB5717399FC@ukr.net> (view parent)
DKIM signature
pass
Download raw message
5. května 2024 11:18:48 SELČ, Misha <ulianich_mihail@ukr.net> napsal:
> BTW, should I use "reply to all" button or reply only to mail
> list (manually remove other addresses)? Do other recipients get
> duplicated emails if they are also subscribed to the mail list? I
> assume I should not press "reply", because mail list doesn’t
> receive the message in that case

Other email clients (I know for certain about mutt, Evolution,
and Thunderbird) have a special function reply-to-list
(description for mutt [1], Thunderbird [2], and just mention of
it for Evolution [3]).

If I understand correctly, when Mail-Followup-To [4] or List-Post
[5] headers are set, the MUA just clears out all senders headers
(To:, Cc:, and Bcc:) are cleared and To: is set to the value of
such header (if both of these are set, I am not sure what’s their
preference, I would vote for List-Post as RFC seems to me more
official than just Professor Bernstein’s rambling).

Practially, for aerc, I would imagine that reply command could
just gain -l parameter.

What do you think?

Best,

Matěj

[1] http://mutt.org/doc/manual/#using-lists
[2] https://wiki.mozilla.org/Thunderbird:Help_Documentation:Mail-Followup-To_and_Mail-Reply-To
[3] https://help.gnome.org/users/evolution/stable/mail-composer-reply.html.en
[4] https://cr.yp.to/proto/replyto.html
[5] https://www.rfc-editor.org/rfc/rfc4021.html#section-2.1.35
-- 
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5  BC1D 7920 5802 880B C9D8
 
Basically, the only “intuitive” interface is the nipple.  After
that, it’s all learned.
    -- Bruce Ediger when discussing intuivity of Mac OS
       http://groups.google.com/group/comp.sys.next.advocacy\
       /msg/7fa8c580900353d0

Re: reply -L [Was: Re: [PATCH aerc] imap: fix panic in connect]

Jens Grassel <jens@wegtam.com>
Details
Message ID
<D12DR1B71I7P.3FXBA83340NCN@wegtam.com>
In-Reply-To
<D11Z0OA5M6GU.33P50IADP22DL@cepl.eu> (view parent)
DKIM signature
pass
Download raw message
On 2024-05-05 (Sun) 21:51 CEST, Matěj Cepl wrote:
> [...]
> Practially, for aerc, I would imagine that reply command could
> just gain -l parameter.
> [...]

I second that and maybe it could be made the default behaviour if an
email contains a header indicating a mailing list (List-Id?)?

Kind regards,

Jens

-- 
Wegtam GmbH, CTO 2024-05-06 09:21
Homepage : https://www.wegtam.com

"As a gentleman, I must warn you, if you so much as glance at another woman,
I'll be over Leela like a fly on a pile of very seductive manure." -Zapp

Re: reply -L [Was: Re: [PATCH aerc] imap: fix panic in connect]

Details
Message ID
<b8dac2c6-8ce9-4bdf-a214-f42752233a66@ferdinandy.com>
In-Reply-To
<D12DR1B71I7P.3FXBA83340NCN@wegtam.com> (view parent)
DKIM signature
missing
Download raw message
2024. máj. 6. 9:24:48 Jens Grassel <jens@wegtam.com>:

> On 2024-05-05 (Sun) 21:51 CEST, Matěj Cepl wrote:
>> [...]
>> Practially, for aerc, I would imagine that reply command could
>> just gain -l parameter.
>> [...]
>
> I second that and maybe it could be made the default behaviour if an
> email contains a header indicating a mailing list (List-Id?)?


It's possible to write on these MLs without being subscribed and often 
times people are purposefully cc-d, and it's impossible to know if they 
are subscribed or not. So that seems a bit dangerous, while I don't think 
sourcehut sends duplicates, so at least for these MLs the current 
behaviour seems to be the saner option.

>
> Kind regards,
>
> Jens
>
> --
> Wegtam GmbH, CTO 2024-05-06 09:21
> Homepage : https://www.wegtam.com
>
> "As a gentleman, I must warn you, if you so much as glance at another 
> woman,
> I'll be over Leela like a fly on a pile of very seductive manure." 
> -Zapp

Re: reply -L [Was: Re: [PATCH aerc] imap: fix panic in connect]

Details
Message ID
<D12HBPA2ZUUK.QEVJULK8MIXP@cepl.eu>
In-Reply-To
<D12DR1B71I7P.3FXBA83340NCN@wegtam.com> (view parent)
DKIM signature
pass
Download raw message
On Mon May 6, 2024 at 9:23 AM CEST, Jens Grassel wrote:
> I second that and maybe it could be made the default behaviour if an
> email contains a header indicating a mailing list (List-Id?)?

The world is actually evil evil place, so there are email list so
screwed up that they have List-Id and no List-Post.

Matěj

-- 
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5  BC1D 7920 5802 880B C9D8
 
Therefore, faithful Christian, seek truth, hear truth, learn
truth, love truth, speak truth, hold truth, defend truth until
death: because truth will free you from sin, from devil, from the
death of soul and finally from the death eternal, which is a
separation from God’s mercy.
    -- Master John Hus, Explanation of Credo, 1412

Applied: [PATCH aerc] imap: fix panic in connect

Details
Message ID
<171528466392.17778.14836999383503641950@ringo.local>
In-Reply-To
<20240504200541.2414823-1-koni.marti@gmail.com> (view parent)
DKIM signature
pass
Download raw message
Koni Marti <koni.marti@gmail.com> wrote:
> Fix a nil pointer dereference panic in connect when trying to obtain the
> folder delimiter. There is a List call to the imap server that in some
> instances can return a nil *imap.MailboxInfo that is not checked before
> it is dereferenced.
>
> Link: https://lists.sr.ht/~rjarry/aerc-devel/%3CEDE672E5-3F6F-402D-B1A4-5477183FC13C@ukr.net%3E
> Reported-by: Misha <ulianich_mihail@ukr.net>
> Signed-off-by: Koni Marti <koni.marti@gmail.com>
> ---

Acked-by: Robin Jarry <robin@jarry.cc>

Applied, thanks.

To git@git.sr.ht:~rjarry/aerc
   1d96da5126e6..8307776f893b  master -> master

Re: Applied: [PATCH aerc] imap: fix panic in connect

Details
Message ID
<DE28F79B-CF2C-4D22-BC9A-E69765899318@ukr.net>
In-Reply-To
<171528466392.17778.14836999383503641950@ringo.local> (view parent)
DKIM signature
pass
Download raw message
Thanks, I confirm that it works! I did not expect aerc to run GNU nano for message compose though, but I guess it's because my $EDITOR is unset XD 

Do you think there is something I should report to my email provider about their protocol implementation? 

-- 
Sent from my Android device with K-9 Mail.
http://belkka.pp.ua

Re: Applied: [PATCH aerc] imap: fix panic in connect

Details
Message ID
<D18S0FFD1RUS.2RMR4YDYLK79P@ringo>
In-Reply-To
<DE28F79B-CF2C-4D22-BC9A-E69765899318@ukr.net> (view parent)
DKIM signature
pass
Download raw message
Misha, May 10, 2024 at 15:29:
> Thanks, I confirm that it works! I did not expect aerc to run GNU nano 
> for message compose though, but I guess it's because my $EDITOR is 
> unset XD

Hmm, weird, it should fallback to vi if $EDITOR is unset. Are you sure 
you don't have a default $EDITOR set to nano by some satanic ubuntu 
profile.d script?

> Do you think there is something I should report to my email provider 
> about their protocol implementation? 

I don't think reporting the folder separator is mandatory in the IMAP 
standard. I could be wrong though.

Re: Applied: [PATCH aerc] imap: fix panic in connect

Details
Message ID
<D1BAARB6J77G.3T7FLZ6EQ2R08@ukr.net>
In-Reply-To
<D18S0FFD1RUS.2RMR4YDYLK79P@ringo> (view parent)
DKIM signature
pass
Download raw message
On Mon May 13, 2024 at 10:50 PM EEST, Robin Jarry wrote:
> Hmm, weird, it should fallback to vi if $EDITOR is unset. Are you sure 
> you don't have a default $EDITOR set to nano by some satanic ubuntu 
> profile.d script?

No, my $EDITOR is unset. But I do not have vi available in $PATH, 
only vim and nvim. vim is actually

/usr/local/bin/vim: symbolic link to /usr/bin/nvim

(probably created manually, because pacman finds no package owning vim) 


BTW, I have $VISUAL set to nvim, but it looks like aerc only checks 
$EDITOR. I used to have EDITOR=nvim, but changed it to VISUAL=nvim after 
I learned that $EDITOR is intended for old, less capable editors like ex, 
and $VISUAL is for more advanced like emacs, vim and even vi:

https://unix.stackexchange.com/questions/4859/visual-vs-editor-what-s-the-difference

I tried to set `editor=$VISUAL` in aerc.conf, but it doesn't expand 
variables. `editor='bash -c $VISUAL'` does run neovim, but, apparently, 
a filename (generated by aerc) is treated as an argument for bash, 
not an argument for $VISUAL (so it doesn't work as expected). 
I gave up playing with this and set `editor=nvim`. It's not a big issue, 
just sharing my experience.

-- 
Your random number is 18666. BTW I use aerc
http://belkka.pp.ua
Reply to thread Export thread (mbox)