bug#67890: 28.2; nnmaildir silently deletes messages upon expiration

[Eric Abrahamsen <eric@ericabrahamsen.net>]

Weird, this is in my Sent folder, but obviously never was actually sent.
I was wondering why no one responded...

Łukasz Stelmach <stlman@poczta.fm> writes:

> I believe I've found a bug leading to a data loss situation.
>
> Steps to reproduce.

Thanks for the very detailed report! Obviously the `nnmaildir--pgname'
problem needs to be fixed, but otherwise it seems like the unlinking
step is also a clear bug: we should not be deleting the message if it
wasn't successfully expired someplace, right?

I'm thinking the end of the `t` clause in the cond there should look
like this:

(when (and (stringp target)
	   (not (string-equal target pgname))) ;; Move it.
  (erase-buffer)
  (nnheader-insert-file-contents nnmaildir--file)
  (let ((group-art (gnus-request-accept-article
		    target nil nil 'no-encode)))
    (if group-art
	(progn
	  (when (consp group-art)
	    ;; Maybe also copy: dormant forward reply save tick
	    ;; (gnus-add-mark? gnus-request-set-mark?)
	    (gnus-group-mark-article-read target (cdr group-art)))
	  ;; Article was successfully moved, delete the original.
	  (nnmaildir--unlink nnmaildir--file)
	  (nnmaildir--expired-article group article))
      ;; Leave it here.
      (setq didnt (cons (nnmaildir--art-num article) didnt)))))

It looks to me like `gnus-request-accept-article` can return a
(group . article) cons, a symbol, or nil for failure. If we get a nil,
we shouldn't remove the original article.

Does that seem reasonable?

Eric