On 5/13/22 08:32, Kevin A. McGrail via MIMEDefang wrote:
MailMunge is amazing and people should definitely look at it but it is
a different approach and rewrite of the code. And while I think every
programmer dreams of a ground up rewrite of legacy code, we agreed to
be the stewards of the MIMEDefang project, for better or worse.
As a user, I would really not like to see MIMEDefang forked, as that's
going to divide attention on what is already a very niche project.
If there is going to be a 3.x (i.e. breaking changes), I agree in
principle with Dianne that the changes should be a significant reworking
of the API to do things like eliminate global variables, etc. I haven't
actually run Mailmunge, but in looking at it, its API seems to be a
significant structural improvement. I do have some concerns, which I'll
list at the end of this message.
For the sake of argument, let's say that Mailmunge's API is perfect.
Even in that best case, I now have to make a choice to either stay with
the current project, which seems to have multiple developers now (yay!)
but hasn't improved the API (boo!) but yet is still throwing in smaller
breaking changes (boo!), or move to Mailmunge, which has one author
(boo!), albeit the original MIMEDefang author with lots of experience
(yay!), and has improved the API (yay!). This trade-off is not great.
And if I'm going to have to deal with all that, then to be honest, I
might be better off just trying to stop using this entirely and
switching entirely to something that feels like it has more long-term
stability (rspamd as a straw example; I haven't seriously investigated
that enough either).
As a specific concern in my situation: mimedefang is currently packaged
in Debian, and mailmunge is not. While I certainly could package
mailmunge (I'm a Debian Developer.), that's a new burden I'd be taking on.
On the other hand, if this MIMEDefang 3 is something where I just need
to add a few use statements or prefix certain function calls with a
namespace, I guess that's not really a big deal. But then what is it
really accomplishing?
From my perspective, I think the ideal way forward here is:
A) If the Mailmunge API is correct/reasonable for a redesigned API, then
merge the projects back together. Which name is used moving forward is
negotiable. MIMEDefang has the history and the inertia (not having to
rename distro packages is easier). But if you're breaking backwards
compatibility anyway, it's not a bad time for a name change.
B) If the Mailmunge API is not correct, can we articulate why? Will
Dianne agree with the proposed changes? If so, then with those changes,
we're back to scenario "A" again.
Specific Mailmunge API concerns:
* Most annoyingly, there are still the two return styles for message
dispositions depending on whether we are in filter_message() or
something earlier. filter_message()'s return value is ignored and
action_{bounce,discard,tempfail}() are used. I can understand how it
may be desirable to keep action_{bounce,discard,tempfail}() for
backwards compatibility with existing code, but they should likely
be deprecated. In any event, filter_message()'s return value should
be a Response which is then converted (by way of
action_from_response(), which should itself be deprecated for use by
callers).
* Most significantly, it seems to have retained the add_*() and
delete_*() APIs for things like headers and recipients. I think it
should instead have a mutable representation that Does The Right Thing.
o Looking at recipients, for example, I want to be able to modify
$ctx->recipients. If I add or delete one, it should do the work
of add_recipient() or delete_recipient() under the hood. Ideally
it only compares them at the end of processing the message, such
that `delete_recipient('bob'); add_recipient('bob');` does
nothing at the milter level if bob was an existing recipient.
o See also change_sender(). The documentation goes out of its way
to tell you that you can change $ctx->sender but that it won't
affect anything.
* Unless it's impossible (or unreasonable) to do optional arguments in
Perl, I don't see why there are both action_add_header() and
action_insert_header(). Just have the insert, with $pos defaulting
to -1 or something to get the add behavior. Likewise for
action_{accept,drop}_with_warning(); just have an optional $warning
parameter on action_{accept,drop}().
* I'm confused by the idea (as Dianne posted on the mailmunge list)
that a module named "Compat" is expected to be a thing that people
use indefinitely and in new installations as opposed to as a bridge
while porting their MIMEDefang 2.x filter.
* In the Mailmunge example video, $ctx->recipients[0] is
'<[email protected]>'. IMHO, mailmunge should be stripping off the
angle brackets before the filter see it. They are just an annoyance.
Perhaps it should be lowercasing too, i.e. using canonical_email().
* This is minor, since it's boilerplate, but I'm not sure what the
run() method is about. What is "server mode" (as opposed to
multiplexor mode)?
--
Richard
_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID. You may ignore it.
MIMEDefang mailing list [email protected]
https://lists.mimedefang.org/mailman/listinfo/mimedefang_lists.mimedefang.org
