Hook to process message before display/reply? #571

larskotthoff · 2018-10-10T21:15:33Z

One of my email providers inserts a warning into the body of the email if certain conditions are met. This is annoying and completely useless, and worse shows up in the quoted text when I reply.

It would be great to have a hook before astroid reads a mail that allows to pipe it through a script. I could remove the warning as if it never existed this way, without having to change mails on the server.

gauteh · 2018-10-11T09:23:07Z

You can use plugins to pre-process for viewing. See the syntax-highlight plugin for an example. This does not change it when including it in the reply though, but we could allow pre-processing here as well.

…

On Wed, Oct 10, 2018 at 11:15 PM Lars Kotthoff ***@***.***> wrote: One of my email providers inserts a warning into the body of the email if certain conditions are met. This is annoying and completely useless, and worse shows up in the quoted text when I reply. It would be great to have a hook before astroid reads a mail that allows to pipe it through a script. I could remove the warning as if it never existed this way, without having to change mails on the server. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#571>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADd-6M_SqqKc0Ai35PHvwl1Vxabvt3uks5ujmN1gaJpZM4XWWSx> .

larskotthoff · 2018-10-11T15:17:49Z

Thanks, I'll have a look at that. It would be great if you could include something that allows this to hook in before a reply.

larskotthoff · 2018-10-11T22:37:22Z

It looks like if I return the plain/text part of a mail from a plugin all the newlines are gone. This happens even if I return the text part verbatim without changes:

  def do_filter_part(self, text, html, mime_type, is_patch):
    if mime_type == 'text/plain':
      return text

Is this a bug or am I missing something?

larskotthoff · 2018-10-11T22:43:15Z

Also, is there a way to modify the headers?

gauteh · 2018-10-12T04:43:52Z

Lars Kotthoff writes on October 12, 2018 0:37:

It looks like if I return the plain/text part of a mail from a plugin all the newlines are gone. This happens even if I return the text part verbatim without changes: def do_filter_part(self, text, html, mime_type, is_patch): if mime_type == 'text/plain': return text Is this a bug or am I missing something?

You have to return HTML. The html argument is the GMime filtered and escaped HTML (so it is pretty safe), this is what is usually displayed, use this if possible. The html vs text is related line by line (as far as I can remember). You can probably also access GMimes filters via python if you need to re-encode later. The documentation (in the wiki) is probably a bit sparse, feel free to elaborate as you go along :) It might be useful to provide a `filter_message` or another `filter_part` with perhaps a context argument. This could be used directly in message reading (chunk.cc / messagethread.cc) to pre-process the mail.

larskotthoff · 2018-10-12T16:08:39Z

Thanks, I was looking through the code and came to the same conclusion. I'm starting to think that I really want a plugin interface that allows to process the raw message.

gauteh · 2018-10-12T16:10:55Z

Lars Kotthoff writes on October 12, 2018 18:08:

Thanks, I was looking through the code and came to the same conclusion. I'm starting to think that I really want a plugin interface that allows to process the raw message.

Yes, the current interface is very ad-hoc for what I need. Could definetely be made much better organized and comprehensive! Contributions very welcome. - gaute

larskotthoff · 2018-10-14T02:44:51Z

Ok, I've had a go at implementing a plugin that returns a modified stream of the raw message (i.e. instead of astroid reading the message directly, it gets a stream from the plugin which is passed the file name). I can't get the plugin function to be called though -- the check whether there's a pointer to the function fails (pointer is NULL, https://github.com/larskotthoff/astroid/blob/master/src/plugin/astroid_activatable.c#L175). I did add it to the interface (https://github.com/larskotthoff/astroid/blob/master/src/plugin/astroid_activatable.h#L31), so I'm not sure what's going on.

It would be great if you could have a look please -- full code at https://github.com/larskotthoff/astroid, diff at larskotthoff@501384f. The python plugin has a function "do_process(self, fname)".

gauteh · 2018-10-16T17:16:26Z

Lars Kotthoff writes on October 14, 2018 4:44:

Ok, I've had a go at implementing a plugin that returns a modified stream of the raw message (i.e. instead of astroid reading the message directly, it gets a stream from the plugin which is passed the file name). I can't get the plugin function to be called though -- the check whether there's a pointer to the function fails (pointer is NULL, https://github.com/larskotthoff/astroid/blob/master/src/plugin/astroid_activatable.c#L175). I did add it to the interface (https://github.com/larskotthoff/astroid/blob/master/src/plugin/astroid_activatable.h#L31), so I'm not sure what's going on. It would be great if you could have a look please -- full code at https://github.com/larskotthoff/astroid, diff at larskotthoff@501384f. The python plugin has a function "do_process(self, fname)".

I'll see if I get the chance ASAP. Looked briefly through, a few things: The comment describing the function has meaning to gobject-introspection. So makes sure the GMimeStream* object is passed along as it should. Maybe a hint can be found in the the GMime introspection? Not sure, but I think this could result in mismatching function declarations. Also, make sure you set the GIR_TYPELIB path etc correctly so that the new gir libraries are found when you run astroid and plugins. See example/astroid. Also make sure that your plugin is found! This should be visible in the debug output. You can just put some print statements in the main scope of the python file and it should be printed regardless of whether the function/class is matched to the interface. Do you have a minimal test plugin I could test?

gauteh · 2018-10-16T17:16:53Z

https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations

larskotthoff · 2018-10-17T02:35:19Z

Thanks, I've verified that the plugin is loaded by putting in print statements like in the example plugins. The problem is almost certainly caused by the introspection, but I can't figure out where. Below is a small example plugin to demonstrate.

import gi
gi.require_version ('Astroid', '0.2')
gi.require_version ('Gtk', '3.0')
gi.require_version ('GMime', '2.6')

from gi.repository import GObject
from gi.repository import Gtk
from gi.repository import Astroid
from gi.repository import GMime

import email
import os
import mimetypes

from email.policy import default

class FooPlugin(GObject.Object, Astroid.Activatable):
  def do_activate(self):
    print('Foo: activated', __file__)

  def do_deactivate(self):
    print('Foo: deactivated')

  def do_process(self, fname):
    print("Processing " + fname)
    with open(fname, 'rb') as fp:
      msg = email.message_from_binary_file(fp, policy = default)
      for pt in msg.walk():
        if pt.get_content_type() == 'text/plain':
            pt.set_content("Lorem ipsum")

      return GMime.StreamMem.new_with_buffer(msg.as_bytes())

larskotthoff · 2018-10-17T02:44:35Z

And of course I discover the problem as soon as I've posted this -- the transfer annotation needs to be "full"...

gauteh · 2018-10-17T07:01:21Z

Lars Kotthoff writes on October 17, 2018 4:44:

And of course I discover the problem as soon as I've posted this -- the transfer annotation needs to be "full"...

Nice, I'll take a look soon. I can imagine this will be very useful in converting HTML to text when replying or forwarding e-mail, but that would be dependent on context. Maybe a context flag should be added. The Message::viewable_text should probably be moved to ::quote or something, I think the only place it is used is in reply/forward. So then the Message-object will know at process time. Related to #532 and #545. Does the memory mangement work out correctly (ref count)? Is the GMimeStream cleared up? I assume this happens in Message::?

larskotthoff · 2018-10-17T16:18:07Z

Ah, good question -- it looks like there's lots of stray WebKit processes now. I'll have a look at that.

Regarding HTML to text; my plugin is adding an alternative text/plain part to the email if it's HTML-only, which I believe should work fine in all cases without a context flag. That said, there seems to be a bug either in the Python email processing library or astroid that doesn't delineate the new part correctly -- when I open an email, it shows the text and that there's an HTML part, but when I open the HTML part it is merged with the text part into one email that doesn't show separate parts anymore.

gauteh · 2018-10-17T16:34:46Z

Lars Kotthoff writes on October 17, 2018 18:18:

Ah, good question -- it looks like there's lots of stray WebKit processes now. I'll have a look at that.

Hm, weird. I don't see how that could be related?

Regarding HTML to text; my plugin is adding an alternative text/plain part to the email if it's HTML-only, which I believe should work fine in all cases without a context flag.

True, that would be a good solution!

That said, there seems to be a bug either in the Python email processing library or astroid that doesn't delineate the new part correctly -- when I open an email, it shows the text and that there's an HTML part, but when I open the HTML part it is merged with the text part into one email that doesn't show separate parts anymore.

You probably have to change the structure to be "multipart/alternative"s. This is almost the only real-world use case of "multipart/" (except PGP, attachments, EFAIL, etc). Otherwise, astroid will think that the parts are independent and that both should be rendered, rather than the highest order, preferred type. Make sure that attachments etc. are not lost inside the original part. They should probably be part of the original structure. It might also be a good idea to not do this for encrypted or signed mail. This plugin would be very interesting for other users, and should definetely be included in the plugin-index [0] (if not also in the astroidmail org if you are interested). [0] https://github.com/astroidmail/astroid-plugins

larskotthoff · 2018-10-17T17:03:21Z

From my reading of the documentation, the python email library should take care of automatically converting it to multipart/alternative when I add the text/plain as an alternative.

I'll definitely make a skeleton version of the plugin available once I've worked everything out.

gauteh · 2018-10-17T17:10:22Z

Lars Kotthoff writes on October 17, 2018 19:03:

From my reading of the documentation, the python email library should take care of automatically converting it to multipart/alternative when I add the text/plain as an alternative.

Ok, hm. Maybe you can get some hints from the output from chunk.cc when it is parsing. Or maybe just print a tree in the plugin before returning.

I'll definitely make a skeleton version of the plugin available once I've worked everything out.

Ok!

larskotthoff · 2018-10-18T02:44:40Z

Ok, figured out the weirdness with the alternative part -- I was adding it as an alternative to the entire multipart/related message instead of just the HTML part.

As far as I can tell, the memory management should work correctly. I've tried to check this using https://stackoverflow.com/questions/24453266/check-if-a-gobject-was-correctly-freed but that fails even without the plugin (i.e. the refcount to stream at the end isn't 0). When I try to free it explicitly I get a core dump because of a double free though.

gauteh · 2018-10-18T08:52:06Z

Lars Kotthoff writes on October 18, 2018 4:44:

Ok, figured out the weirdness with the alternative part -- I was adding it as an alternative to the entire multipart/related message instead of just the HTML part.

Nice!

As far as I can tell, the memory management should work correctly. I've tried to check this using https://stackoverflow.com/questions/24453266/check-if-a-gobject-was-correctly-freed but that fails even without the plugin (i.e. the refcount to `stream` at the end isn't 0). When I try to free it explicitly I get a core dump because of a double free though.

Probably the GMimeMessage now holds a reference to it. I think it looks correct from the code, with transfer: full it should be unref'ed in that function when done with it. The GMimeMessage should be cleared up in ~Message. It is sometimes confusing with how GObject refptr's work compared to C++ smart ptrs' when they operate with the ownership concept together with refptr concept (requiring intial reference count or not...). They integrate very well with GTK and sigc, but I would like to move to C++ smartptrs where possible (#570).

gauteh · 2018-10-19T13:11:33Z

I have been thinking about the use case when you need to quote for replying. We can add a HTML part when sending by using a processor like markdown now. So one way to quote HTML is to convert it to markdown, this should give a reply that looks a bit more like the original. It does of course only support a subset of HTML. I do not think this needs any special consideration except that the user has to remember to flick on the Markdown processor. The other thing I was thinking about is that I want to use this feature only when quoting, not when reading e-mail normally. It presumably also causes a bit of overhead. I think that we therefore need an additional `process()` which is only used when `quoting`. If you want to use the same for both just wrap one to the other in your plugin. What do you think? It could be named just `quote()` since users might use it for other things like double-signature removing, etc..

larskotthoff · 2018-10-19T15:49:27Z

This could be useful yes. I've noticed a bug with encodings in my current plugin; when saving in the external editor non-ASCII characters get mangled up in the view in Astroid. Not sure what the problem is yet.

There's also a bunch of other functionality that I want to implement. I'll focus on that first before coming back to separating processing for viewing and quoting.

gauteh · 2018-10-19T16:21:28Z

Lars Kotthoff writes on October 19, 2018 17:49:

This could be useful yes. I've noticed a bug with encodings in my current plugin; when saving in the external editor non-ASCII characters get mangled up in the view in Astroid. Not sure what the problem is yet.

compose_message.cc reads the message through viewable_text() which essentially just concatenates all plain-text parts. So if you modify the structure this might be a problem. This is also a place where we do not want the processing code to take effect actually. I would not be surprised if this pops up as bug pretty soon after someone using the process hook.. :/

There's also a bunch of other functionality that I want to implement. I'll focus on that first before coming back to separating processing for viewing and quoting.

👍

larskotthoff · 2018-10-20T04:18:21Z

I've looked into this further, but I can't track down the bug. Somewhere the encoding is messed up, but I don't see where -- there are temporary files written and read in ~/.cache/astroid/ and /tmp/astroid-*, and processing the former (though not the latter) in the plugin causes the problem. For now I'm simply checking the path of the file to load in the plugin and if it's in ~/.cache/astroid/ I don't to the processing.

No idea what's going on, as all reading/writing and processing is using the exact same code, but the issue only occur in some cases.

gauteh · 2018-10-20T06:06:07Z

Lars Kotthoff writes on October 20, 2018 6:18:

I've looked into this further, but I can't track down the bug. Somewhere the encoding is messed up, but I don't see where -- there are temporary files written and read in `~/.cache/astroid/` and `/tmp/astroid-*`, and processing the former (though not the latter) in the plugin causes the problem. For now I'm simply checking the path of the file to load in the plugin and if it's in `~/.cache/astroid/` I don't to the processing.

Email encoding is hard to get right. You can have both content encoding and content transfer encoding. You typically have to use something like quoted-printable to be able to take your regular encoding down to something which is supported by the raw email. I remember you set the transfer-encoding manually in your example? It can usually only have a few valid values: https://en.wikipedia.org/wiki/MIME#Content-Transfer-Encoding I would think that anything in ~/.cache/astroid and in /tmp/astroid-* should not be processed: these are used in the composition chain. In .cache the pseudo-email which is seen in the composer is stored, while the message is finalized (bunch of headers added, etc) and then written to /tmp before it is loaded for preview. The first message is not necessarily a valid message, GMime might guess that it is US-ASCII for instance.

No idea what's going on, as all reading/writing and processing is using the exact same code, but the issue only occur in some cases.

Is there any reason why you want to process the messages in the composition chain?

gauteh · 2018-10-20T06:22:53Z

Gaute Hope writes on October 20, 2018 8:06:

You can have both content encoding and content transfer encoding. You typically have to use something like quoted-printable to be able to take your regular encoding down to something which is supported by the raw email.

The charset is set in compose_message.cc:133, while the content-transfer-encoding is set depending on whether you have attachments or not. In EditMessage the message is read, processed, written to /tmp and re-read for preview in read_edited_message. prepare_message is the opposite which prepares a message for editing in the editor. setup_message in EditMessage does the initial reading, while the rest of the processing is done in compose_message->build/finalize.

gauteh · 2018-10-20T07:59:47Z

Lars Kotthoff writes on October 20, 2018 6:18:

I've looked into this further, but I can't track down the bug. Somewhere the encoding is messed up, but I don't see where -- there are temporary files written and read in `~/.cache/astroid/` and `/tmp/astroid-*`, and processing the former (though not the latter) in the plugin causes the problem. For now I'm simply checking the path of the file to load in the plugin and if it's in `~/.cache/astroid/` I don't to the processing.

The write to /tmp could be avoided using GMimeStreamMem, in #578 I've done this. Do you have the chance to review it?

larskotthoff · 2018-10-20T18:39:06Z

Thanks, it sounds like the reason is that the file in ~/.cache isn't a complete email. Is there an easy way of checking whether one of the temporary files is processed to avoid the processing (other than hard-coding the ~/.config path)? As far as I can see this would require adding a flag to a bunch of functions throughout and that seems rather heavy-handed to me.

#578 looks good, but I don't think it'll help with this since the problem is the other temporary file.

gauteh · 2018-10-20T18:46:33Z

Lars Kotthoff writes on October 20, 2018 20:39:

Thanks, it sounds like the reason is that the file in ~/.cache isn't a complete email. Is there an easy way of checking whether one of the temporary files is processed to avoid the processing (other than hard-coding the ~/.config path)? As far as I can see this would require adding a flag to a bunch of functions throughout and that seems rather heavy-handed to me.

Maybe, it's the `Message msg (..)` on line 263 in compose_message.cc which causes you problems. I don't think this should be included in `process()`.

#578 looks good, but I don't think it'll help with this since the problem is the other temporary file.

Ok, yes, just thought it would be good to avoid another tmp file.

larskotthoff mentioned this issue Oct 17, 2018

More flexible plugin API that allows to process entire message #577

Merged

larskotthoff closed this as completed Oct 18, 2018

gauteh mentioned this issue Oct 28, 2018

process message: process message plugin should not always be applied #587

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hook to process message before display/reply? #571

Hook to process message before display/reply? #571

larskotthoff commented Oct 10, 2018

gauteh commented Oct 11, 2018 via email

larskotthoff commented Oct 11, 2018

larskotthoff commented Oct 11, 2018

larskotthoff commented Oct 11, 2018

gauteh commented Oct 12, 2018 via email

larskotthoff commented Oct 12, 2018

gauteh commented Oct 12, 2018 via email

larskotthoff commented Oct 14, 2018

gauteh commented Oct 16, 2018 via email

gauteh commented Oct 16, 2018

larskotthoff commented Oct 17, 2018

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 18, 2018

gauteh commented Oct 18, 2018 via email

gauteh commented Oct 19, 2018 via email

larskotthoff commented Oct 19, 2018

gauteh commented Oct 19, 2018 via email

larskotthoff commented Oct 20, 2018

gauteh commented Oct 20, 2018 via email

gauteh commented Oct 20, 2018 via email

gauteh commented Oct 20, 2018 via email

larskotthoff commented Oct 20, 2018

gauteh commented Oct 20, 2018 via email

Hook to process message before display/reply? #571

Hook to process message before display/reply? #571

Comments

larskotthoff commented Oct 10, 2018

gauteh commented Oct 11, 2018 via email

larskotthoff commented Oct 11, 2018

larskotthoff commented Oct 11, 2018

larskotthoff commented Oct 11, 2018

gauteh commented Oct 12, 2018 via email

larskotthoff commented Oct 12, 2018

gauteh commented Oct 12, 2018 via email

larskotthoff commented Oct 14, 2018

gauteh commented Oct 16, 2018 via email

gauteh commented Oct 16, 2018

larskotthoff commented Oct 17, 2018

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 17, 2018

gauteh commented Oct 17, 2018 via email

larskotthoff commented Oct 18, 2018

gauteh commented Oct 18, 2018 via email

gauteh commented Oct 19, 2018 via email

larskotthoff commented Oct 19, 2018

gauteh commented Oct 19, 2018 via email

larskotthoff commented Oct 20, 2018

gauteh commented Oct 20, 2018 via email

gauteh commented Oct 20, 2018 via email

gauteh commented Oct 20, 2018 via email

larskotthoff commented Oct 20, 2018

gauteh commented Oct 20, 2018 via email