Bug#857248: nullmailer: bug found!

On Tue, Sep 5, 2017 at 3:52 AM, David Bremner <david@tethera.net> wrote:

Felix Lechner <felix.lechner@lease-up.com> writes:

> David,
>
> I found the bug affecting the tests. (It was an issue only when TLS was
> enabled, and there were some race conditions.) I packaged version 2.0
> and uploaded
> it to Mentors <https://mentors.debian.net/package/nullmailer>.
>
> You are still welcome to adopt the package. Or, you can sponsor me---I
> really don't care. Just trying to help.

OK, I'm (slowly) looking at your patches. My current plan is to
integrate (most of) your changes and add you as an Uploader to the
package in collab-maint. That's mostly symbolic but it might help get a
sponsor in the future if I'm not available.

That's a great solution. Thank you!

> Some of the open items in my version are:
>
> 1. Syslog code (and therefore '--daemon') not adopted from prior version.
> It probably would be a good idea, but is complicated, and the package works
> without it.

I think this is probably gone for good. We do have to take a bit more
care: I think /etc/init.d/nullmailer probably needs to be dropped from
the package, since it relies on -s. A NEWS item for those people relying
on non-systemd inits seems also needed.

Thank you for catching that. It's probably the right thing to do.

>
> 2. I slightly modified the interplay between '/etc/mailname', 'me' and
> 'defaulthost' to allow the tests to complete. Not sure if that broke the
> '/etc/mailname' behavior. I don't use that feature.
>

Was there more than just grouping patches here?

Yes, I honored 'me' when '/etc/mailname' does not exist; the previous maintainer ignored 'me'. (Now the tests complete and testing for a Debian-compliant '/etc/mailname' may not be necessary.) Also, the patched man pages substitute '/etc/mailname' for 'me', so the fallback behavior is undocumented.

- if (!config_read("me", me)) {
+ if (!config_read("../mailname", me) && !config_read("me", me)) {

vs.

- if (!config_read("me", me)) {
+ if (!config_read("../mailname", me)) {

In addition, I did not follow the previous maintainer when he no longer required at least one dot in the domain part of an email address. He commented out a call in src/queue.cc to upstream's function find_first(), which is replicated here for your convenience. The check takes place after a possible remapping of 'localhost' and seemed reasonable, so I kept it.

int mystring::find_first(char ch, size_t offset[default 0]) const
{
if(offset >= rep->length)
return -1;
char* ptr = strchr(rep->buf+offset, ch);
return ptr ? ptr-rep->buf : -1;
}

bool validate_addr(mystring& addr, bool doremap)
{
int i = addr.find_last('@');
if(i < 0)
    return false;
mystring hostname = addr.right(i+1);
if(doremap && remapadmin) {
    if(hostname == me || hostname == "localhost")
      addr = adminaddr;
}
// else if(hostname.find_first('.') < 0) [NOT ADOPTED]
    // return false; [NOT ADOPTED]
return true;
}

Best regards,

Felix