Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello,

On Tue 03 Mar 2020 at 03:43PM -05, Daniel Kahn Gillmor wrote:

> I'd be happy to entertain this mode of operation for imap-dl if someone
> wants to offer a robust patch for it with a very minimal footprint in
> terms of configuration complexity :)
>
> In general, i'm happy for imap-dl to remain fairly opinionated about
> what workflows are "sensible" and not supporting too many others
> directly, given the combinatoric risks of adding little tweaky options
> to what's effectively a state machine manager.
>
> Users who want a super-flexible toolkit might be better off writing
> their own, using the existing libraries.

This makes sense to me.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

On Fri 2020-02-14 07:58:34 -0700, Sean Whitton wrote:
>>> Also, will imap-dl skip messages with the deleted flag?  Do you think it
>>> should?
>>
>> I don't think it should -- the use case at the moment is just to fetch
>> all messages that exist in the inbox.  Why should it treat any flag
>> differently?
>
> What I was thinking was that the user might want imap-dl to set the
> delete flag and not expunge, and then skip deleted messages on future
> runs.  Then they'd expunge themselves from time-to-time.
>
> This way, if imap-dl makes any mistakes, there is a sort of backup.
>
> I was particularly thinking that someone might want to use this at
> first, until they could feel sure that imap-dl doesn't have any bugs
> with their particular IMAP server.

I'd be happy to entertain this mode of operation for imap-dl if someone
wants to offer a robust patch for it with a very minimal footprint in
terms of configuration complexity :)

In general, i'm happy for imap-dl to remain fairly opinionated about
what workflows are "sensible" and not supporting too many others
directly, given the combinatoric risks of adding little tweaky options
to what's effectively a state machine manager.

Users who want a super-flexible toolkit might be better off writing
their own, using the existing libraries.

  --dkg


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

control: tag -1 +pending

Hello dkg,

On Fri 31 Jan 2020 at 05:43PM -05, Daniel Kahn Gillmor wrote:

> Thanks for the extensive review.  I've revised imap-dl, taking it into
> account, and have attached the revised version here.  You can also find
> it on my imap-dl-v2 branch on salsa.

Very nice.  Applied :)  Thank you.

> I've dug further into imaplib, and i've pushed the typeshed folks toward
> annotating imaplib further based on those findings.  We now expect the
> response to the uids() call to be a list of items that alternates
> between Tuple[bytes,bytes] and b')'.

This is definitely more maintainable.

>>> +fname = mdst.add(f[1].replace(b'\r\n', b'\n'))
>>
>> Could a message contain a mixture of UNIX and Windows line endings, such
>> that this line corrupts the message?  If not, please write a comment
>> saying why it is always safe to perform this replacement.
>
> I know of no way to have this create an actual corruption, unless the
> message itself doesn't actually have line endings at all (e.g. an 8-bit
> attachment in a MIME message) but i don't have anything like that handy
> and i've never seen it in practice.

Okay, cool.

>> Not a blocker, but it would be nice if the user could request that the
>> expunge step be skipped.
>
> this is a pretty subtle distinction -- you want to set the Deleted flag
> but not expunge?  can you describe the use case?
>
>> Also, will imap-dl skip messages with the deleted flag?  Do you think it
>> should?
>
> I don't think it should -- the use case at the moment is just to fetch
> all messages that exist in the inbox.  Why should it treat any flag
> differently?

What I was thinking was that the user might want imap-dl to set the
delete flag and not expunge, and then skip deleted messages on future
runs.  Then they'd expunge themselves from time-to-time.

This way, if imap-dl makes any mistakes, there is a sort of backup.

I was particularly thinking that someone might want to use this at
first, until they could feel sure that imap-dl doesn't have any bugs
with their particular IMAP server.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello,

On Sat 08 Feb 2020 at 12:37PM -05, Daniel Kahn Gillmor wrote:

> I don't understand what sort of rebase you are asking for -- the
> imap-dl-v2 branch on https://salsa.debian.org/dkg/mailscripts.git is
> (and been) based directly atop the imap-dl-squashed branch, so it's
> accessible with piecewise commits, with explanatory comments.
>
> But i'll send a squashed v3 patch as well :)

I looked at your branch but couldn't figure out how to get a diff of
your v2 patch against your v3 patch, which I wanted to compare against
my review comments.  Otherwise, it would definitely have been useful to
see the piecewise commits.

I'd like to suggest using branch names which correspond to versions of
the patch in the bug -- if I had seen imap-dl-v2 and imap-dl-v3 I would
have known immediately what to do.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

On Mon 2020-02-03 14:42:03 -0700, Sean Whitton wrote:
> On Fri 31 Jan 2020 at 05:43PM -05, Daniel Kahn Gillmor wrote:
>
>> Thanks for the extensive review.  I've revised imap-dl, taking it into
>> account, and have attached the revised version here.  You can also find
>> it on my imap-dl-v2 branch on salsa.
>
> Please send a v3 patch, or rebase your branch,

I don't understand what sort of rebase you are asking for -- the
imap-dl-v2 branch on https://salsa.debian.org/dkg/mailscripts.git is
(and been) based directly atop the imap-dl-squashed branch, so it's
accessible with piecewise commits, with explanatory comments.

But i'll send a squashed v3 patch as well :)

Thanks for reviewing!

--dkg

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello dkg,

On Fri 31 Jan 2020 at 05:43PM -05, Daniel Kahn Gillmor wrote:

> Thanks for the extensive review.  I've revised imap-dl, taking it into
> account, and have attached the revised version here.  You can also find
> it on my imap-dl-v2 branch on salsa.

Please send a v3 patch, or rebase your branch, so that I can compare
directly your v2 and v3 revisions of this work.  Right now it is
difficult for me to know which changes are in response to my review of
your v2 patch.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

Hi Sean--

Thanks for the extensive review.  I've revised imap-dl, taking it into
account, and have attached the revised version here.  You can also find
it on my imap-dl-v2 branch on salsa.

I've adopted all of the suggested revisions in pretty straightforward
ways except for the comments below.

On Thu 2020-01-23 12:24:31 -0700, Sean Whitton wrote:

> Is there really no documentation from the IMAP library you're using
> about the closeparens?

I've dug further into imaplib, and i've pushed the typeshed folks toward
annotating imaplib further based on those findings.  We now expect the
response to the uids() call to be a list of items that alternates
between Tuple[bytes,bytes] and b')'.

>> +fname = mdst.add(f[1].replace(b'\r\n', b'\n'))
>
> Could a message contain a mixture of UNIX and Windows line endings, such
> that this line corrupts the message?  If not, please write a comment
> saying why it is always safe to perform this replacement.

I know of no way to have this create an actual corruption, unless the
message itself doesn't actually have line endings at all (e.g. an 8-bit
attachment in a MIME message) but i don't have anything like that handy
and i've never seen it in practice.

> Maybe python has a e-mail message processing library with a function to
> transform line endings safely?

not that i know of, but if you want to point me to something specific
i'd be happy to adopt it.  I suppose we could round-trip the blob
through some function of email.message, but i'm not sure that's useful
or necessary.

>> +if delete:
>> +logging.info('trying to delete %d messages from IMAP store', 
>> len(fetched))
>> +resp = imap.uid('STORE', ','.join(map(str, fetched.keys())), 
>> '+FLAGS', r'(\Deleted)')
>> +if resp[0] != 'OK':
>> +raise Exception('failed to set \\Deleted flag: %s'%(resp))
>> +resp = imap.expunge()
>
> Not a blocker, but it would be nice if the user could request that the
> expunge step be skipped.

this is a pretty subtle distinction -- you want to set the Deleted flag
but not expunge?  can you describe the use case?

> Also, will imap-dl skip messages with the deleted flag?  Do you think it
> should?

I don't think it should -- the use case at the moment is just to fetch
all messages that exist in the inbox.  Why should it treat any flag
differently?

--dkg

#!/usr/bin/python3
# PYTHON_ARGCOMPLETE_OK
# -*- coding: utf-8 -*-

# Copyright (C) 2019 Daniel Kahn Gillmor
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or (at
# your option) any later version.
#
# This program is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
# General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see .

DESCRIPTION = '''A simple replacement for a minimalist use of getmail.

In particular, if you use getmail to reach an IMAP server as though it
were POP (retrieving from the server and optionally deleting), you can
point this script to the getmail config and it should do the same
thing.

It tries to ensure that the configuration file is of the expected
type, and will terminate raising an exception, and it should not lose
messages.

If there's any interest in supporting other similarly simple use cases
for getmail, patches are welcome.

If you've never used getmail, you can make the simplest possible
config file like so:

--
[retriever]
server = mail.example.net
username = foo
password = sekr1t!

[destination]
path = /home/foo/Maildir

[options]
delete = True
--
'''

import re
import sys
import ssl
import enum
import time
import imaplib
import logging
import mailbox
import os.path
import argparse
import statistics
import configparser

from typing import Dict, List, Union, Tuple

try:
import argcomplete #type: ignore
except ImportError:
argcomplete = None

class Splitter(object):
def __init__(self, name:str, match:bytes):
self._splitter = re.compile(match)
self._name = name
def breakup(self, line:bytes) -> Dict[str,int]:
match = self._splitter.match(line)
if not match:
raise Exception(f'malformed {self._name} line {line!r}')
ret:Dict[str,int] = {}
i:str
for i in ['id', 'uid', 'size']:
ret[i] = int(match[i])
return ret

OnSizeMismatch = enum.Enum('OnSizeMismatch', 'warn error none')

# b'1 (UID 160 RFC822.SIZE 1867)' -> {id: 1, uid: 160, size: 1867}
_summary_re = rb'^(?P[0-9]+) \(UID (?P[0-9]+) RFC822.SIZE (?P[0-9]+)\)$'
summary_splitter = Splitter('summary', _summary_re)
# b'1 (UID 160 BODY[] 

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello Daniel,

On Wed 22 Jan 2020 at 04:52PM -05, Daniel Kahn Gillmor wrote:

> The attached git-formatted patch is also present on the imap-dl-squashed
> branch on https://salsa.debian.org/dkg/mailscripts.  jrollins confirmed
> that it was OK, which is why it bears both of our signoffs.

Thanks for sorting that out.  Here are my comments on the current
version of imap-dl.  Looking forward to including it in mailscripts.

> diff --git a/imap-dl b/imap-dl
> new file mode 100755
> index 000..f5d7a85
> --- /dev/null
> +++ b/imap-dl
> +
> +_summary_splitter = re.compile(rb'^(?P[0-9]+) \(UID (?P[0-9]+) 
> RFC822.SIZE (?P[0-9]+)\)$')
> +def break_fetch_summary(line:bytes) -> Dict[str,int]:
> +'''b'1 (UID 160 RFC822.SIZE 1867)' -> {id: 1, uid: 160, size: 1867}'''
> +match = _summary_splitter.match(line)
> +if not match:
> +raise Exception(f'malformed summary line {line!r}')
> +ret:Dict[str,int] = {}
> +i:str
> +for i in ['id', 'uid', 'size']:
> +ret[i] = int(match[i])
> +return ret
> +
> +_fetch_splitter = re.compile(rb'^(?P[0-9]+) \(UID (?P[0-9]+) (FLAGS 
> \([\\A-Za-z ]*\) )?BODY\[\] \{(?P[0-9]+)\}$')
> +def break_fetch(line:bytes) -> Dict[str,int]:
> +'''b'1 (UID 160 BODY[] {1867}' -> {id: 1, uid: 160, size: 1867}'''
> +match = _fetch_splitter.match(line)
> +if not match:
> +raise Exception(f'malformed fetch line {line!r}')
> +ret:Dict[str,int] = {}
> +i:str
> +for i in ['id', 'uid', 'size']:
> +ret[i] = int(match[i])
> +return ret

These two functions are almost identical; please refactor.

> +if verbose: # only enable debugging after login to avoid leaking 
> credentials in the log
> +imap.debug = 4
> +logging.info("capabilities reported: %s", ', 
> '.join(imap.capabilities))

I am not familiar with idiomatic logging in Python, but shouldn't the
logging.info call be outside of the 'if' block, like all the others?

And shouldn't this be `if verbose or conf_verbose`?

> +if n == 0:
> +logging.info('No messages to retrieve')
> +logging.getLogger().setLevel(oldloglevel)
> +return

This is not a blocker, but it would be better to refactor such that
resetting the log level does not involve you repeating yourself here and
at the end of pull_msgs.

Can you use a 'with' keyword to temporarily set the loglevel, perhaps?
Or move the part of pull_msgs after config parsing into its own
subroutine.

> +uids = ','.join(map(lambda x: str(x['uid']), sorted(pending, 
> key=lambda x: x['uid'])))

This line could be shorter and easier to read, afaict.  How about:

from operator import itemgetter
uids = ','.join(map(str, sorted(map(itemgetter('uid'), pending

or

uids = ','.join(map(str, sorted([x['uid'] for x in pending])))

(untested)

> +for f in resp[1]:
> +# these objects are weirdly structured. i don't know why
> +# these trailing close-parens show up.  so this is very
> +# ad-hoc and nonsense
> +if isinstance(f, bytes):
> +if f != b')':
> +raise Exception('got bytes object of length %d but 
> expected simple closeparen'%(len(f),))
> +elif isinstance(f, tuple):

What is f is neither bytes nor tuple?  Should probably raise an
exception in that case.

Is there really no documentation from the IMAP library you're using
about the closeparens?

> +if on_size_mismatch == 'warn':
> +if len(sizes_mismatched) == 0:
> +logging.warning('size mismatch: summary said %d 
> octets, fetch sent %d',
> +sizes[m['uid']], m['size'])
> +elif len(sizes_mismatched) == 1:
> +logging.warning('size mismatch: (mismatches 
> after the first suppressed until summary)')
> +sizes_mismatched.append(sizes[m['uid']] - m['size'])
> +elif on_size_mismatch == 'exception':
> +raise Exception('size mismatch: summary said %d 
> octets, fetch sent %d\n(set options.on_size_mismatch to none or warn to avoid 
> hard failure)',
> +sizes[m['uid']], m['size'])
> +elif on_size_mismatch != 'none':
> +raise Exception('size_mismatch: 
> options.on_size_mismatch should be none, warn, or exception (found "%s")', 
> on_size_mismatch)

Checking whether size_mismatch contains the wrong sort of value should
not happen in the middle of the IMAP response processing code.  Please
move the check to where on_size_mismatch gets set.

Would be nice if you could avoid the really long lines here.

> +fname = mdst.add(f[1].replace(b'\r\n', b'\n'))

Could a message contain a mixture of UNIX and Windows line endings, such
that this line corrupts the message?  If not, 

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Jameson Graef Rollins]

On Wed, Jan 22 2020, Sean Whitton  wrote:
> On Wed 22 Jan 2020 at 05:01PM -08, Jameson Graef Rollins wrote:
>
>>> Signed-off-by: Jameson Graef Rollins 
>>> Signed-off-by: Daniel Kahn Gillmor 
>>
>> I confirm that I truly do sign off on this code, and fully support it's
>> inclusion in mailscripts.
>
> Could you confirm that you certify the contents of DEVELOPER-CERTIFICATE
> (in mailscripts.git, not just that you think the code is good?

Yes, I certify the mailscripts DEVELOPER-CERTIFICATE (attached) as well.

jamie.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.



signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello,

On Wed 22 Jan 2020 at 05:01PM -08, Jameson Graef Rollins wrote:

>> Signed-off-by: Jameson Graef Rollins 
>> Signed-off-by: Daniel Kahn Gillmor 
>
> I confirm that I truly do sign off on this code, and fully support it's
> inclusion in mailscripts.

Could you confirm that you certify the contents of DEVELOPER-CERTIFICATE
(in mailscripts.git, not just that you think the code is good?

Thanks.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Jameson Graef Rollins]

On Wed, Jan 22 2020, Daniel Kahn Gillmor  wrote:
> Hi Sean--
>
> On Fri 2020-01-17 09:26:38 -0700, Sean Whitton wrote:
>> I think the easiest thing to do would be for one of you to prepare a
>> single patch, signed off, and for the other to write an e-mail signing
>> it off.  I'll then do a code review of the latest version of the script.
>
> The attached git-formatted patch is also present on the imap-dl-squashed
> branch on https://salsa.debian.org/dkg/mailscripts.  jrollins confirmed
> that it was OK, which is why it bears both of our signoffs.
>
> Thanks for considering imap-dl for inclusion within mailscripts!
>
>  --dkg
>
> From 9e5c1a893c17343102b042de23bdaa0f91b37d66 Mon Sep 17 00:00:00 2001
> From: Daniel Kahn Gillmor 
> Date: Sun, 15 Sep 2019 19:55:07 -0400
> Subject: [PATCH] Add imap-dl, a simple imap downloader
>
> getmail upstream appears to have no plans to convert to python3 in the
> near future.
>
> Some of us use only a minimal subset of features of getmail, and it
> would be nice to have something simpler, with the main complexity
> offloaded to the modern python3 stdlib.
>
> This patch represents a squashed series of changes from both Jameson
> Graef Rollins and Daniel Kahn Gillmor (dkg), though dkg is primarily
> responsible for any remaining bugs.
>
> Signed-off-by: Jameson Graef Rollins 
> Signed-off-by: Daniel Kahn Gillmor 

I confirm that I truly do sign off on this code, and fully support it's
inclusion in mailscripts.

jamie.


> ---
>  Makefile   |   4 +-
>  debian/control |   2 +
>  debian/mailscripts.bash-completion |   1 +
>  debian/mailscripts.install |   1 +
>  debian/mailscripts.manpages|   1 +
>  imap-dl| 254 +
>  imap-dl.1.pod  |  88 ++
>  7 files changed, 350 insertions(+), 1 deletion(-)
>  create mode 100755 imap-dl
>  create mode 100644 imap-dl.1.pod
>
> diff --git a/Makefile b/Makefile
> index af30616..ec3d851 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,15 +1,17 @@
>  MANPAGES=mdmv.1 mbox2maildir.1 \
>   notmuch-slurp-debbug.1 notmuch-extract-patch.1 maildir-import-patch.1 \
> + imap-dl.1 \
>   email-extract-openpgp-certs.1 \
>   email-print-mime-structure.1 \
>   notmuch-import-patch.1
> -COMPLETIONS=completions/bash/email-print-mime-structure
> +COMPLETIONS=completions/bash/email-print-mime-structure 
> completions/bash/imap-dl
>  
>  all: $(MANPAGES) $(COMPLETIONS)
>  
>  check:
>   ./tests/email-print-mime-structure.sh
>   mypy --strict ./email-print-mime-structure
> + mypy --strict ./imap-dl
>  
>  clean:
>   rm -f $(MANPAGES)
> diff --git a/debian/control b/debian/control
> index bc8268a..21afa45 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -77,3 +77,5 @@ Description: collection of scripts for manipulating e-mail 
> on Debian
>   email-print-mime-structure -- tree view of a message's MIME structure
>   .
>   email-extract-openpgp-certs -- extract OpenPGP certificates from a message
> + .
> + imap-dl -- download messages from an IMAP mailbox to a maildir
> diff --git a/debian/mailscripts.bash-completion 
> b/debian/mailscripts.bash-completion
> index 435576f..657de01 100644
> --- a/debian/mailscripts.bash-completion
> +++ b/debian/mailscripts.bash-completion
> @@ -1 +1,2 @@
>  completions/bash/email-print-mime-structure
> +completions/bash/imap-dl
> diff --git a/debian/mailscripts.install b/debian/mailscripts.install
> index 2c060df..3739c49 100644
> --- a/debian/mailscripts.install
> +++ b/debian/mailscripts.install
> @@ -1,5 +1,6 @@
>  email-extract-openpgp-certs /usr/bin
>  email-print-mime-structure /usr/bin
> +imap-dl /usr/bin
>  maildir-import-patch /usr/bin
>  mbox2maildir /usr/bin
>  mdmv /usr/bin
> diff --git a/debian/mailscripts.manpages b/debian/mailscripts.manpages
> index 1de088f..a915617 100644
> --- a/debian/mailscripts.manpages
> +++ b/debian/mailscripts.manpages
> @@ -1,5 +1,6 @@
>  email-extract-openpgp-certs.1
>  email-print-mime-structure.1
> +imap-dl.1
>  maildir-import-patch.1
>  mbox2maildir.1
>  mdmv.1
> diff --git a/imap-dl b/imap-dl
> new file mode 100755
> index 000..f5d7a85
> --- /dev/null
> +++ b/imap-dl
> @@ -0,0 +1,254 @@
> +#!/usr/bin/python3
> +# PYTHON_ARGCOMPLETE_OK
> +# -*- coding: utf-8 -*-
> +
> +# Copyright (C) 2019 Daniel Kahn Gillmor
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or (at
> +# your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public 

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

Hi Sean--

On Fri 2020-01-17 09:26:38 -0700, Sean Whitton wrote:
> I think the easiest thing to do would be for one of you to prepare a
> single patch, signed off, and for the other to write an e-mail signing
> it off.  I'll then do a code review of the latest version of the script.

The attached git-formatted patch is also present on the imap-dl-squashed
branch on https://salsa.debian.org/dkg/mailscripts.  jrollins confirmed
that it was OK, which is why it bears both of our signoffs.

Thanks for considering imap-dl for inclusion within mailscripts!

 --dkg

From 9e5c1a893c17343102b042de23bdaa0f91b37d66 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor 
Date: Sun, 15 Sep 2019 19:55:07 -0400
Subject: [PATCH] Add imap-dl, a simple imap downloader

getmail upstream appears to have no plans to convert to python3 in the
near future.

Some of us use only a minimal subset of features of getmail, and it
would be nice to have something simpler, with the main complexity
offloaded to the modern python3 stdlib.

This patch represents a squashed series of changes from both Jameson
Graef Rollins and Daniel Kahn Gillmor (dkg), though dkg is primarily
responsible for any remaining bugs.

Signed-off-by: Jameson Graef Rollins 
Signed-off-by: Daniel Kahn Gillmor 
---
 Makefile   |   4 +-
 debian/control |   2 +
 debian/mailscripts.bash-completion |   1 +
 debian/mailscripts.install |   1 +
 debian/mailscripts.manpages|   1 +
 imap-dl| 254 +
 imap-dl.1.pod  |  88 ++
 7 files changed, 350 insertions(+), 1 deletion(-)
 create mode 100755 imap-dl
 create mode 100644 imap-dl.1.pod

diff --git a/Makefile b/Makefile
index af30616..ec3d851 100644
--- a/Makefile
+++ b/Makefile
@@ -1,15 +1,17 @@
 MANPAGES=mdmv.1 mbox2maildir.1 \
 	notmuch-slurp-debbug.1 notmuch-extract-patch.1 maildir-import-patch.1 \
+	imap-dl.1 \
 	email-extract-openpgp-certs.1 \
 	email-print-mime-structure.1 \
 	notmuch-import-patch.1
-COMPLETIONS=completions/bash/email-print-mime-structure
+COMPLETIONS=completions/bash/email-print-mime-structure completions/bash/imap-dl
 
 all: $(MANPAGES) $(COMPLETIONS)
 
 check:
 	./tests/email-print-mime-structure.sh
 	mypy --strict ./email-print-mime-structure
+	mypy --strict ./imap-dl
 
 clean:
 	rm -f $(MANPAGES)
diff --git a/debian/control b/debian/control
index bc8268a..21afa45 100644
--- a/debian/control
+++ b/debian/control
@@ -77,3 +77,5 @@ Description: collection of scripts for manipulating e-mail on Debian
  email-print-mime-structure -- tree view of a message's MIME structure
  .
  email-extract-openpgp-certs -- extract OpenPGP certificates from a message
+ .
+ imap-dl -- download messages from an IMAP mailbox to a maildir
diff --git a/debian/mailscripts.bash-completion b/debian/mailscripts.bash-completion
index 435576f..657de01 100644
--- a/debian/mailscripts.bash-completion
+++ b/debian/mailscripts.bash-completion
@@ -1 +1,2 @@
 completions/bash/email-print-mime-structure
+completions/bash/imap-dl
diff --git a/debian/mailscripts.install b/debian/mailscripts.install
index 2c060df..3739c49 100644
--- a/debian/mailscripts.install
+++ b/debian/mailscripts.install
@@ -1,5 +1,6 @@
 email-extract-openpgp-certs /usr/bin
 email-print-mime-structure /usr/bin
+imap-dl /usr/bin
 maildir-import-patch /usr/bin
 mbox2maildir /usr/bin
 mdmv /usr/bin
diff --git a/debian/mailscripts.manpages b/debian/mailscripts.manpages
index 1de088f..a915617 100644
--- a/debian/mailscripts.manpages
+++ b/debian/mailscripts.manpages
@@ -1,5 +1,6 @@
 email-extract-openpgp-certs.1
 email-print-mime-structure.1
+imap-dl.1
 maildir-import-patch.1
 mbox2maildir.1
 mdmv.1
diff --git a/imap-dl b/imap-dl
new file mode 100755
index 000..f5d7a85
--- /dev/null
+++ b/imap-dl
@@ -0,0 +1,254 @@
+#!/usr/bin/python3
+# PYTHON_ARGCOMPLETE_OK
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2019 Daniel Kahn Gillmor
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or (at
+# your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+DESCRIPTION = '''A simple replacement for a minimalist use of getmail.
+
+In particular, if you use getmail to reach an IMAP server as though it
+were POP (retrieving from the server and optionally deleting), you can
+point this script to the getmail config and it should do the same
+thing.
+
+It tries to ensure that the configuration file is of the expected
+type, 

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello dkg, jrollins,

On Mon 06 Jan 2020 at 06:38PM -05, Daniel Kahn Gillmor wrote:

> It's now been three months, and the only changes i've made to imap-dl
> have been trivial ones:
>
>  - accept that some IMAP daemons will lie about message sizes before
>download, offer workaround for users stuck with those servers
>(thanks, bremner)
>  - some grammar cleanup (thanks, Clint)
>  - auto-creating the target maildir if it wasn't already present
>(thanks, jrollins)
>  - produce sensible --help output
>(thanks, jrollins)
>  - clean up internal typechecking
>  - add tab completion in bash
>
> The whole series is available on the "imap-dl" branch at
> https://salsa.debian.org/dkg/mailscripts.git, or if you prefer to import
> it a single patch, i'm attaching that here.

I've looked through `git diff 8b1f05d..0b492bd`, i.e. your changes made
since the original patch posting, and I'm satisfied that they are
sufficiently minimal.

I can't merge dkg's branch as it stands because there are substantive
changes by both dkg and jrollins which are not signed off.

I think the easiest thing to do would be for one of you to prepare a
single patch, signed off, and for the other to write an e-mail signing
it off.  I'll then do a code review of the latest version of the script.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

On Sun 2019-10-06 14:18:16 -0400, Daniel Kahn Gillmor wrote:
> On Sat 2019-10-05 10:21:05 -0700, Sean Whitton wrote:
>
>> As an alternative to adding the integration tests, how about you use
>> imap-dl on a daily basis for ~3 months with (I assume) a standard IMAP
>> server, and if you don't have to make any nontrivial changes to the
>> script during that time, we can ship it in mailscripts?
>
> i'm using imap-dl on a daily basis now, and will happily report back
> then too.  the last changes i made were supplying more debugging details
> on october 2nd, so i suppose the new year is ~3 months if you want to
> start the clock from my last changes :)

It's now been three months, and the only changes i've made to imap-dl
have been trivial ones:

 - accept that some IMAP daemons will lie about message sizes before
   download, offer workaround for users stuck with those servers
   (thanks, bremner)
 - some grammar cleanup (thanks, Clint)
 - auto-creating the target maildir if it wasn't already present
   (thanks, jrollins)
 - produce sensible --help output
   (thanks, jrollins)
 - clean up internal typechecking
 - add tab completion in bash

The whole series is available on the "imap-dl" branch at
https://salsa.debian.org/dkg/mailscripts.git, or if you prefer to import
it a single patch, i'm attaching that here.

Thanks for maintaining mailscripts,

   --dkg

diff --git a/Makefile b/Makefile
index af30616..ec3d851 100644
--- a/Makefile
+++ b/Makefile
@@ -1,15 +1,17 @@
 MANPAGES=mdmv.1 mbox2maildir.1 \
 	notmuch-slurp-debbug.1 notmuch-extract-patch.1 maildir-import-patch.1 \
+	imap-dl.1 \
 	email-extract-openpgp-certs.1 \
 	email-print-mime-structure.1 \
 	notmuch-import-patch.1
-COMPLETIONS=completions/bash/email-print-mime-structure
+COMPLETIONS=completions/bash/email-print-mime-structure completions/bash/imap-dl
 
 all: $(MANPAGES) $(COMPLETIONS)
 
 check:
 	./tests/email-print-mime-structure.sh
 	mypy --strict ./email-print-mime-structure
+	mypy --strict ./imap-dl
 
 clean:
 	rm -f $(MANPAGES)
diff --git a/debian/control b/debian/control
index bc8268a..21afa45 100644
--- a/debian/control
+++ b/debian/control
@@ -77,3 +77,5 @@ Description: collection of scripts for manipulating e-mail on Debian
  email-print-mime-structure -- tree view of a message's MIME structure
  .
  email-extract-openpgp-certs -- extract OpenPGP certificates from a message
+ .
+ imap-dl -- download messages from an IMAP mailbox to a maildir
diff --git a/debian/mailscripts.bash-completion b/debian/mailscripts.bash-completion
index 435576f..657de01 100644
--- a/debian/mailscripts.bash-completion
+++ b/debian/mailscripts.bash-completion
@@ -1 +1,2 @@
 completions/bash/email-print-mime-structure
+completions/bash/imap-dl
diff --git a/debian/mailscripts.install b/debian/mailscripts.install
index 2c060df..3739c49 100644
--- a/debian/mailscripts.install
+++ b/debian/mailscripts.install
@@ -1,5 +1,6 @@
 email-extract-openpgp-certs /usr/bin
 email-print-mime-structure /usr/bin
+imap-dl /usr/bin
 maildir-import-patch /usr/bin
 mbox2maildir /usr/bin
 mdmv /usr/bin
diff --git a/debian/mailscripts.manpages b/debian/mailscripts.manpages
index 1de088f..a915617 100644
--- a/debian/mailscripts.manpages
+++ b/debian/mailscripts.manpages
@@ -1,5 +1,6 @@
 email-extract-openpgp-certs.1
 email-print-mime-structure.1
+imap-dl.1
 maildir-import-patch.1
 mbox2maildir.1
 mdmv.1
diff --git a/imap-dl b/imap-dl
new file mode 100755
index 000..f5d7a85
--- /dev/null
+++ b/imap-dl
@@ -0,0 +1,254 @@
+#!/usr/bin/python3
+# PYTHON_ARGCOMPLETE_OK
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2019 Daniel Kahn Gillmor
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or (at
+# your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+DESCRIPTION = '''A simple replacement for a minimalist use of getmail.
+
+In particular, if you use getmail to reach an IMAP server as though it
+were POP (retrieving from the server and optionally deleting), you can
+point this script to the getmail config and it should do the same
+thing.
+
+It tries to ensure that the configuration file is of the expected
+type, and will terminate raising an exception, and it should not lose
+messages.
+
+If there's any interest in supporting other use cases for getmail,
+patches are welcome.
+
+If you've never used getmail, you can make the simplest possible
+config file like so:
+
+--
+[retriever]
+server = mail.example.net
+username = foo
+password = sekr1t!
+

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello,

On Sun 06 Oct 2019 at 02:18PM -04, Daniel Kahn Gillmor wrote:

> i'm using imap-dl on a daily basis now, and will happily report back
> then too.  the last changes i made were supplying more debugging details
> on october 2nd, so i suppose the new year is ~3 months if you want to
> start the clock from my last changes :)
>
> I'd really love to have the integration stuff working, but i don't know
> when i'll get to that.

Cool -- please drop the moreinfo tag if no nontrivial changes in that
period.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

control: tag -1 +moreinfo

Hello,

On Sat 05 Oct 2019 at 04:04PM -03, David Bremner wrote:

> FWIW I'm already using imap-dl daily, so I guess ask me in a few months
> if I noticed any problems.

Cool -- tagging moreinfo to reflect this.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

On Sat 2019-10-05 10:21:05 -0700, Sean Whitton wrote:

> As an alternative to adding the integration tests, how about you use
> imap-dl on a daily basis for ~3 months with (I assume) a standard IMAP
> server, and if you don't have to make any nontrivial changes to the
> script during that time, we can ship it in mailscripts?

i'm using imap-dl on a daily basis now, and will happily report back
then too.  the last changes i made were supplying more debugging details
on october 2nd, so i suppose the new year is ~3 months if you want to
start the clock from my last changes :)

I'd really love to have the integration stuff working, but i don't know
when i'll get to that.

  --dkg

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[David Bremner]

Sean Whitton  writes:

> Hello,
>
> On Sun 29 Sep 2019 at 11:21AM +02, Daniel Kahn Gillmor wrote:
>
>> Do you think we need this to be done in order for mailscripts to adopt
>> imap-dl?  that would make me kind of sad, though of course i can
>> continue to use imap-dl without its adoption by mailscripts.
>
> As an alternative to adding the integration tests, how about you use
> imap-dl on a daily basis for ~3 months with (I assume) a standard IMAP
> server, and if you don't have to make any nontrivial changes to the
> script during that time, we can ship it in mailscripts?

FWIW I'm already using imap-dl daily, so I guess ask me in a few months
if I noticed any problems.

d

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello,

On Sun 29 Sep 2019 at 11:21AM +02, Daniel Kahn Gillmor wrote:

> Do you think we need this to be done in order for mailscripts to adopt
> imap-dl?  that would make me kind of sad, though of course i can
> continue to use imap-dl without its adoption by mailscripts.

As an alternative to adding the integration tests, how about you use
imap-dl on a daily basis for ~3 months with (I assume) a standard IMAP
server, and if you don't have to make any nontrivial changes to the
script during that time, we can ship it in mailscripts?

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

Hi Sean--

Thanks for the review!

On Sat 2019-09-28 08:53:38 -0700, Sean Whitton wrote:
> I read through the script and I'm a bit apprehensive about the
> complexity involved in talking to the IMAP server, because it renders
> imap-dl significantly more complicated than anything else in
> mailscripts:
>
>> +# these objects are weirdly structured. i don't know why
>> +# these trailing close-parens show up.  so this is very
>> +# ad-hoc and nonsense

Yeah, this is a bit of a shame.  I would welcome someone with more IMAP
knowledge than me making it clearer what's happening here or suggesting
a more rigorous handling of the returned tuples.

> In general, it seems to me that mailscripts should fairly strongly
> favour simplicity over features, so that the scripts in mailscripts can
> be useful fixed points around which to build more complex (and perhaps
> more fragile) systems.
>
> Of course, imap-dl *is* simple in terms of its features, so I hope we
> can mitigate the risks of this IMAP complexity somehow.

right, i'm aiming for simple features.  that means masking some of the
complexity.

> Would you consider writing an integration test suite for imap-dl?  I
> would like it to be possible for us to be completely confident that
> imap-dl won't ever lose any user messages.

I would love more generally to have an integration test suite that can
work for multiple IMAP clients (i want a test suite for
thunderbird+enigmail too, for example!)

To do this, we need an imap server that can receive and deliver
messages, though, right?

I've sketched out a design for this over at
https://salsa.debian.org/debian/test-daemons -- it's a lot of work!  But
it seems like it could pay off in the future, too.

Once it's done, i'd be happy to add an integration test that uses
imap-dl this way.

Do you think we need this to be done in order for mailscripts to adopt
imap-dl?  that would make me kind of sad, though of course i can
continue to use imap-dl without its adoption by mailscripts.

> I'm not keen on introducing imap-dl as a getmail replacement.
> Hopefully, in the future, the majority of imap-dl's users won't be
> people who ever used getmail.  It would be good for the docs to
> introduce imap-dl without reference to getmail, and then explain later
> how it can be a getmail replacement.

That makes sense to me.  do you want to propose a patch to the
documentation?

--dkg


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Sean Whitton]

Hello dkg,

On Tue 17 Sep 2019 at 09:31AM -04, Daniel Kahn Gillmor wrote:

> getmail upstream appears to have no plans to convert to python3 in the
> near future.
>
> Some of us use only a minimal subset of features of getmail, and it
> would be nice to have something simpler, with the main complexity
> offloaded to the modern python3 stdlib.

This is a cool idea.

I read through the script and I'm a bit apprehensive about the
complexity involved in talking to the IMAP server, because it renders
imap-dl significantly more complicated than anything else in
mailscripts:

> +# these objects are weirdly structured. i don't know why
> +# these trailing close-parens show up.  so this is very
> +# ad-hoc and nonsense

In general, it seems to me that mailscripts should fairly strongly
favour simplicity over features, so that the scripts in mailscripts can
be useful fixed points around which to build more complex (and perhaps
more fragile) systems.

Of course, imap-dl *is* simple in terms of its features, so I hope we
can mitigate the risks of this IMAP complexity somehow.

Would you consider writing an integration test suite for imap-dl?  I
would like it to be possible for us to be completely confident that
imap-dl won't ever lose any user messages.

> +=head1 DESCRIPTION
> +
> +If you use getmail to reach an IMAP server as though it was POP
> +(retrieving from the server, storing it in a maildir and optionally
> +deleting), you can point this script to the getmail config and it
> +should do the same thing.
> +
> +It tries to ensure that the configuration file is of the expected
> +type, and will terminate raising an exception, and it should not lose
> +messages.
> +
> +If there's any interest in supporting other use cases for getmail,
> +patches are welcome.

I'm not keen on introducing imap-dl as a getmail replacement.
Hopefully, in the future, the majority of imap-dl's users won't be
people who ever used getmail.  It would be good for the docs to
introduce imap-dl without reference to getmail, and then explain later
how it can be a getmail replacement.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#940461: [PATCH v2] Add imap-dl, a simple imap downloader

[Daniel Kahn Gillmor]

getmail upstream appears to have no plans to convert to python3 in the
near future.

Some of us use only a minimal subset of features of getmail, and it
would be nice to have something simpler, with the main complexity
offloaded to the modern python3 stdlib.

Signed-off-by: Daniel Kahn Gillmor 
---
 Makefile|   1 +
 debian/mailscripts.install  |   1 +
 debian/mailscripts.manpages |   1 +
 imap-dl | 221 
 imap-dl.1.pod   |  88 ++
 5 files changed, 312 insertions(+)
 create mode 100755 imap-dl
 create mode 100644 imap-dl.1.pod

diff --git a/Makefile b/Makefile
index 352f6f0..860ec27 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,6 @@
 MANPAGES=mdmv.1 mbox2maildir.1 \
notmuch-slurp-debbug.1 notmuch-extract-patch.1 maildir-import-patch.1 \
+   imap-dl.1 \
email-extract-openpgp-certs.1 \
email-print-mime-structure.1 \
notmuch-import-patch.1
diff --git a/debian/mailscripts.install b/debian/mailscripts.install
index 99216c1..221f7bf 100644
--- a/debian/mailscripts.install
+++ b/debian/mailscripts.install
@@ -6,3 +6,4 @@ notmuch-import-patch /usr/bin
 notmuch-extract-patch/notmuch-extract-patch /usr/bin
 email-extract-openpgp-certs /usr/bin
 email-print-mime-structure /usr/bin
+imap-dl /usr/bin
diff --git a/debian/mailscripts.manpages b/debian/mailscripts.manpages
index 6d7cb30..bfbd56b 100644
--- a/debian/mailscripts.manpages
+++ b/debian/mailscripts.manpages
@@ -6,3 +6,4 @@ notmuch-import-patch.1
 notmuch-extract-patch.1
 email-extract-openpgp-certs.1
 email-print-mime-structure.1
+imap-dl.1
diff --git a/imap-dl b/imap-dl
new file mode 100755
index 000..c2a8186
--- /dev/null
+++ b/imap-dl
@@ -0,0 +1,221 @@
+#!/usr/bin/python3
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2019 Daniel Kahn Gillmor
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or (at
+# your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+'''A simple replacement for a minimalist use of getmail.
+
+Usage: 
+
+   imap-dl [-v|--verbose] configfile…
+
+In particular, if you use getmail to reach an IMAP server as though it
+was POP (retrieving from the server and optionally deleting), you can
+point this script to the getmail config and it should do the same
+thing.
+
+It tries to ensure that the configuration file is of the expected
+type, and will terminate raising an exception, and it should not lose
+messages.
+
+If there's any interest in supporting other use cases for getmail,
+patches are welcome.
+
+If you've never used getmail, you can make the simplest possible
+config file like so:
+
+--
+[retriever]
+server = mail.example.net
+username = foo
+password = sekr1t!
+
+[destination]
+path = /home/foo/Maildir
+
+[options]
+delete = True
+--
+'''
+
+import configparser
+import sys
+import ssl
+import imaplib
+import re
+import logging
+import mailbox
+import os.path
+import statistics
+import time
+
+_summary_splitter = re.compile(rb'^(?P[0-9]+) \(UID (?P[0-9]+) 
RFC822.SIZE (?P[0-9]+)\)$')
+def break_fetch_summary(line):
+'''b'1 (UID 160 RFC822.SIZE 1867)' -> {id: 1, uid: 160, size: 1867}'''
+match = _summary_splitter.match(line)
+if not match:
+raise Exception('malformed summary line %s'%(line))
+ret = {}
+for i in ['id', 'uid', 'size']:
+ret[i] = int(match[i])
+return ret
+
+_fetch_splitter = re.compile(rb'^(?P[0-9]+) \(UID (?P[0-9]+) (FLAGS 
\([\\A-Za-z ]*\) )?BODY\[\] \{(?P[0-9]+)\}$')
+def break_fetch(line):
+'''b'1 (UID 160 BODY[] {1867}' -> {id: 1, uid: 160, size: 1867}'''
+match = _fetch_splitter.match(line)
+if not match:
+raise Exception('malformed fetch line %s'%(line))
+ret = {}
+for i in ['id', 'uid', 'size']:
+ret[i] = int(match[i])
+return ret
+
+def pull_msgs(configfile):
+conf = configparser.ConfigParser()
+conf.read_file(open(configfile, 'r'))
+oldloglevel = logging.getLogger().getEffectiveLevel()
+conf_verbose = conf.getint('options', 'verbose', fallback=1)
+if conf_verbose > 1:
+logging.getLogger().setLevel(logging.INFO)
+logging.info('pulling from config file %s', configfile)
+delete = conf.getboolean('options', 'delete', fallback=False)
+read_all = conf.getboolean('options', 'read_all', fallback=True)
+if not read_all:
+raise NotImplementedError('imap-dl only supports 
options.read_all=True, got False')
+