On 5/14/22 07:18, Richard Laager via MIMEDefang wrote: > 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? > Some methods (entity_contains_virus_* for example), should not be used directly by mimedefang-filters imho, entity_contains_virus should be used instead. As already mentioned, I have no problems in keeping all those functions public like in older MIMEDefang version to maintain compatibility, I can take a different approach to deprecate some subs. Redesigning MIMEDefang API will probably break all/most/many filters and this is not what I have in mind for the future.
Regards
Giovanni
>
> 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
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ 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
