[PATCH 0/4] emacs: avoid type errors due to nil as content-type

[PATCH 0/4] emacs: avoid type errors due to nil as content-type

[Jonas Bernoulli]

The fourth commit tries to address the issue raised in
id:87k0sxa6c3.fsf@fifthhorseman.net.

  The output of "notmuch show --format=sexp --format-version=4"
  may contain `:content-type' entries with `nil' as the value,
  when it fails to detect the correct value.  Account for that
  in a few places where we would otherwise risk a type error.

The other three commits cleanup the code I had to look at to do
the above, because I just can't help myself.

This does not apply without first applying
id:20201214162401.19569-1-jonas@bernoul.li.

     Jonas

Jonas Bernoulli (4):
  emacs: notmuch-mua-add-more-hidden-headers: use local binding
  emacs: notmuch-show--register-cids: fix names of bindings
  emacs: notmuch-show--get-cid-content: cosmetics
  emacs: avoid type errors due to nil as content-type

 emacs/notmuch-lib.el  | 23 ++++++++++++----------
 emacs/notmuch-mua.el  |  7 +++----
 emacs/notmuch-show.el | 46 +++++++++++++++++++++----------------------
 3 files changed, 39 insertions(+), 37 deletions(-)

-- 
2.30.0

[PATCH 1/4] emacs: notmuch-mua-add-more-hidden-headers: use local binding

[Jonas Bernoulli]

---
 emacs/notmuch-mua.el | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 08c73c16..bbf059a2 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -217,11 +217,10 @@ (defun notmuch-mua-add-more-hidden-headers ()
 (defun notmuch-mua-reply-crypto (parts)
   "Add mml sign-encrypt flag if any part of original message is encrypted."
   (cl-loop for part in parts
-	   if (notmuch-match-content-type (plist-get part :content-type)
-					  "multipart/encrypted")
+	   for type = (plist-get part :content-type)
+	   if (notmuch-match-content-type type "multipart/encrypted")
 	   do (mml-secure-message-sign-encrypt)
-	   else if (notmuch-match-content-type (plist-get part :content-type)
-					       "multipart/*")
+	   else if (notmuch-match-content-type type "multipart/*")
 	   do (notmuch-mua-reply-crypto (plist-get part :content))))
 
 ;; There is a bug in Emacs' message.el that results in a newline
-- 
2.30.0

[PATCH 2/4] emacs: notmuch-show--register-cids: fix names of bindings

[Jonas Bernoulli]

---
 emacs/notmuch-show.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index fdf4ab3c..21133611 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -581,13 +581,13 @@ (defun notmuch-show--register-cids (msg part)
       ;; alternative (even if we can't render it).
       (push (list content-id msg part) notmuch-show--cids)))
   ;; Recurse on sub-parts
-  (pcase-let ((`(,content ,type)
+  (pcase-let ((`(,type ,subtype)
 	       (split-string (downcase (plist-get part :content-type)) "/")))
-    (cond ((equal content "multipart")
+    (cond ((equal type "multipart")
 	   (mapc (apply-partially #'notmuch-show--register-cids msg)
 		 (plist-get part :content)))
-	  ((and (equal content "message")
-		(equal type "rfc822"))
+	  ((and (equal type "message")
+		(equal subtype "rfc822"))
 	   (notmuch-show--register-cids
 	    msg
 	    (car (plist-get (car (plist-get part :content)) :body)))))))
-- 
2.30.0

[PATCH 3/4] emacs: notmuch-show--get-cid-content: cosmetics

[Jonas Bernoulli]

---
 emacs/notmuch-show.el | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 21133611..8c846fb2 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -599,16 +599,13 @@ (defun notmuch-show--get-cid-content (cid)
 into the current buffer.  CID must be a raw content ID, without
 enclosing angle brackets, a cid: prefix, or URL encoding.  This
 will return nil if the CID is unknown or cannot be retrieved."
-  (let ((descriptor (cdr (assoc cid notmuch-show--cids))))
-    (when descriptor
-      (let* ((msg (car descriptor))
-	     (part (cadr descriptor))
-	     ;; Request caching for this content, as some messages
-	     ;; reference the same cid: part many times (hundreds!).
-	     (content (notmuch-get-bodypart-binary
-		       msg part notmuch-show-process-crypto 'cache))
-	     (content-type (plist-get part :content-type)))
-	(list content content-type)))))
+  (when-let ((descriptor (cdr (assoc cid notmuch-show--cids))))
+    (pcase-let ((`(,msg ,part) descriptor))
+      ;; Request caching for this content, as some messages
+      ;; reference the same cid: part many times (hundreds!).
+      (list (notmuch-get-bodypart-binary
+	     msg part notmuch-show-process-crypto 'cache)
+	    (plist-get part :content-type)))))
 
 (defun notmuch-show-setup-w3m ()
   "Instruct w3m how to retrieve content from a \"related\" part of a message."
-- 
2.30.0

[PATCH 4/4] emacs: avoid type errors due to nil as content-type

[Jonas Bernoulli]

The output of "notmuch show --format=sexp --format-version=4"
may contain `:content-type' entries with `nil' as the value,
when it fails to detect the correct value.  Account for that
in a few places where we would otherwise risk a type error.

Note that `string=' does not choke on `nil' because it uses
the `symbol-name' when encountering a symbol.
---
 emacs/notmuch-lib.el  | 23 +++++++++++++----------
 emacs/notmuch-show.el | 29 ++++++++++++++++-------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index bc550dc2..c7bb2091 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -552,16 +552,19 @@ (defun notmuch-plist-delete (plist property)
 ;;; MML Utilities
 
 (defun notmuch-match-content-type (t1 t2)
-  "Return t if t1 and t2 are matching content types, taking wildcards into account."
-  (let ((st1 (split-string t1 "/"))
-	(st2 (split-string t2 "/")))
-    (if (or (string= (cadr st1) "*")
-	    (string= (cadr st2) "*"))
-	;; Comparison of content types should be case insensitive.
-	(string= (downcase (car st1))
-		 (downcase (car st2)))
-      (string= (downcase t1)
-	       (downcase t2)))))
+  "Return t if t1 and t2 are matching content types.
+Take wildcards into account."
+  (and (stringp t1)
+       (stringp t2)
+       (let ((st1 (split-string t1 "/"))
+	     (st2 (split-string t2 "/")))
+	 (if (or (string= (cadr st1) "*")
+		 (string= (cadr st2) "*"))
+	     ;; Comparison of content types should be case insensitive.
+	     (string= (downcase (car st1))
+		      (downcase (car st2)))
+	   (string= (downcase t1)
+		    (downcase t2))))))
 
 (defvar notmuch-multipart/alternative-discouraged
   '(;; Avoid HTML parts.
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 8c846fb2..ba93febb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -581,16 +581,17 @@ (defun notmuch-show--register-cids (msg part)
       ;; alternative (even if we can't render it).
       (push (list content-id msg part) notmuch-show--cids)))
   ;; Recurse on sub-parts
-  (pcase-let ((`(,type ,subtype)
-	       (split-string (downcase (plist-get part :content-type)) "/")))
-    (cond ((equal type "multipart")
-	   (mapc (apply-partially #'notmuch-show--register-cids msg)
-		 (plist-get part :content)))
-	  ((and (equal type "message")
-		(equal subtype "rfc822"))
-	   (notmuch-show--register-cids
-	    msg
-	    (car (plist-get (car (plist-get part :content)) :body)))))))
+  (when-let ((type (plist-get part :content-type)))
+    (pcase-let ((`(,type ,subtype)
+		 (split-string (downcase type) "/")))
+      (cond ((equal type "multipart")
+	     (mapc (apply-partially #'notmuch-show--register-cids msg)
+		   (plist-get part :content)))
+	    ((and (equal type "message")
+		  (equal subtype "rfc822"))
+	     (notmuch-show--register-cids
+	      msg
+	      (car (plist-get (car (plist-get part :content)) :body))))))))
 
 (defun notmuch-show--get-cid-content (cid)
   "Return a list (CID-content content-type) or nil.
@@ -948,7 +949,8 @@ (defun notmuch-show-lazy-part (part-args button)
 
 (defun notmuch-show-mime-type (part)
   "Return the correct mime-type to use for PART."
-  (let ((content-type (downcase (plist-get part :content-type))))
+  (when-let ((content-type (plist-get part :content-type)))
+    (setq content-type (downcase content-type))
     (or (and (string= content-type "application/octet-stream")
 	     (notmuch-show-get-mime-type-of-application/octet-stream part))
 	(and (string= content-type "inline patch")
@@ -988,7 +990,7 @@ (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
 HIDE determines whether to show or hide the part and the button
 as follows: If HIDE is nil, show the part and the button. If HIDE
 is t, hide the part initially and show the button."
-  (let* ((content-type (downcase (plist-get part :content-type)))
+  (let* ((content-type (plist-get part :content-type))
 	 (mime-type (notmuch-show-mime-type part))
 	 (nth (plist-get part :id))
 	 (long (and (notmuch-match-content-type mime-type "text/*")
@@ -1000,7 +1002,8 @@ (defun notmuch-show-insert-bodypart (msg part depth &optional hide)
 	 ;; the first (or only) part if this is text/plain.
 	 (button (and (funcall notmuch-show-insert-header-p-function part hide)
 		      (notmuch-show-insert-part-header
-		       nth mime-type content-type
+		       nth mime-type
+		       (and content-type (downcase content-type))
 		       (plist-get part :filename))))
 	 ;; Hide the part initially if HIDE is t, or if it is too long
 	 ;; and we have a button to allow toggling.
-- 
2.30.0

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[David Edmondson]

On Sunday, 2021-01-10 at 19:47:18 +01, Jonas Bernoulli wrote:

> The fourth commit tries to address the issue raised in
> id:87k0sxa6c3.fsf@fifthhorseman.net.
>
>   The output of "notmuch show --format=sexp --format-version=4"
>   may contain `:content-type' entries with `nil' as the value,
>   when it fails to detect the correct value.  Account for that
>   in a few places where we would otherwise risk a type error.
>
> The other three commits cleanup the code I had to look at to do
> the above, because I just can't help myself.
>
> This does not apply without first applying
> id:20201214162401.19569-1-jonas@bernoul.li.

I don't seem to have this one, but that could be a problem at my end.

Anyway, for the series...

Reviewed-by: David Edmondson <dme@dme.org>

>
>      Jonas
>
> Jonas Bernoulli (4):
>   emacs: notmuch-mua-add-more-hidden-headers: use local binding
>   emacs: notmuch-show--register-cids: fix names of bindings
>   emacs: notmuch-show--get-cid-content: cosmetics
>   emacs: avoid type errors due to nil as content-type
>
>  emacs/notmuch-lib.el  | 23 ++++++++++++----------
>  emacs/notmuch-mua.el  |  7 +++----
>  emacs/notmuch-show.el | 46 +++++++++++++++++++++----------------------
>  3 files changed, 39 insertions(+), 37 deletions(-)
>
> -- 
> 2.30.0
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

dme.
-- 
It was a perfectly good grand piano.

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[Jonas Bernoulli]

David Edmondson <dme@dme.org> writes:

> On Sunday, 2021-01-10 at 19:47:18 +01, Jonas Bernoulli wrote:
>
>> The fourth commit tries to address the issue raised in
>> id:87k0sxa6c3.fsf@fifthhorseman.net.
>>
>>   The output of "notmuch show --format=sexp --format-version=4"
>>   may contain `:content-type' entries with `nil' as the value,
>>   when it fails to detect the correct value.  Account for that
>>   in a few places where we would otherwise risk a type error.
>>
>> The other three commits cleanup the code I had to look at to do
>> the above, because I just can't help myself.
>>
>> This does not apply without first applying
>> id:20201214162401.19569-1-jonas@bernoul.li.
>
> I don't seem to have this one, but that could be a problem at my end.

I am sending you a copy of the original cover letter and
I have send v2 of the series earlier today, starting at
id:20210110140112.25930-1-jonas@bernoul.li.

> Anyway, for the series...
>
> Reviewed-by: David Edmondson <dme@dme.org>
>
>>
>>      Jonas
>>
>> Jonas Bernoulli (4):
>>   emacs: notmuch-mua-add-more-hidden-headers: use local binding
>>   emacs: notmuch-show--register-cids: fix names of bindings
>>   emacs: notmuch-show--get-cid-content: cosmetics
>>   emacs: avoid type errors due to nil as content-type
>>
>>  emacs/notmuch-lib.el  | 23 ++++++++++++----------
>>  emacs/notmuch-mua.el  |  7 +++----
>>  emacs/notmuch-show.el | 46 +++++++++++++++++++++----------------------
>>  3 files changed, 39 insertions(+), 37 deletions(-)
>>
>> -- 
>> 2.30.0
>> _______________________________________________
>> notmuch mailing list -- notmuch@notmuchmail.org
>> To unsubscribe send an email to notmuch-leave@notmuchmail.org
>
> dme.
> -- 
> It was a perfectly good grand piano.

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[Tomi Ollila]

On Sun, Jan 10 2021, Jonas Bernoulli wrote:

> The fourth commit tries to address the issue raised in
> id:87k0sxa6c3.fsf@fifthhorseman.net.
>
>   The output of "notmuch show --format=sexp --format-version=4"
>   may contain `:content-type' entries with `nil' as the value,
>   when it fails to detect the correct value.  Account for that
>   in a few places where we would otherwise risk a type error.
>
> The other three commits cleanup the code I had to look at to do
> the above, because I just can't help myself.
>
> This does not apply without first applying
> id:20201214162401.19569-1-jonas@bernoul.li.

Perhaps this series still applies on top of that series from 2020-12-14,
but perhaps one should apply it on top of series (from 2021-01-10):

id:20210110140112.25930-2-jonas@bernoul.li

Meaning emails in range

id:20210110140112.25930-2-jonas@bernoul.li 
... 
id:20210110140112.25930-37-jonas@bernoul.li

(also visible in https://nmbug.notmuchmail.org/status/)

Note that messages 

20210110140112.25930-11-jonas@bernoul.li and
20210110140112.25930-12-jonas@bernoul.li

have base64 -encoded content, with CRLF line endings
so those don't apply as is, but CR's from the encoded
content must be deleted.

I used a hack to do so, appended to the end of this
email; David has done something that is more robust,
and perhaps from that we could get a tool to be added
in devel/ which could do this in the future.

Both of these series applied cleanly, and I will
start using those in my emacs mua.

I like these changes and hope these can be applied soon.

>
>      Jonas

Tomi

Usage: ./rmcr-base64.pl < input.mbox > output.mbox

--8<----8<----8<----8<----8<----8<----8<----8<--

#!/usr/bin/perl
# -*- mode: cperl; cperl-indent-level: 4 -*-

use 5.8.1;
use strict;
use warnings;
use MIME::Base64;

while (<STDIN>) {
    print $_;
    if (/^Content-Transfer-Encoding: base64/) {
	while (<STDIN>) { print $_; last if /^\s*$/ }
	my @lines;
	while (<STDIN>) { last if /^\s*$/; push @lines, $_ }
	my $decoded = decode_base64 join('', @lines);
	$decoded =~ tr/\r//d;
	print(encode_base64 $decoded);
	print "\n"
    }
}

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[Jonas Bernoulli]

Tomi Ollila <tomi.ollila@iki.fi> writes:
> Note that messages 
>
> 20210110140112.25930-11-jonas@bernoul.li and
> 20210110140112.25930-12-jonas@bernoul.li
>
> have base64 -encoded content, with CRLF line endings
> so those don't apply as is, but CR's from the encoded
> content must be deleted.

Do you know why git would decide to send these commits that way?
Maybe we should try to keep it from doing that?  I didn't write
down the prompt but git actually asked something along the line
of "for these two messages you have to select the base64 encoding,
does utf-8 work for you?".

     Jonas

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[Tomi Ollila]

On Tue, Jan 12 2021, Jonas Bernoulli wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>> Note that messages 
>>
>> 20210110140112.25930-11-jonas@bernoul.li and
>> 20210110140112.25930-12-jonas@bernoul.li
>>
>> have base64 -encoded content, with CRLF line endings
>> so those don't apply as is, but CR's from the encoded
>> content must be deleted.
>
> Do you know why git would decide to send these commits that way?

I do know:

In my case:

- git format-patch ...

- in one of the patch files there are utf-8 characters

- git send-email (to mailing list and to tomi.ollila@iki.fi)

- first stop in email is in smtp.iki.fi (which I use to send email)

- the email that returns directly from iki.fi does not have base64
  encoded content *nor* CR's in email body

- the mail that is sent to notmuchmail.org, will contain CRLF line
  endings; either smtp.iki.fi adds those or notmuchmail.org smtpd
  added those

- the mailman in notmuchmail.org notices that there is 8bit content
  in email body, and decides to base64-encode the whole content
  with CRLF line endings.

- git am, the --no-keep-cr option (default) if not effective in 
  "embedded" base64-encoded content. That particular feature is
  just for that purpose that smtp content should have CRLF line
  endings and the files those patches refer to often don't have
  (--keep-cr can be used if CRLF file endings are expected). It
  is a "feature" that in base64-encoded content is handled as
  verbatim by git-am, git-apply and so on.

For reference: the email 

https://www.mail-archive.com/notmuch@notmuchmail.org/msg50113.html

got directly (via-iki-smtpd) and via mailing list (via-notmuch-mailman)
has the following headers (among other headers) in my mail storage
(and as said above, the one got directly did not have CRLF line endings)

  $ grep Content via-iki-smtpd via-notmuch-mailman

  via-iki-smtpd:Content-Type: text/plain; charset=UTF-8
  via-iki-smtpd:Content-Transfer-Encoding: 8bit

  via-notmuch-mailman:Content-Type: text/plain; charset="utf-8"
  via-notmuch-mailman:Content-Transfer-Encoding: base64


> Maybe we should try to keep it from doing that?  I didn't write
> down the prompt but git actually asked something along the line
> of "for these two messages you have to select the base64 encoding,
> does utf-8 work for you?".

Right, the base64 encoding can be used when using git send-email, but how
many knows to do that :) (Tried to look what option is it, but 
man git-send-email gave me manpage of version 1.7.1 >;/).


Other options:

- modify mailman (David Bremner contacted mailman mailing list, they don't
  think that is a bug (well IMO is it not a bug, but...) anyway one could
  hack locally ;/ (or the smtpd frontend if there is any)

- modify git-am (based on Julio Hamano's comment on some mailing list
  something like if there is clear and sound implementation -- was it
  some filter option then something could get in... (or something))

- based on irclogs (2020-08) David has tool to be offered to 
  (debian)-mailscripts to filter CR's out from base64-encoded content.
  -- which I think is better place as this problem isn't notmuch specific
  (although having such a tool in devel/ would be more convenient for me)

>
>      Jonas

Tomi

Re: [PATCH 0/4] emacs: avoid type errors due to nil as content-type

[David Bremner]

Jonas Bernoulli <jonas@bernoul.li> writes:

> The fourth commit tries to address the issue raised in
> id:87k0sxa6c3.fsf@fifthhorseman.net.

Series applied to master