[PATCH] Check for erroneous subjects

[PATCH] Check for erroneous subjects

[Tony Zorman]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

Hi,

this adds a new custom variable, `notmuch-mua-subject-regexp`, and an
associated function, `notmuch-mua-subject-check`, to warn the user when
the subject contains potentially troublesome things (e.g., nothing at
all). The idea is the same as `notmuch-mua-attachment-regexp`—which has
saved my skin quite a few times in the past—but for the subject instead
of an attachment. By default, it checks for empty subjects, as that
seems to be a reasonable thing to safeguard against.

I've not found any NEWS entry (nor any documentation) for
`notmuch-mua-attachment-regexp`, so I wasn't sure whether to add any
myself. So far, I haven't done so, but that can of course quickly be
fixed.

Best,
  Tony


[-- Attachment #2: 0001-emacs-mua-optionally-check-for-erroneous-subjects.patch --]
[-- Type: text/x-patch, Size: 1924 bytes --]

From 3ccd1f36cbda55766bf21578b5258b21105e0803 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Sun, 29 Oct 2023 08:26:32 +0100
Subject: [PATCH 1/2] emacs/mua: optionally check for erroneous subjects

This works much like notmuch-mua-attachment-regexp, but for the
subject instead. By default, check for empty subjects, as that seems a
reasonable thing to safeguard against.
---
 emacs/notmuch-mua.el | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index e4b7e9d1..5750f181 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -142,6 +142,16 @@ to `notmuch-mua-send-hook'."
   :type 'regexp
   :group 'notmuch-send)
 
+(defcustom notmuch-mua-subject-regexp
+  "[[:blank:]]*$"
+  "Message subject indicating that something may be amiss.
+By default, this checks for empty subject lines.
+
+This is not used unless `notmuch-mua-subject-check' is added to
+`notmuch-mua-send-hook'."
+  :type 'regexp
+  :group 'notmuch-send)
+
 ;;; Various functions
 
 (defun notmuch-mua-attachment-check ()
@@ -179,6 +189,19 @@ Typically this is added to `notmuch-mua-send-hook'."
     ;; ...signal an error.
     (error "Missing attachment")))
 
+(defun notmuch-mua-subject-check ()
+  "Signal an error if the subject seems amiss.
+More precisely, if the subject conforms to
+`notmuch-mua-subject-regexp'.
+
+Typically this is added to `notmuch-mua-send-hook'."
+  (or (save-excursion
+        (message-goto-subject)
+        (message-beginning-of-header t)
+        (not (looking-at-p notmuch-mua-subject-regexp)))
+      (y-or-n-p "Subject may be erroneous – is that okay?")
+      (error "Erroneous subject")))
+
 (defun notmuch-mua-get-switch-function ()
   "Get a switch function according to `notmuch-mua-compose-in'."
   (pcase notmuch-mua-compose-in
-- 
2.42.0


[-- Attachment #3: 0002-test-emacs-test-notmuch-mua-subject-check.patch --]
[-- Type: text/x-patch, Size: 2719 bytes --]

From df875e9dcacda21bc09ccef869c883424a2b4467 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Sun, 29 Oct 2023 08:25:21 +0100
Subject: [PATCH 2/2] test/emacs: test notmuch-mua-subject-check

---
 test/T740-emacs-subject-warnings.sh | 12 ++++++++
 test/emacs-subject-warnings.el      | 48 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100755 test/T740-emacs-subject-warnings.sh
 create mode 100644 test/emacs-subject-warnings.el

diff --git a/test/T740-emacs-subject-warnings.sh b/test/T740-emacs-subject-warnings.sh
new file mode 100755
index 00000000..f9d27140
--- /dev/null
+++ b/test/T740-emacs-subject-warnings.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+test_description="emacs subject warnings"
+. $(dirname "$0")/test-lib.sh || exit 1
+. $NOTMUCH_SRCDIR/test/test-lib-emacs.sh || exit 1
+
+test_require_emacs
+
+test_begin_subtest "notmuch-test-subject-warning part 1"
+test_emacs_expect_t '(notmuch-test-subject-warning-1)'
+
+test_done
diff --git a/test/emacs-subject-warnings.el b/test/emacs-subject-warnings.el
new file mode 100644
index 00000000..1e547429
--- /dev/null
+++ b/test/emacs-subject-warnings.el
@@ -0,0 +1,48 @@
+(require 'cl-lib)
+(require 'notmuch-mua)
+
+(defun subject-check-test (&optional fn)
+  "Test `notmuch-mua-subject-check'.
+Optionally, evaluate FN before doing the test.
+
+Return t if the message would be sent, and nil otherwise."
+  (notmuch-mua-mail)
+  (message-goto-subject)
+  (when fn
+    (funcall fn))
+  (prog1
+      (condition-case nil
+	  ;; Force `y-or-n-p' to always return `nil', as if the user
+	  ;; pressed "n".
+	  (cl-letf (((symbol-function 'y-or-n-p)
+		     (lambda (&rest args) nil)))
+	    (notmuch-mua-subject-check)
+	    t)
+	('error nil))
+    (set-buffer-modified-p nil)
+    (kill-buffer (current-buffer))))
+
+(defvar subject-check-tests
+  '(;; These are okay.
+    (t . (lambda () (insert "something")))
+    ;; These should not be okay.
+    (nil)
+    (nil . (lambda () (insert "         ")))
+    (nil . (lambda () (insert "		  	")))
+    (nil . (lambda () (insert " ")))	; NON-BREAKING SPACE
+    ))
+
+(defun notmuch-test-subject-warning-1 ()
+  (let (output expected)
+    (dolist (test subject-check-tests)
+      (let* ((expect (car test))
+	     (body (cdr test))
+	     (result (subject-check-test body)))
+	(push expect expected)
+	(push (if (eq result expect)
+		  result
+		;; In the case of a failure, include the test
+		;; details to make it simpler to debug.
+		(format "%S <-- %S" result body))
+	      output)))
+    (notmuch-test-expect-equal output expected)))
-- 
2.42.0


[-- Attachment #4: Type: text/plain, Size: 44 bytes --]


-- 
Tony Zorman | https://tony-zorman.com/

[-- Attachment #5: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] Check for erroneous subjects

[Tony Zorman]

Just checking in to see how this is doing :)

On Sun, Oct 29 2023 08:49, Tony Zorman wrote:
> Hi,
>
> this adds a new custom variable, `notmuch-mua-subject-regexp`, and an
> associated function, `notmuch-mua-subject-check`, to warn the user when
> the subject contains potentially troublesome things (e.g., nothing at
> all). The idea is the same as `notmuch-mua-attachment-regexp`—which has
> saved my skin quite a few times in the past—but for the subject instead
> of an attachment. By default, it checks for empty subjects, as that
> seems to be a reasonable thing to safeguard against.
>
> I've not found any NEWS entry (nor any documentation) for
> `notmuch-mua-attachment-regexp`, so I wasn't sure whether to add any
> myself. So far, I haven't done so, but that can of course quickly be
> fixed.
>
> Best,
>   Tony
>
> From 3ccd1f36cbda55766bf21578b5258b21105e0803 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Sun, 29 Oct 2023 08:26:32 +0100
> Subject: [PATCH 1/2] emacs/mua: optionally check for erroneous subjects
>
> This works much like notmuch-mua-attachment-regexp, but for the
> subject instead. By default, check for empty subjects, as that seems a
> reasonable thing to safeguard against.
> ---
>  emacs/notmuch-mua.el | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index e4b7e9d1..5750f181 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -142,6 +142,16 @@ to `notmuch-mua-send-hook'."
>    :type 'regexp
>    :group 'notmuch-send)
>  
> +(defcustom notmuch-mua-subject-regexp
> +  "[[:blank:]]*$"
> +  "Message subject indicating that something may be amiss.
> +By default, this checks for empty subject lines.
> +
> +This is not used unless `notmuch-mua-subject-check' is added to
> +`notmuch-mua-send-hook'."
> +  :type 'regexp
> +  :group 'notmuch-send)
> +
>  ;;; Various functions
>  
>  (defun notmuch-mua-attachment-check ()
> @@ -179,6 +189,19 @@ Typically this is added to `notmuch-mua-send-hook'."
>      ;; ...signal an error.
>      (error "Missing attachment")))
>  
> +(defun notmuch-mua-subject-check ()
> +  "Signal an error if the subject seems amiss.
> +More precisely, if the subject conforms to
> +`notmuch-mua-subject-regexp'.
> +
> +Typically this is added to `notmuch-mua-send-hook'."
> +  (or (save-excursion
> +        (message-goto-subject)
> +        (message-beginning-of-header t)
> +        (not (looking-at-p notmuch-mua-subject-regexp)))
> +      (y-or-n-p "Subject may be erroneous – is that okay?")
> +      (error "Erroneous subject")))
> +
>  (defun notmuch-mua-get-switch-function ()
>    "Get a switch function according to `notmuch-mua-compose-in'."
>    (pcase notmuch-mua-compose-in
> -- 
> 2.42.0
>
> From df875e9dcacda21bc09ccef869c883424a2b4467 Mon Sep 17 00:00:00 2001
> From: Tony Zorman <soliditsallgood@mailbox.org>
> Date: Sun, 29 Oct 2023 08:25:21 +0100
> Subject: [PATCH 2/2] test/emacs: test notmuch-mua-subject-check
>
> ---
>  test/T740-emacs-subject-warnings.sh | 12 ++++++++
>  test/emacs-subject-warnings.el      | 48 +++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100755 test/T740-emacs-subject-warnings.sh
>  create mode 100644 test/emacs-subject-warnings.el
>
> diff --git a/test/T740-emacs-subject-warnings.sh b/test/T740-emacs-subject-warnings.sh
> new file mode 100755
> index 00000000..f9d27140
> --- /dev/null
> +++ b/test/T740-emacs-subject-warnings.sh
> @@ -0,0 +1,12 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs subject warnings"
> +. $(dirname "$0")/test-lib.sh || exit 1
> +. $NOTMUCH_SRCDIR/test/test-lib-emacs.sh || exit 1
> +
> +test_require_emacs
> +
> +test_begin_subtest "notmuch-test-subject-warning part 1"
> +test_emacs_expect_t '(notmuch-test-subject-warning-1)'
> +
> +test_done
> diff --git a/test/emacs-subject-warnings.el b/test/emacs-subject-warnings.el
> new file mode 100644
> index 00000000..1e547429
> --- /dev/null
> +++ b/test/emacs-subject-warnings.el
> @@ -0,0 +1,48 @@
> +(require 'cl-lib)
> +(require 'notmuch-mua)
> +
> +(defun subject-check-test (&optional fn)
> +  "Test `notmuch-mua-subject-check'.
> +Optionally, evaluate FN before doing the test.
> +
> +Return t if the message would be sent, and nil otherwise."
> +  (notmuch-mua-mail)
> +  (message-goto-subject)
> +  (when fn
> +    (funcall fn))
> +  (prog1
> +      (condition-case nil
> +	  ;; Force `y-or-n-p' to always return `nil', as if the user
> +	  ;; pressed "n".
> +	  (cl-letf (((symbol-function 'y-or-n-p)
> +		     (lambda (&rest args) nil)))
> +	    (notmuch-mua-subject-check)
> +	    t)
> +	('error nil))
> +    (set-buffer-modified-p nil)
> +    (kill-buffer (current-buffer))))
> +
> +(defvar subject-check-tests
> +  '(;; These are okay.
> +    (t . (lambda () (insert "something")))
> +    ;; These should not be okay.
> +    (nil)
> +    (nil . (lambda () (insert "         ")))
> +    (nil . (lambda () (insert "		  	")))
> +    (nil . (lambda () (insert " ")))	; NON-BREAKING SPACE
> +    ))
> +
> +(defun notmuch-test-subject-warning-1 ()
> +  (let (output expected)
> +    (dolist (test subject-check-tests)
> +      (let* ((expect (car test))
> +	     (body (cdr test))
> +	     (result (subject-check-test body)))
> +	(push expect expected)
> +	(push (if (eq result expect)
> +		  result
> +		;; In the case of a failure, include the test
> +		;; details to make it simpler to debug.
> +		(format "%S <-- %S" result body))
> +	      output)))
> +    (notmuch-test-expect-equal output expected)))
> -- 
> 2.42.0
-- 
Tony Zorman | https://tony-zorman.com/\r

Re: [PATCH] Check for erroneous subjects

[David Bremner]

Tony Zorman <soliditsallgood@mailbox.org> writes:

> Hi,
>
> this adds a new custom variable, `notmuch-mua-subject-regexp`, and an
> associated function, `notmuch-mua-subject-check`, to warn the user when
> the subject contains potentially troublesome things (e.g., nothing at
> all). The idea is the same as `notmuch-mua-attachment-regexp`—which has
> saved my skin quite a few times in the past—but for the subject instead
> of an attachment. By default, it checks for empty subjects, as that
> seems to be a reasonable thing to safeguard against.

Applied to master. Thanks for writing such nice tests.

d\r