From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: emacs-f@media.mit.edu Newsgroups: gmane.emacs.bugs Subject: bug#45128: [PATCH] rmail-spam-filter can lose mail Date: Tue, 08 Dec 2020 18:22:30 -0500 Message-ID: <87czzjnart.fsf@media.mit.edu> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7368"; mail-complaints-to="usenet@ciao.gmane.io" To: 45128@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Dec 09 00:36:49 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kmmXJ-0001r3-1C for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 09 Dec 2020 00:36:49 +0100 Original-Received: from localhost ([::1]:46098 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kmmXI-0006wG-3C for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 08 Dec 2020 18:36:48 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51322) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kmmUc-00053r-Px for bug-gnu-emacs@gnu.org; Tue, 08 Dec 2020 18:34:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48665) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kmmUc-0000Gy-Af for bug-gnu-emacs@gnu.org; Tue, 08 Dec 2020 18:34:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kmmUc-0002uE-8R for bug-gnu-emacs@gnu.org; Tue, 08 Dec 2020 18:34:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: emacs-f@media.mit.edu Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 08 Dec 2020 23:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 45128 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.160747043811155 (code B ref -1); Tue, 08 Dec 2020 23:34:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 8 Dec 2020 23:33:58 +0000 Original-Received: from localhost ([127.0.0.1]:60211 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kmmUX-0002tr-1B for submit@debbugs.gnu.org; Tue, 08 Dec 2020 18:33:58 -0500 Original-Received: from lists.gnu.org ([209.51.188.17]:46546) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kmmJZ-0000TY-E3 for submit@debbugs.gnu.org; Tue, 08 Dec 2020 18:22:37 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:47656) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kmmJZ-0005ZH-8h for bug-gnu-emacs@gnu.org; Tue, 08 Dec 2020 18:22:37 -0500 Original-Received: from amoxicillin.media.mit.edu ([18.27.72.70]:34632) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kmmJV-0004a9-Gp for bug-gnu-emacs@gnu.org; Tue, 08 Dec 2020 18:22:35 -0500 Original-Received: from bella.media.mit.edu (bella.media.mit.edu [18.27.127.33]) by amoxicillin.media.mit.edu (8.15.2/8.15.2/MEDIA) with ESMTP id 0B8NMV6a006547; Tue, 8 Dec 2020 18:22:31 -0500 Original-Received: from bella.media.mit.edu (bella.media.mit.edu [18.27.127.33]) by bella.media.mit.edu (Postfix) with ESMTP id DC32F32604; Tue, 8 Dec 2020 18:22:30 -0500 (EST) Original-Received: by bella.media.mit.edu (Postfix, from userid 1000) id B10B7123E20; Tue, 8 Dec 2020 18:22:30 -0500 (EST) Received-SPF: pass client-ip=18.27.72.70; envelope-from=foner@media.mit.edu; helo=amoxicillin.media.mit.edu X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Tue, 08 Dec 2020 18:33:55 -0500 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:195465 Archived-At: [See after description for patch and reproducer.] I just started using rmail-spam-filter to flush a couple of trolls, and discovered that it's got bugs which will cause mail to be spuriously deleted and/or overlooked. The bugs probably date back to the package's introduction; I'm running 24.3.1 and the code is unchanged in 27.1 except for tiny stylistic changes to three comments. There are two bugs here: (1) If an output-and-delete action fires, and rsf-file does not already exist, -the wrong message will be filed and deleted-! Specifically, the affected message will be the first unseen message, and -not- the spam. (This message will be "stealth-deleted"---it won't show up with a D in the summary window, but it -will- have a label of "deleted". Expunging or saving doesn't seem to actually delete it, but if you save, kill the RMAIL buffer, and reload it, -now- the message has a D and -then- it will be expunged on save. Also, it was simply deleted without the kill & reload in my main RMAIL file when I first hit this, though I haven't reproduced that. See below. It's buggy either way.) (2) Anytime rsf decides to prompt, it spuriously moves to the first unseen message in order to make the rest of the screen look better during the prompt. But that means that when control leaves rsf and reverts back to the usual mailreading flow, the user will entirely miss seeing that message, since RMAIL will have been incorrectly told the user has seen it. (This is because RMAIL marks the message as seen while rsf is displaying an entirely unrelated prompt about a piece of -spam-, so the user is likely looking at the prompt, not an entirely unrelated non-spam message. Then, when rsf exits, the message -after- what was the first unseen message is made current. This will cause users to simply miss mail unless they notice (perhaps from the summary buffer) that they've skipped one. And if there are two prompts (such as for an expunge), yet -another- message will be skipped.) Both of these bugs are caused by use of rmail-first-unseen-message in an undisciplined fashion---showing the message it denotes will side-effect RMAIL's state, but rsf's author didn't seem to know that. Note that bug #1 is caused by a failure to reset the current message (from the first unseen message to the target message) before carrying out the file & delete. This may have been overlooked because it will only occur if the output file doesn't already exist, so it'll happen on the first use and then the bug will seem to vanish, if it's even noticed at all. [Note: Even though I didn't do the kill-and-reload-the-buffer thing for my main RMAIL file, which is where I first noticed this bug, the wrong message -was- deleted. I could verify that this did in fact happen, because it wound up in rsf-file, and no line of that message could be found in my RMAIL file via grep, even though it -could- be found in a backup taken the day before. So the message was clearly deleted, and it was clearly filed---in fact, I wouldn't have caught the deletion at all if I hadn't immediately looked at rsf-file (after all, I was just starting to use rsf and was thus testing it out). Someone who was less paranoid could easily have simply lost an important message. My test case clearly shows the misfiling of the wrong message, through, and the stealth-deletion of it as well.] [Note: There are a lot of FIXME's scattered through the original code; I've removed one of them, but there are several left that I haven't looked at. In my own version (not in this patch), I've also added a few small features, such as (a) auto-expunge without prompting and/or (b) not even trying to offer to expunge and not expunging (to be left to the user later), and I've also tightened up the prompts and feedback, but those are -not- in this patch; if there's interest, I'll post a second patch. This patch is solely a bugfix.] Patch: --- rsf.original.el 2020-12-07 18:36:10.597449493 -0500 +++ rsf.patched.el 2020-12-07 18:40:29.811489011 -0500 @@ -216,0 +217,5 @@ +;; Don't spuriously advance to the next unseen message while prompting, because that causes it to then +;; be -missed- while actually reading mail afterwards! Call this instead of rmail-first-unseen-message. +(defun rsf-rmail-last-seen-message () + (max 1 (1- (or (rmail-first-unseen-message) 1)))) ; r-f-u-m can return nil in a completely empty buffer. + @@ -330,2 +335 @@ - (let ((rmail-current-message msg) ; FIXME does this do anything? - (action (cdr (assq 'action + (let ((action (cdr (assq 'action @@ -340 +344,2 @@ - (rmail-show-message (rmail-first-unseen-message) t)) + (rmail-show-message (rsf-rmail-last-seen-message) t)) + (setq rmail-current-message msg) ; Reset to spammy message! @@ -380 +385 @@ - (rmail-show-message (or (rmail-first-unseen-message) 1) t) + (rmail-show-message (rsf-rmail-last-seen-message) t) To reproduce: Create a file (let's call it test-rmail) with these contents: [BEGIN] >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 1 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: -------- foo. >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 2 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: -------- foo. >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 3 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: -------- foo. >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 4 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: ------U- foo. >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 5 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: ------U- foo. >From foo@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: foo@example.com To: foo@example.com Subject: 6 Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: ------U- foo. [END] Create a second file (let's call it test-spam) with these contents: [BEGIN] >From spam@example.com Mon Dec 7 19:05:49 2020 Return-Path: From: spam@example.com To: spam@example.com Subject: spammy Date: Mon, 07 Dec 2020 19:05:49 -0500 Message-ID: <87r1ozlffh.fsf@example.com> MIME-Version: 1.0 Content-Type: text/plain X-RMAIL-ATTRIBUTES: ------U- spam. [END] To reproduce in the unpatched code, make a second copy of "test-rmail" (it'll get destructively modified, and you want to be able to put it back), ensure that "spam-output" does not exist, and: (require 'rmail-spam-filter) (setq rsf-file "spam-output") (setq rsf-definitions-alist '( ((from . "spam@example.com") (action . output-and-delete)) )) Type: M-X rmail-input test-rmail If you type h, note that only message #1 is marked seen. Type: C-U g test-spam You'll be prompted to create spam-output; answer yes. Note that the message whose subject is "2" is now in spam-output (and potentially also deleted from rmail-test), and -not- the spam! Note also that you're magically on message #4, whereas typing g should have only incremented the current message by one, so you should be on message #2. To verify the patch: delete spam-output, regenerate test-rmail from its backup, kill the test-rmail buffer, load the patched code, and try the above steps again. None of the bugs should happen any more---the current message stays current (except for advancing by 1 only if g was used), and the spam message---and not the first unread message---winds up in spam-output and is deleted (and the deletion shows up in the summary buffer as a D, instead of being stealthed via just its label).