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

[Jonas Bernoulli <jonas@bernoul.li>]

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