failure in emacs notmuch-show: notmuch-show--register-cids: Wrong type argument: char-or-string-p, nil

failure in emacs notmuch-show: notmuch-show--register-cids: Wrong type argument: char-or-string-p, nil

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 1568 bytes --]

I'm trying to look at an encrypted+signed PGP/MIME message in emacs.

it's in a thread with other signed messages.

but none of the contents of the message show up, and *Messages* buffer
says:

    notmuch-show--register-cids: Wrong type argument: char-or-string-p, nil

If i position the cursor in the notmuch show buffer where the message
would be, and try to reply to it, i end up composing a reply to the
previous message in the thread.

My elisp is too weak to know how to debug this well.  suggestions
welcome!

I should note that interacting with the message via the command line
shows no apparent problems.

not sure whether this is useful, but the structure of the message looks
like this:

~~~
0 dkg@alice:~$ notmuch show --decrypt=false --format=raw id:$messageid  | email-print-mime-structure --use-gpg-agent
└┬╴multipart/encrypted 27703 bytes
 ├─╴application/pgp-encrypted 11 bytes
 └─╴application/octet-stream inline [encrypted.asc] 23828 bytes
  ↧ (decrypts to)
  └┬╴multipart/mixed 26085 bytes
   ├─╴text/plain 1028 bytes
   └┬╴message/rfc822 attachment [attachment.eml] 24707 bytes
    └─╴text/plain 24510 bytes
0 dkg@alice:~$ 
~~~

The "attachment.eml" subtree, fwiw, is a forwarded message that i
already have a copy of in my local message store, and is part of the
same thread.  Maybe that has something to do with it?  I'm grasping at
straws here.

Any help would be appreciated, it'd be nice to be able to easily read
and reply to this message!

      --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

Re: failure in emacs notmuch-show: notmuch-show--register-cids: Wrong type argument: char-or-string-p, nil

[Jonas Bernoulli]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> My elisp is too weak to know how to debug this well.  suggestions
> welcome!

"M-x toggle-debug-on-error" and then trying to show the message should
give you a backtrace.  But before doing that also "M-x eval-buffer" in
"notmuch-show.el" to make it more meaningful.

>     notmuch-show--register-cids: Wrong type argument:
>     char-or-string-p, nil

With only that information my guess is that
  (plist-get part :content-type)
returns nil, which "downcase" understandably isn't happy with.

The "part" plist comes from "notmuch show ..." in
"notmuch-query-get-threads", so one problem seems to be that that
can return nil as the type (as opposed to e.g. "unknown/unknown")
while this elisp function (and maybe others) expect a string.

> 0 dkg@alice:~$ notmuch show --decrypt=false --format=raw id:$messageid  | email-print-mime-structure --use-gpg-agent
> └┬╴multipart/encrypted 27703 bytes
>  ├─╴application/pgp-encrypted 11 bytes
>  └─╴application/octet-stream inline [encrypted.asc] 23828 bytes
>   ↧ (decrypts to)
>   └┬╴multipart/mixed 26085 bytes
>    ├─╴text/plain 1028 bytes
>    └┬╴message/rfc822 attachment [attachment.eml] 24707 bytes
>     └─╴text/plain 24510 bytes

And another problem seems to be that notmuch cannot determine the type
of some part even though "email-print-mime-structure" can.  You should
be able to figure out which part by adding a debug statement such as:

  (message "> %S" part)

     Cheers,
     Jonas

Re: failure in emacs notmuch-show: notmuch-show--register-cids: Wrong type argument: char-or-string-p, nil

[Daniel Kahn Gillmor]

[-- Attachment #1.1.1: Type: text/plain, Size: 3106 bytes --]

Hi Jonas--

Thanks for the followup!  i got the backtrace.

On Sat 2021-01-02 10:47:44 +0100, Jonas Bernoulli wrote:
>>     notmuch-show--register-cids: Wrong type argument:
>>     char-or-string-p, nil
>
> With only that information my guess is that
>   (plist-get part :content-type)
> returns nil, which "downcase" understandably isn't happy with.

According to the backtrace, this is exactly right.  the error is in
(downcase nil).

> The "part" plist comes from "notmuch show ..." in
> "notmuch-query-get-threads", so one problem seems to be that that
> can return nil as the type (as opposed to e.g. "unknown/unknown")
> while this elisp function (and maybe others) expect a string.
>
>> 0 dkg@alice:~$ notmuch show --decrypt=false --format=raw id:$messageid  | email-print-mime-structure --use-gpg-agent
>> └┬╴multipart/encrypted 27703 bytes
>>  ├─╴application/pgp-encrypted 11 bytes
>>  └─╴application/octet-stream inline [encrypted.asc] 23828 bytes
>>   ↧ (decrypts to)
>>   └┬╴multipart/mixed 26085 bytes
>>    ├─╴text/plain 1028 bytes
>>    └┬╴message/rfc822 attachment [attachment.eml] 24707 bytes
>>     └─╴text/plain 24510 bytes

hm, this is interesting, upon further digging, the internal
message/rfc822 part is base64-encoded.  It has a
"Content-Transfer-Encoding: base64" header, despite
https://tools.ietf.org/html/rfc2046#section-5.2.1, which says:

   No encoding other than "7bit", "8bit", or "binary" is permitted for
   the body of a "message/rfc822" entity.

So perhaps this message is malformed within the encryption envelope.
Indeed, base64-decoding body of the attached message reveals that it has
significantly more structure than the "text/plain" claim made by
email-print-mime-structure above.

So i've generated a simple message with a base64-encoded message/rfc822
part (but without the cryptographic wrapper), which is attached in a zip
file below (i didn't want to attach it directly because it might break
rendering).

If you unzip it, and then:

    notmuch-insert < attached-message-b64-encoded.eml

then in emacs:

   M-x notmuch-search id:attached-message-b64-encoded@mailscripts.example

you should be able to replicate the failure.

I admit i don't really understand why RFC 2046 *doesn't* accept that a
message/rfc822 part might be base64-encoded, so i don't know whether the
right answer is for notmuch to try b64-decode the message subpart
itself, or to go ahead and ignore that "unpermitted"
Content-Transfer-Encoding value.

At any rate, whether this ends up being rendered as the sender intended
to or not, notmuch-show shouldn't choke when processing it.

Note that this has come up for a few other MUAs, including recently:

thunderbird: https://bugzilla.mozilla.org/show_bug.cgi?id=333880
  evolution: https://bugzilla.gnome.org/show_bug.cgi?id=651197
      gmime: https://gitlab.gnome.org/GNOME/gmime/-/issues/1#note_373164

Makes me think that some library or tool recently started mis-generating
this stuff, ugh.

          --dkg


[-- Attachment #1.1.2: attached-message-b64-encoded.zip --]
[-- Type: application/zip, Size: 926 bytes --]

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

[PATCH] emacs/notmuch-show: Work around errors where a part lacks a content-type

[Daniel Kahn Gillmor]

This is an inadequate workaround to the concerns raised in
id:87wnwu8tzf.fsf@fifthhorseman.net -- when it is installed, a
particular kind of malformed message (in particular, one containing a
message/rfc822 part that is improperly transfer-encoded with base64)
won't break the rendering.

However, with this applied, there are definitely still problems.

For example, the rendering of such a message shows internal errors for
me:

    [ attachment.eml: message/rfc822 ]

    !!! Bodypart handler `notmuch-show-insert-part-message/rfc822' threw an error:
    !!! Wrong type argument: char-or-string-p, nil
    !!! Bodypart handler `notmuch-show-insert-part-*/*' threw an error:
    !!! Symbol’s value as variable is void: gnus-newsgroup-charset

But it's better than causing the whole thread to fail to render.

I don't know what the right solution is, so i'm offering this
workaround in the spirit of harm reduction.

(note: this is on the "release" branch -- this function has changed in
master, so i don't think it applies there, but i haven't tested further)
---
 emacs/notmuch-show.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b08ceb97..625a5d55 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -575,7 +575,9 @@ message at DEPTH in the current thread."
       (push (list content-id msg part) notmuch-show--cids)))
   ;; Recurse on sub-parts
   (let ((ctype (notmuch-split-content-type
-		(downcase (plist-get part :content-type)))))
+		(downcase (if (plist-get part :content-type)
+                              (plist-get part :content-type)
+                            "text/plain")))))
     (cond ((equal (car ctype) "multipart")
 	   (mapc (apply-partially #'notmuch-show--register-cids msg)
 		 (plist-get part :content)))
-- 
2.29.2\r

Re: [PATCH] emacs/notmuch-show: Work around errors where a part lacks a content-type

[Tomi Ollila]

On Sun, Jan 03 2021, Daniel Kahn Gillmor wrote:

> This is an inadequate workaround to the concerns raised in
> id:87wnwu8tzf.fsf@fifthhorseman.net -- when it is installed, a
> particular kind of malformed message (in particular, one containing a
> message/rfc822 part that is improperly transfer-encoded with base64)
> won't break the rendering.
>
> However, with this applied, there are definitely still problems.
>
> For example, the rendering of such a message shows internal errors for
> me:
>
>     [ attachment.eml: message/rfc822 ]
>
>     !!! Bodypart handler `notmuch-show-insert-part-message/rfc822' threw an error:
>     !!! Wrong type argument: char-or-string-p, nil
>     !!! Bodypart handler `notmuch-show-insert-part-*/*' threw an error:
>     !!! Symbol’s value as variable is void: gnus-newsgroup-charset
>
> But it's better than causing the whole thread to fail to render.
>
> I don't know what the right solution is, so i'm offering this
> workaround in the spirit of harm reduction.
>
> (note: this is on the "release" branch -- this function has changed in
> master, so i don't think it applies there, but i haven't tested further)
> ---
>  emacs/notmuch-show.el | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index b08ceb97..625a5d55 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -575,7 +575,9 @@ message at DEPTH in the current thread."
>        (push (list content-id msg part) notmuch-show--cids)))
>    ;; Recurse on sub-parts
>    (let ((ctype (notmuch-split-content-type
> -		(downcase (plist-get part :content-type)))))
> +		(downcase (if (plist-get part :content-type)
> +                              (plist-get part :content-type)
> +                            "text/plain")))))

What I remember from someone's latest patches (was it Jonas or someone
else),

		(downcase (or (plist-get part :content-type) "text/plain"))))))

(if this were otherwise good enough to be applied :D)


Tomi

Re: [PATCH] emacs/notmuch-show: Work around errors where a part lacks a content-type

[Jonas Bernoulli]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Jan 03 2021, Daniel Kahn Gillmor wrote:
>
>> This is an inadequate workaround to the concerns raised in
>> id:87wnwu8tzf.fsf@fifthhorseman.net -- when it is installed, a
>> particular kind of malformed message (in particular, one containing a
>> message/rfc822 part that is improperly transfer-encoded with base64)
>> won't break the rendering.
>>
>> However, with this applied, there are definitely still problems.
>>
>> For example, the rendering of such a message shows internal errors for
>> me:
>>
>>     [ attachment.eml: message/rfc822 ]
>>
>>     !!! Bodypart handler `notmuch-show-insert-part-message/rfc822' threw an error:
>>     !!! Wrong type argument: char-or-string-p, nil
>>     !!! Bodypart handler `notmuch-show-insert-part-*/*' threw an error:
>>     !!! Symbol’s value as variable is void: gnus-newsgroup-charset
>>
>> But it's better than causing the whole thread to fail to render.
>>
>> I don't know what the right solution is, so i'm offering this
>> workaround in the spirit of harm reduction.
>>
>> (note: this is on the "release" branch -- this function has changed in
>> master, so i don't think it applies there, but i haven't tested further)
>> ---
>>  emacs/notmuch-show.el | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index b08ceb97..625a5d55 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -575,7 +575,9 @@ message at DEPTH in the current thread."
>>        (push (list content-id msg part) notmuch-show--cids)))
>>    ;; Recurse on sub-parts
>>    (let ((ctype (notmuch-split-content-type
>> -		(downcase (plist-get part :content-type)))))
>> +		(downcase (if (plist-get part :content-type)
>> +                              (plist-get part :content-type)
>> +                            "text/plain")))))
>
> What I remember from someone's latest patches (was it Jonas or someone
> else),
>
> 		(downcase (or (plist-get part :content-type) "text/plain"))))))

Yes, please.

> (if this were otherwise good enough to be applied :D)

There are other places in the elisp code where the `:content-type' is
assumed to be a string, so fixing it just here doesn't cut it.  To fix
it for everyone, "notmuch show..." should probably take care of falling
back to some sane default if the type cannot be determined.

     Jonas

Re: [PATCH] emacs/notmuch-show: Work around errors where a part lacks a content-type

[Daniel Kahn Gillmor]

[-- Attachment #1.1: Type: text/plain, Size: 1414 bytes --]

On Mon 2021-01-04 22:07:49 +0100, Jonas Bernoulli wrote:
> There are other places in the elisp code where the `:content-type' is
> assumed to be a string, so fixing it just here doesn't cut it.  To fix
> it for everyone, "notmuch show..." should probably take care of falling
> back to some sane default if the type cannot be determined.

the only two "sane defaults" would be either "text/plain" or
"application/octet-stream", but the circumstances in which they are
appropriate might differ.  e.g. a the top level of a message (or the top
level of a message/rfc822 subpart), the "sane default" would be
"text/plain" (because that's how messages by MIME-ignorant mailers look,
and they should still be readable text).  But a subpart without a
Content-Type designation should maybe default to
"application/octet-stream", or (even fancier) maybe a guess based on
other part headers, discovered filenames, or by sniffing content (yikes,
there be dragons).

All that said, i think fixing all the places in the elisp code is the
safer route, because the elisp code can't guarantee what the local
version of notmuch will do.  At the very least, if :content-type isn't a
string, notmuch-show shouldn't choke and break the rendering of the
thread.

So i think we should identify all the places where :content-type is
expected to be a string, and degrade gracefully if that is not how the
field is populated.

      --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

Re: [PATCH] emacs/notmuch-show: Work around errors where a part lacks a content-type

[Jonas Bernoulli]

I accidentally responded off-list.  Here is (part of) Daniel's response
to that message:

On Wed 2021-01-06 18:48:09 +0100, Jonas Bernoulli wrote:
> Okay, I'll prepare a patch that does that.

Thanks!

> I wasn't so much worried having to deal with this in multiple places in
> elisp--that's quite manageable--but about all the other front-ends that
> will also have to discover and address this issue over time.

yep, makes sense.  This might well be a "both and" situation instead of
an "either or".