Bugzilla Bug 199428

New patches to make DSPAM work better with newer editions of MySQL and QMail.

Reproducible: Always

Created an attachment (id=136141) [edit]
mail-filter/dspam/dspam-3.8.0-r8.ebuild

New DSPAM ebuild with following changes:
--- dspam-3.8.0-r8.ebuild       2007-11-17 10:49:15.788190750 +0100
+++ /usr/portage/mail-filter/dspam/dspam-3.8.0-r7.ebuild        2007-11-05
18:06:18.000000000 +0100
@@ -1,6 +1,6 @@
 # Copyright 1999-2007 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
-# $Header: Exp $
+# $Header: /var/cvsroot/gentoo-x86/mail-filter/dspam/dspam-3.8.0-r7.ebuild,v
1.6 2007/11/05 16:57:04 armin76 Exp $

 WANT_AUTOCONF="latest"
 WANT_AUTOMAKE="latest"
@@ -66,26 +66,6 @@
        EPATCH_SUFFIX="patch"
        epatch "${WORKDIR}"/patches

-       # Add MySQLReconnect option
-       epatch "${FILESDIR}"/${PN}-${PV}-mysql_reconnect.patch
-
-       # Fix domain blocklisting
-       epatch "${FILESDIR}"/${PN}-${PV}-blocklist.patch
-
-       # Fix non existent path for notification
-       epatch "${FILESDIR}"/${PN}-${PV}-notification_path.patch
-
-       # Fix dspam_train
-       epatch "${FILESDIR}"/${PN}-${PV}-dspam_train.patch
-
-       # Fix various problems with QMail
-       epatch "${FILESDIR}"/${PN}-${PV}-eol.patch
-       epatch "${FILESDIR}"/${PN}-${PV}-src_client.c.patch
-       epatch "${FILESDIR}"/${PN}-${PV}-src_dspam.c.patch
-
-       # Fix problem with unknown users
-       epatch "${FILESDIR}"/${PN}-${PV}-nosuchuser.patch
-
        # Fix Lazy bindings
        append-flags $(bindnow-flags)

@@ -280,12 +260,6 @@
                        -i "${D}/${DSPAM_CONFDIR}"/dspam.conf
        fi

-       # Add MySQLReconnect option after MySQLCompress
-       if ! ( grep -iqe "^#*MySQLReconnect[[:space:]]"
"${D}"/${CONFDIR}/dspam.conf ) ; then
-               sed -e "s:^\(\(#*\)MySQLCompress[\t
].*\):\1\n\2MySQLReconnect\t\ttrue:" \
-                       -i "${D}"/${DSPAM_CONFDIR}/dspam.conf
-       fi
-
        # installs the notification messages
        # -> The documentation is wrong! The files need to be in ./txt
        echo "Scanned and tagged as SPAM with DSPAM ${PV} by Your
ISP.com">"${T}"/msgtag.spam

Created an attachment (id=136143) [edit]
files/dspam-3.8.0-blocklist.patch

Created an attachment (id=136145) [edit]
files/dspam-3.8.0-dspam_train.patch

Created an attachment (id=136146) [edit]
files/dspam-3.8.0-eol.patch

Created an attachment (id=136147) [edit]
files/dspam-3.8.0-mysql_reconnect.patch

Created an attachment (id=136149) [edit]
files/dspam-3.8.0-nosuchuser.patch

Created an attachment (id=136150) [edit]
files/dspam-3.8.0-notification_path.patch

Created an attachment (id=136151) [edit]
files/dspam-3.8.0-src_client.c.patch

Created an attachment (id=136153) [edit]
files/dspam-3.8.0-src_dspam.c.patch

Because we need to apply a large number of patches, I've made a tarball with
these patches.

In order to incorporate these new patches into the tarball, they must have a
proper name. Names like "src_client.c.patch" aren't proper, so please rename
them so I could understand easily what purpose do they serve.

(In reply to comment #10)
> Because we need to apply a large number of patches, I've made a tarball with
> these patches.
>
I know.


> In order to incorporate these new patches into the tarball, they must have a
> proper name. Names like "src_client.c.patch" aren't proper, so please rename
> them so I could understand easily what purpose do they serve.
> 
Okay. Will fix that. Give me time to finish cooking my Fondue and eat it. After
that I will fix the issue with the patch naming convention.

Created an attachment (id=136318) [edit]
files/dspam-3.8.0-better_error_handling.patch

Created an attachment (id=136320) [edit]
files/dspam-3.8.0-mysql_reconnect.patch

(From update of attachment 136141 [edit])
Not needed any more.

Okay. Fixed the naming of the patches and moved some stuff into a patch instead
of using GNU sed to modify dspam.conf. Now no changes are needed to the
structure of the ebuild. Just add the patches into the archive and reference
the new patch-archive and all is done :)

I am running since months on those patches. They work all fine on my
installation (but I am in no way a reference for all DSPAM users out there).

Most of the patches do fix stuff or enhance the way DSPAM handles error
conditions. None of the patches do remove functionality or change fundamentally
how DSPAM works.

All patches looks OK by me, except  the better_error_handling.patch. I think
the last hunk should be:
+            if (!strcmp (user, username) || !strncmp(user, "*") ||
+               (!strncmp(user, "*@", 2) && !strcmp(user+1,
strchr(username,'@'))))

Am I right? 

(In reply to comment #16)
> All patches looks OK by me, except  the better_error_handling.patch. I think
> the last hunk should be:
> +            if (!strcmp (user, username) || !strncmp(user, "*") ||
> +               (!strncmp(user, "*@", 2) && !strcmp(user+1,
> strchr(username,'@'))))
> 
> Am I right? 
> 
I don't think so. The second strncmp will trigger under unwanted conditions.
The original one is more narrow.

(In reply to comment #17)
> (In reply to comment #16)
> > All patches looks OK by me, except  the better_error_handling.patch. I think
> > the last hunk should be:
> > +            if (!strcmp (user, username) || !strncmp(user, "*") ||
> > +               (!strncmp(user, "*@", 2) && !strcmp(user+1,
> > strchr(username,'@'))))
> > 
> > Am I right? 
> > 
> I don't think so. The second strncmp will trigger under unwanted conditions.
> The original one is more narrow.
> 
btw: If this is last hunk is a problem, then leave it out. It is not so ultra
important.

I cannot remove the hunk entirely because the original version have problems. 

IMO the condition that author wanted to implement was this:
  user == username || user == "*" || user == "*@domain_part_of_the_username"
You say that second condition should be user ==
"*anything_else_as_long_as_it_doesn_t_start_with_@", which doesn't make sense
to me.

(In reply to comment #19)
> I cannot remove the hunk entirely because the original version have problems. 
> 
> IMO the condition that author wanted to implement was this:
>   user == username || user == "*" || user == "*@domain_part_of_the_username"
> You say that second condition should be user ==
> "*anything_else_as_long_as_it_doesn_t_start_with_@", which doesn't make sense
> to me.
> 
You are right! It does not make sense. The patched code is handling groups and
username is passed as a parameter to the function and it holds the real
username (aka: user@domain.tld). While variable user holds the parsed users
from a group. user can be "*" (matching all users) and it could be
"*@domain.tld" (matching all users from one domain).

I can't remember why I have added this code to the group handling. The only
thing I can remember was that on the DSPAM mailing list a user had problems
with wrong username handling with groups. The patch resulted from this problem.
But I really can not remember now why this stupid handling. Especially since
the variable "user" is the parsed result from reading the group file.

Okay. Now I see the problem. Here a small c code for testing:
netbox cdadmin # cat dspam-test.c
int main(int argc, char *argv[]){
        if( argc < 3 ) {
                printf("USAGE: %s <username> <user>\n", argv[0]);
                return 1;
        }
        char *username = argv[1];
        char *user = argv[2];

        printf("Username: %s\n", username);
        printf("User: %s\n\n", user);

        if (!strcmp (user, username) || user[0] == '*' ||
           (!strncmp(user, "*@", 2) && !strcmp(user+2, strchr(username,'@'))))
        {
                printf("Matching original code\n");
        }
        if (!strcmp (user, username) || (user[0] == '*' && strncmp(user, "*@",
2)) ||
           (!strncmp(user, "*@", 2) && !strcmp(user+1, strchr(username,'@'))))
        {
                printf("Matching new code\n");
        }
}
netbox cdadmin # gcc dspam-test.c -o dspam-test
netbox cdadmin # ./dspam-test user@domain.tld "*@"
Username: user@domain.tld
User: *@

Matching original code
netbox cdadmin # ./dspam-test user@domain.tld "*@domain.tld"
Username: user@domain.tld
User: *@domain.tld

Matching original code
Matching new code
netbox cdadmin # ./dspam-test user@domain.tld "*whatever@domain.tld"
Username: user@domain.tld
User: *whatever@domain.tld

Matching original code
Matching new code
netbox cdadmin #


The patch is not good at all. I need to go back and fix this. Sorry.

The reason why the original DSPAM code is not okay is because you can add
"*@domain.tld" into a group definition (according to the documentation). The
original code will not honor that because it will match against "user[0] ==
'*'" and not evaluate the other conditions.

btw: Your code needs an additional argument for "strncmp".

(In reply to comment #21)
> btw: Your code needs an additional argument for "strncmp".

Ah, I intended to use strcmp, not strncmp. The second condition must be:
  !strcmp(user, "*") 

Created an attachment (id=136599) [edit]
files/dspam-3.8.0-better_error_handling.patch

Removed the patch for group handling. I need time to look at that issue. If
there should be any patching to the way groups are handled, then it sure does
not belong into a patch named "better_error_handling".

(In reply to comment #22)
> (In reply to comment #21)
> > btw: Your code needs an additional argument for "strncmp".
> 
> Ah, I intended to use strcmp, not strncmp. The second condition must be:
>   !strcmp(user, "*") 
> 
Yes! Using strcmp(user, "*") would be the best approach. If adding any patch
for that issue, then the patch should be:
@@ -2321,8 +2321,8 @@

           while (user != NULL)
           {
-            if (!strcmp (user, username) || user[0] == '*' ||
-               (!strncmp(user, "*@", 2) && !strcmp(user+2,
strchr(username,'@'))))
+            if (!strcmp (user, username) || !strncmp(user, "*") ||
+               (!strncmp(user, "*@", 2) && !strcmp(user+1,
strchr(username,'@'))))
             {

               /* If we're reporting a spam, report it as a spam to all other



I leave it up to you to add that patch. It would be better to separate it into
another patch file and not misuse the better_error_handling patch.

Thanks for looking into that.

// Steve

(In reply to comment #24)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > btw: Your code needs an additional argument for "strncmp".
> > 
> > Ah, I intended to use strcmp, not strncmp. The second condition must be:
> >   !strcmp(user, "*") 
> > 
> Yes! Using strcmp(user, "*") would be the best approach. If adding any patch
> for that issue, then the patch should be:
> @@ -2321,8 +2321,8 @@
> 
>            while (user != NULL)
>            {
> -            if (!strcmp (user, username) || user[0] == '*' ||
> -               (!strncmp(user, "*@", 2) && !strcmp(user+2,
> strchr(username,'@'))))
> +            if (!strcmp (user, username) || !strncmp(user, "*") ||
> +               (!strncmp(user, "*@", 2) && !strcmp(user+1,
> strchr(username,'@'))))
>              {
> 
>                /* If we're reporting a spam, report it as a spam to all other
> 
> 
> 
> I leave it up to you to add that patch. It would be better to separate it into
> another patch file and not misuse the better_error_handling patch.
> 
> Thanks for looking into that.
> 
> // Steve
> 
Shit! Used wrongly strncmp instead of strcmp. But you probably spotted it
already :)

Created an attachment (id=136602) [edit]
files/dspam-3.8.0-group_member_matching.patch

Alin's code for better group member matching.

@Alin: I have two other patches sitting around here. One of them adds
read_config support to libdpsam and the other makes dspam linking against the
shared libdspam library (making the dspam binary in my installation some kilo
byte smaller).

Should I post them as well?

Yes, please.

Created an attachment (id=136649) [edit]
files/dspam-3.8.0-linking_against_libdspam.patch

This is the patch to make the drivers link against libdspam. The patch needs a
patched DSPAM on the stage of 3.8.0-r7.

Created an attachment (id=136650) [edit]
files/dspam-3.8.0-use_libdspam.patch

This patch makes the dspam binary link against libdspam.
This patch allows libdspam to use the function read_config.
The patch needs a patched DSPAM on the stage of 3.8.0-r7.

(In reply to comment #28)
> Yes, please.
> 
The last two patches (dspam-3.8.0-linking_against_libdspam.patch and
dspam-3.8.0-use_libdspam.patch) are the one making the dspam binary smaller.

Committed in cvs as dspam-3.8.0-r8.
Thanks!

(In reply to comment #32)
> Committed in cvs as dspam-3.8.0-r8.
> Thanks!
> 
@Alin: Often I see people complaining about Gentoo developers and maintainers.
Well... I have to say that you are always very responsive and open minded.
Thank you for that!

Steve

Alin, Stevb,

Has either of you sent these patches upstream?

Thanks!

Seemant

I didn't sent anything to upstream (more exactly I don't remember doing such
thing), mostly because some patches are not original work (for example, some
patches have been derived from Debian counterparts).

(In reply to comment #34)
> Alin, Stevb,
> 
> Has either of you sent these patches upstream?
> 
> Thanks!
> 
> Seemant
> 
Hallo Seemant

There is no "upstream" any more. Okay. Sensory Networks is responsible for
development. See this link here:
http://mailing-list.nuclearelephant.com/6257.html

But sending them mails or patches is pointless. They don't respond.

Probably soon or later DSPAM will be forked. Currently a lot of people on the
mailing list are thinking about this move.

// SteveB

Ahh, I see! Thanks guys!  Sorry for the bugspam.