Re: [fetchmail-devel] STARTTLS for ODMR

[Matthias A. <mat...@gm...>]

Jakob Hirsch schrieb am 2007-08-23:

> here's a patch to add STARTTLS to the ODMR protocol. (I was a little
> surprised yesterday when I noticed that it isn't already there...)

Thanks.

> Most of the code is copied from imap.c. I inserted it into the generic
> smtp code, so it could probably be used to deliver fetched mail via TLS
> (if somebody ever wanted to do that).

Without looking at the code, is it possible to factor out the common
code into tls.c? Copy & paste is always prone to leaving a bug in one
place while the other has been fixed.  I should like the crucial code to
be in ONE place when reasonably possible even if that requires using
callback hooks or function address arguments, so that we don't have to
patch two or three places when a bug is found in once, and "copying"
breaks that, and SSL/TLS code makes delicate decisions -- let's avoid
any avoidable embarrassment in that area.

> You may notice that the patch removes adding our hostname to the
> ctl->domainlist for ODMR. I think this was plain wrong and bothered me
> everytime I set up fetchmail with odmr. ODMR's default is a simple
> "ATRN" without any arguments, which means "fetch mail for all domains
> associated with the account", so we should stick with that.

If it's still possible to add it manually, I don't mind, although I
wonder if that makes the patch suitable for inclusion into 6.3.

> Additional notes:
> - The logging looks broken, with -v you get mixed ODMR and SMTP line
> prefixes, but that's not related to my patch. Fixing this would could
> probably easily be done by changing smtp_mode and using it as an index
> to an array { "SMTP", "LMTP", "ODMR", ... }.

Sunil knows that part of the code better than I do, but we can fix that
if we catch all occurrences and switch() statements related to
smtp_mode. (I wonder if that's a reasonable design anyways, but I'm
loathe to change it in 6.3.)

> - What's with the formatting in odmr.c?? Mixed spaces and tabs, that's
> quite horrible... if you don't mind, I'll clean that up (to whatever the
> preferred format is).

Well, my canonical style (I'm using vim) is an indentation width of 4
columns per level and using a tabwidth of 8, I.e. it's zero or more tabs
followed by zero or four spaces. Any reasonable editor should be capable
of writing that, and if not, use "all spaces" and run the code through
unexpand --first-only before diffing. Thanks.

I hope to get around to do some bugfixing later tonight (European time)
so that I can release 6.3.9 soon and then move on to 6.4.

Thanks for your efforts again.

-- 
Matthias Andree