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

classic Classic list List threaded Threaded
11 messages Options
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

The fourth commit tries to address the issue raised in
id:[hidden email].

  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:[hidden email].

     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 -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

---
 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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by 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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by 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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by 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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by Jonas Bernoulli
On Sunday, 2021-01-10 at 19:47:18 +01, Jonas Bernoulli wrote:

> The fourth commit tries to address the issue raised in
> id:[hidden email].
>
>   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:[hidden email].

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 <[hidden email]>

>
>      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 -- [hidden email]
> To unsubscribe send an email to [hidden email]

dme.
--
It was a perfectly good grand piano.
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

David Edmondson <[hidden email]> 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:[hidden email].
>>
>>   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:[hidden email].
>
> 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:[hidden email].

> Anyway, for the series...
>
> Reviewed-by: David Edmondson <[hidden email]>
>
>>
>>      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 -- [hidden email]
>> To unsubscribe send an email to [hidden email]
>
> dme.
> --
> It was a perfectly good grand piano.
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by Jonas Bernoulli
On Sun, Jan 10 2021, Jonas Bernoulli wrote:

> The fourth commit tries to address the issue raised in
> id:[hidden email].
>
>   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:[hidden email].

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:[hidden email]

Meaning emails in range

id:[hidden email]
...
id:[hidden email]

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

Note that messages

[hidden email] and
[hidden email]

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"
    }
}
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Jonas Bernoulli Jonas Bernoulli
Reply | Threaded
Open this post in threaded view
|

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

Tomi Ollila <[hidden email]> writes:
> Note that messages
>
> [hidden email] and
> [hidden email]
>
> 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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

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

On Tue, Jan 12 2021, Jonas Bernoulli wrote:

> Tomi Ollila <[hidden email]> writes:
>> Note that messages
>>
>> [hidden email] and
>> [hidden email]
>>
>> 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 [hidden email])

- 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@.../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
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

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

In reply to this post by Jonas Bernoulli
Jonas Bernoulli <[hidden email]> writes:

> The fourth commit tries to address the issue raised in
> id:[hidden email].

Series applied to master
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]