(In reply to Kip from comment #0)
> This was working fine in Ubuntu 12.04, but stopped working upon a fresh
> install to 13.04.

Both Ubuntu 12.04 and 13.04 are shipping SA 3.3.2, thus not a SA bug.
Please ask your distro for support.

Which it seems you did. Ubuntu bug 1178826 is the same report...

Kip, please do *not* report a single problem to multiple bug-trackers. Usually, there is only one bug, not many.
Keep in mind this is reporting a bug. If you are unsure whether something is a bug or not, or if you need help finding the root cause, please tend to the various support ways (mailing lists, forums) of your distro, and possibly also the products involved. A bugtracker is not a place for generic support.

Closing RESOLVED INVALID, not a SA bug.

(In reply to Karsten Bräckelmann from comment #1)
> Both Ubuntu 12.04 and 13.04 are shipping SA 3.3.2, thus not a SA bug.
> Please ask your distro for support.

Sorry, I don't follow. I thought this was the issue tracker for SA?

> Which it seems you did. Ubuntu bug 1178826 is the same report...

That's a downstream report, but the actual upstream it is linked to is
here.

> Kip, please do *not* report a single problem to multiple bug-trackers.
> Usually, there is only one bug, not many.

I'm sorry, but that's actually standard practise with Launchpad, and a
good thing at that. This is why we have a feature to link to the
upstream issue.

  <https://bugs.launchpad.net/ubuntu/+source/evolution/+bug/1178826>

> Keep in mind this is reporting a bug. If you are unsure whether something is
> a bug or not, or if you need help finding the root cause, please tend to the
> various support ways (mailing lists, forums) of your distro, and possibly
> also the products involved. A bugtracker is not a place for generic support.
> 
> Closing RESOLVED INVALID, not a SA bug.

I'm sorry, but that's just not a constructive answer. If it's not a bug,
then you could at least tell me why you believe it is not a bug. There
isn't any information in your post to indicate otherwise, even if you
are right.

(In reply to Kip from comment #2)
> > Both Ubuntu 12.04 and 13.04 are shipping SA 3.3.2, thus not a SA bug.
> > Please ask your distro for support.
> 
> Sorry, I don't follow. I thought this was the issue tracker for SA?

It is indeed.

> > Which it seems you did. Ubuntu bug 1178826 is the same report...
> 
> That's a downstream report, but the actual upstream it is linked to is
> here.

> I'm sorry, but that's actually standard practise with Launchpad, and a
> good thing at that. This is why we have a feature to link to the
> upstream issue.
>   <https://bugs.launchpad.net/ubuntu/+source/evolution/+bug/1178826>

That bug is filed against the "evolution" package in Ubuntu 13.04.

The upstream bug-tracker for Evolution is https://bugzilla.gnome.org
Product Evolution, Component Plugins (which is where the spam filter integration lives).

> > Closing RESOLVED INVALID, not a SA bug.
> 
> I'm sorry, but that's just not a constructive answer. If it's not a bug,
> then you could at least tell me why you believe it is not a bug. There
> isn't any information in your post to indicate otherwise, even if you
> are right.

My answer was constructive, in telling you where to head next for support.

Moreover, I did tell you why I believe it is not a SA bug. (Note the subtle difference in your and my phrase. I did not say "not a bug", but "not a SA bug".)  The first sentence of my comment 1 provided the reasoning for closing the bug report in this bug-tracker.

According to your original report comment 0, the bug did not happen with Ubuntu 12.04. It does happen with Ubuntu 13.04 after a fresh install. However, *both* versions are shipping the same SA upstream version 3.3.2.

It should be easy to understand that there is no code change in SA.

There has been a plethora of changes in Ubuntu in general, though, as well as major parts of the Linux kernel, GNOME, Evolution, Perl. Possibly even in the Ubuntu provided packages with custom patches.

And since this is a fresh install, there could even be missing dependencies, or configuration changes -- including really low-level config like enabling SELinux.


Closing RESOLVED INVALID, not a SA bug.

(FWIW, I am not yelling there, neither emphasizing "invalid", which is just the less friendly default way of saying "not a bug". These parts are uppercase, because that's how they are referred to in bugzilla. The case, as well as always clearly stating status changes is how I got used to during many years of GNOME bugsquad participation.)

(In reply to Karsten Bräckelmann from comment #3)
> That bug is filed against the "evolution" package in Ubuntu 13.04.

So then just fix it and replace it with SA...Done already...

> The upstream bug-tracker for Evolution is https://bugzilla.gnome.org
> Product Evolution, Component Plugins (which is where the spam filter
> integration lives).

See above.

> My answer was constructive, in telling you where to head next for support.

Use your common sense. Just looking at the console output you can see that the error is being raised in spamc process. Even if the OP associated it with Evolution, you could have simply commented on what you knew they had intended.

> Moreover, I did tell you why I believe it is not a SA bug. (Note the subtle
> difference in your and my phrase. I did not say "not a bug", but "not a SA
> bug".)  The first sentence of my comment 1 provided the reasoning for
> closing the bug report in this bug-tracker.
> 
> According to your original report comment 0, the bug did not happen with
> Ubuntu 12.04. It does happen with Ubuntu 13.04 after a fresh install.
> However, *both* versions are shipping the same SA upstream version 3.3.2.

Great, that's still not helpful.

> It should be easy to understand that there is no code change in SA.

Again, that's still not helpful. 

> There has been a plethora of changes in Ubuntu in general, though, as well
> as major parts of the Linux kernel, GNOME, Evolution, Perl. Possibly even in
> the Ubuntu provided packages with custom patches.
> 
> And since this is a fresh install, there could even be missing dependencies,
> or configuration changes -- including really low-level config like enabling
> SELinux.

That could well be, and if I had written spamc, I would have issued a more useful error message even if it was missing a dependency.

> (FWIW, I am not yelling there, neither emphasizing "invalid", which is just
> the less friendly default way of saying "not a bug". These parts are
> uppercase, because that's how they are referred to in bugzilla. The case, as
> well as always clearly stating status changes is how I got used to during
> many years of GNOME bugsquad participation.)

I'm not worried about it. I'm more concerned with the possible bug. If you don't have an answer why this is happening, that is ok, but just wait for someone who does.

(In reply to Kip from comment #4)
> Use your common sense. Just looking at the console output you can see that
> the error is being raised in spamc process. Even if the OP associated it
> with Evolution, you could have simply commented on what you knew they had
> intended.

The "OP [who] associated it with Evolution" is you. You filed both, the Ubuntu bug (against Evolution) and this bug (against SA upstream). Please do not refer to you in third person.

The error logged by syslog states failing to create a UNIX socket. By default, both spamc and spamd are using TCP port 783, not a UNIX socket.

The ps output in comment 0 shows the system-wide running spamd (pid 18498) to listen on the default TCP 783, whereas the per-user spamd daemon (pid 2789) is indeed using custom options to use a UNIX socket.

What are the owner, group and mode of that socket? What group(s) is the user 'kip' a member of?

The per-user spamd daemon has been launched by Evolution / its spam filter plugin with these custom options. Evolution also is what is starting a spamc process for each message to be scanned. Having both using the same unique, randomized socketpath and proper permissions is the responsibility of Evolution.


>                                                             [...]  If you
> don't have an answer why this is happening, that is ok, but just wait for
> someone who does.

You are seriously having an attitude problem.

Oh, and your recent comment of "This is still driving me nuts. Any help appreciated." to the Ubuntu bug report clearly shows you do not understand what a bug-tracker is. But you dismissed this when I pointed it out in comment 1, so I do not expect you to listen this time either.


This is not a SA bug. I am tired of you, though, and will wait for someone else to close this bug.

(In reply to Karsten Bräckelmann from comment #5)
> The "OP [who] associated it with Evolution" is you. You filed both, the
> Ubuntu bug (against Evolution) and this bug (against SA upstream). Please do
> not refer to you in third person.

This is painfully obvious and has nothing to do with the substance of the OP (original post - listen for the context).

> The error logged by syslog states failing to create a UNIX socket. By
> default, both spamc and spamd are using TCP port 783, not a UNIX socket.
> 
> The ps output in comment 0 shows the system-wide running spamd (pid 18498)
> to listen on the default TCP 783, whereas the per-user spamd daemon (pid
> 2789) is indeed using custom options to use a UNIX socket.

That's more useful.

> What are the owner, group and mode of that socket? What group(s) is the user
> 'kip' a member of?

-rw-rw-r-- 1 kip kip 5 May 27 12:26 /run/user/kip/spamd-pid-file-EFUQXW

$ groups kip
kip : kip adm cdrom sudo dip plugdev fuse lpadmin sambashare

> The per-user spamd daemon has been launched by Evolution / its spam filter
> plugin with these custom options. Evolution also is what is starting a spamc
> process for each message to be scanned. Having both using the same unique,
> randomized socketpath and proper permissions is the responsibility of
> Evolution.

$ ps aux | grep spam
...
kip       4659  0.0  1.3 158020 54676 ?        S    May27   0:16 /usr/bin/perl -T -w /usr/sbin/spamd --socketpath /run/user/kip/spamd-socket-path-BEUQXW --max-children=1 --pidfile /run/user/kip/spamd-pid-file-EFUQXW
...

$ cat /run/user/kip/spamd-pid-file-EFUQXW
4659

Yet /etc/default/spamassassin defines PIDFILE to /var/run/spamd.pid

$ cat /var/run/spamd.pid
1896

That process is also valid (root):
root      1896  0.0  0.0 145472  2580 ?        Ss   May27   0:17 /usr/sbin/spamd --create-prefs --max-children 5 --helper-home-dir -d --pidfile=/var/run/spamd.pid

> You are seriously having an attitude problem.

No, I think you're having an attutude problem. You don't know why this issue is happening, but rather than simply leaving it to someone who can help, you have to try and demand I conform to some arbitrary spur the moment social etiqutte you've just invented that has nothing to do with the actual issue.

> Oh, and your recent comment of "This is still driving me nuts. Any help
> appreciated." to the Ubuntu bug report clearly shows you do not understand
> what a bug-tracker is. But you dismissed this when I pointed it out in
> comment 1, so I do not expect you to listen this time either.

I wasn't talking about the bug. I was talking about you. 

As for not knowing what a bug tracker is for, that's a rather peculiar deductive gymnastic. But you're welcome to create your own. Any time anyone has an alleged attitude problem, they can post on your personnel tracker and await your remedial tutelage.

> This is not a SA bug. 

A bug is something software does that it shouldn't. Evolution may be invoking SA wrong, but the fact remains that the error messages in mail.err are not useful. It's a bug. It's not an attack on your ego. It's a constructive criticism of a computer program. Get over it.

> I am tired of you, though, and will wait for someone
> else to close this bug.

I think we both agree that you don't have any useful information here.

Closing RESOLVED INVALID, not a SA bug.

Created attachment 5144 [details]
Suggested patch.

Generated again r17546 of branch bzr+ssh://bazaar.launchpad.net/+branch/spamassassin.

I'm going to reference the LP branch because the SVN server was down when I tried. 

So addrbuf.sun_path which looks to be initialized from tp->socketpath on line 406, although not null, contains the empty string.

<https://bazaar.launchpad.net/~vcs-imports/spamassassin/trunk/view/head:/spamc/libspamc.c#L406>

This is why on line 428 in the call to libspamc_log the sun_path reference is dead. Like I said, the error message it was giving was not useful.

This is happening because socketpath is initialized via getopt_long loop to spamc_optarg, which is an empty string when either Evolution or any other client doesn't pass a parameter to -U. I don't know why it isn't being caught earlier because the switch option is declared with required_argument set.

Um, just chiming in as an independent third party observer here.

It appears Kip has found a bug and even provided a patch. At this point, a dev with commit privileges should be notified and merge the patch in. 

It seems there are claims about attitude, amongst others, on Kip or Karsten Bräckelmann. Let's not discuss that. Just forget it and move on. Let's just improve the software :D

(In reply to Zeroedout from comment #10)
> Um, just chiming in as an independent third party observer here.
> 
> It appears Kip has found a bug and even provided a patch. At this point, a
> dev with commit privileges should be notified and merge the patch in. 

Perhaps but the bug is only apparent with an incorrect implementation.  So all this does is give a better error message unless I'm reading it incorrectly.

(In reply to Kevin A. McGrail from comment #11)
> Perhaps but the bug is only apparent with an incorrect implementation.  

It doesn't matter. I will look into Evolution's incorrect usage anyways. SA's error control is still within the purview of SA.

> So
> all this does is give a better error message unless I'm reading it
> incorrectly.

That's a good start, but it also addresses the ignored 'required_argument' bit for -U in SA's getopt_long handling.

Look at adding the patch to spamc.  It isn't a bad patch despite the volatility of the discussion surrounding it.

Thanks Kevin. It does a couple things. First it makes sure that a path to a UNIX socket was provided when -U is specified to spamc. Then it makes sure that it points to an existing file that we can stat. Then it makes sure that that file is indeed a UNIX socket. On systems without stat() support, this isn't compiled.

(In reply to Kip from comment #8)
> Created attachment 5144 [details]
> Suggested patch.

The patch does not fix the issue. As-is, when invoked with the -U option without the required argument, spamc

 (a) reports the issue to STDERR,
 (b) dumps the Usage instructions to STDOUT, and
 (c) returns with exit code 64, EX_USAGE.

It is not a SA bug, that the spamc calling process ignores all of that.

In particular ignoring the exit code is a no-go. The calling process should have informed the user, without any need to go hunt in syslog files.


(In reply to Kip from comment #12)
> That's a good start, but it also addresses the ignored 'required_argument'
> bit for -U in SA's getopt_long handling.

$ spamc -U >> /dev/null; echo $?
Error in argument 1, char 2: argument required for option U
64

The required_argument bit is not ignored.


(In reply to Kip from comment #6)
> > Oh, and your recent comment of "This is still driving me nuts. Any help
> > appreciated." to the Ubuntu bug report [...]
> 
> I wasn't talking about the bug. I was talking about you.

The comment in question was added by you to launchpad issue #1178826
at 21:05 UTC.  My first appearance comment 1 was at 21:59 UTC, 50 minutes later.

(In reply to Karsten Bräckelmann from comment #15)
> The patch does not fix the issue. 

The patch fixes the issue. I justed tested it and it works fine.

> As-is, when invoked with the -U option
> without the required argument, spamc
> 
>  (a) reports the issue to STDERR,

How many people run spamc manually from the command line and observe stderr? It's typically invoked by some mail kind of mail client behind the scenes where that information would not be immediately visible. The standard place for it to log errors would be in /var/log/mail.err.

>  (b) dumps the Usage instructions to STDOUT, and

Actually it didn't when I ran it.

>  (c) returns with exit code 64, EX_USAGE.

Again, it didn't, hence the patch. If that happened, addrbuf.sun_path wouldn't have been empty when mail.err was written to.

> It is not a SA bug, that the spamc calling process ignores all of that.

It's both a SA and an Evolution bug. Get over it.

> In particular ignoring the exit code is a no-go. The calling process should
> have informed the user, without any need to go hunt in syslog files.

See above.

> $ spamc -U >> /dev/null; echo $?
> Error in argument 1, char 2: argument required for option U
> 64
> 
> The required_argument bit is not ignored.

That's great that it worked for you, but as I've already pointed out multiple times, that wasn't replicated at my end.

> > I wasn't talking about the bug. I was talking about you.
> 
> The comment in question was added by you to launchpad issue #1178826
> at 21:05 UTC.  My first appearance comment 1 was at 21:59 UTC, 50 minutes
> later.

Do you have aspergers?

Karsten, if you don't have anything constructive to add, then stop trolling and let us patch the problem rather than compounding it with your presence.

$ spamc -U "" >> /dev/null; echo $?
64

No error message on either stdout or stderr, but now with the patch, the following in mail.err:

spamc[29471]: -U switch expects socketpath to spamd, but none provided

In reply to Kip Warner, comment #16 and comment #17:

Consider this the final warning. Your account will be suspended, if you keep up your attitude and tone.

The Ubuntu Code of Conduct you signed does not apply to Ubuntu or launchpad only.

Kip, you have much to learn about effective discussions to achieve your means and insulting our team is not going to get your patch done any faster.  This is a VERY low priority issue because it is improper usage.  I will look at your patch to see if it has some merit.

It's very likely that the exit code of 64 is expected and documented and your patch is adding unnecessary output to a product intended to be daemonized.

Regards,
KAM

Kevi(In reply to Kevin A. McGrail from comment #20)
> Kip, you have much to learn about effective discussions to achieve your
> means and insulting our team is not going to get your patch done any faster.
> This is a VERY low priority issue because it is improper usage.  I will look
> at your patch to see if it has some merit.
> 
> It's very likely that the exit code of 64 is expected and documented and
> your patch is adding unnecessary output to a product intended to be
> daemonized.
> 
> Regards,
> KAM

Kevin, if you want to lecture me in fatherly tones, that's fine. Everything you're accusing me of your buddy was doing. In any case, I'm really not here for that, but to fix a bug. If you're not interested in that, that's fine and we can solve the problem via a downstream patch. If your buddy feels that disabling someone's account because they lost an argument is the best way to solve a problem a patch has been provided for, then go for it.

This is a bug tracker not a forum.  I'm tired of dealing with your patch and closing the case.  Congratulations, you just pissed off the only person interested in helping you.

(In reply to Kevin A. McGrail from comment #20)
> ...a product intended to be daemonized.

Exactly. Hence why the meaningful error message should be in mail.err and not on just stdout or stderr.

(In reply to Kevin A. McGrail from comment #22)
> This is a bug tracker not a forum.  I'm tired of dealing with your patch and
> closing the case.  Congratulations, you just pissed off the only person
> interested in helping you.

There's no technical solution to an attitude problem. So don't worry about it, since patching my installation isn't dependent on you anyways.

I'm not personalizing a constructive and valid criticism of spamc. To do so would be an attitude problem, to wit Karsten. 

We all agree that Evolution is invoking spamc wrong. That's not the issue here and we can take that up with Evolution's devs. The issue here is that that would have been far more evident had spamc's error control governing its interface been more robust and emit a more meaningful error message where, as you say, a daemonized application ought. It is treating an empty "" parameter to -U as an actual path to a socket file. If that was intentional by design, the asserts on tp->socketpath, e.g. line 406, would have checked for !strlen(tp->socketpath) as well as just to make sure it wasn't going to dereference a NULL pointer.

(In reply to Kip Warner from comment #24)
> Changed Resolution to FIXED

Kip, please stop changing the Status and Resolution. These are clearly defined:

  FIXED
    A fix for this bug is checked into the tree and tested.
  INVALID
    The problem described is not a bug.

Resolution INVALID. This is not a SA bug.

(In reply to Karsten Bräckelmann from comment #26)
> Kip, please stop changing the Status and Resolution. These are clearly
> defined:

Actually, to be fair, that last change was accidental.

> Resolution INVALID. This is not a SA bug.

Apparently that's a matter of conjecture and I'm confident that had Kevin supplied the same identical patch, you wouldn't be giving me attitude.

(In reply to Karsten Bräckelmann from comment #26)
> This is not a SA bug.

It's a bug. It's nothing personal. It's code. Get over it.

(In reply to Kip from comment #27)
> Actually, to be fair, that last change was accidental.

OK, no problem then. Does happen.


> > Resolution INVALID. This is not a SA bug.
> 
> Apparently that's a matter of conjecture and I'm confident that had Kevin
> supplied the same identical patch, you wouldn't be giving me attitude.

Frankly, I wouldn't just accept the patch by Kevin either.

Anyway, while the patch adds some really fine-grained testing and logging of differentiated explicit error messages, the real issue is not fixed by it. Evolution will still fail to filter spam using SA, because it still does not provide the correct socket, where its own per-user spamd is listening.

Now that we know the -U socket argument is passed as empty string by Evolution, that's what to report to GNOME bugzilla (assuming this isn't an Ubuntu custom patch).

(In reply to Karsten Bräckelmann from comment #29)
> Anyway, while the patch adds some really fine-grained testing and logging of
> differentiated explicit error messages, the real issue is not fixed by it.
> Evolution will still fail to filter spam using SA, because it still does not
> provide the correct socket, where its own per-user spamd is listening.

That depends how you want to frame the issue. For you, it's an issue with Evolution's invocation of SA. For me, it's that, in addition to another issue in spamc's poor error control making the former not evident to most people who experience this same problem. 

I've also added some additional check to ensure that the socket path, if a non-empty string is provided is sane in a portable manner.

> Now that we know the -U socket argument is passed as empty string by
> Evolution, that's what to report to GNOME bugzilla (assuming this isn't an
> Ubuntu custom patch).

And that will come too, but both parties need to take responsibility for their respective issue.

What it also comes down to is the robustness principle. The software should be accomodating of input, but be well defined in its behaviour. In this context, different types of bad input that are categorically / effectively the same should yield the same effect.

(In reply to Kip from comment #31)
> What it also comes down to is the robustness principle. The software should
> be accomodating of input, but be well defined in its behaviour.

spamc returns an error indicating exit code. It is the calling process's responsibility to check this.

(In reply to Karsten Bräckelmann from comment #32)
> spamc returns an error indicating exit code. It is the calling process's
> responsibility to check this.

You're missing the point. We are both in agreement about this. The point is that Evolutions' devs are as fallible as SA's. Evolution isn't checking the return as it should and that's a problem. The other problem is that this former problem could occur with any mail client or other tool that uses SA and the devs could have made the same mistake - and that it could be identified and rectified faster with the patch I've already supplied.

It is definitely half a bug in SA because there is no reason for the connect() call to rely on an empty string since that doesn't have any meaningful semantics in this context.

One other thing,

$ spamc -U ""

, before my patch does not exit at all, but appears to hang. This means that in this case, even if Evolution checked a spamc return code, it would still be pointless if it never receives any at all.

(In reply to Kip from comment #34)
> , before my patch does not exit at all, but appears to hang. This means that
> in this case, even if Evolution checked a spamc return code, it would still
> be pointless if it never receives any at all.

Yes, it appears to hang. It waits for input on STDIN. The message spamc is supposed to pass along to spamd to check. Unless the calling process fails to pipe a message (which would be an unrelated bug), it will not wait indefinitely.

Can we please stop beating this dead horse?

(In reply to Karsten Bräckelmann from comment #35)
> Yes, it appears to hang. It waits for input on STDIN. The message spamc is
> supposed to pass along to spamd to check. 

How is it "supposed to pass along to spamd to check" if the socket file is not sane and it can already determine that before even trying to connect, let alone have the user or mail client wait "indefinitely"?

> Can we please stop beating this dead horse?

No one is beating a dead horse. Karsten, if you don't want to deal with this, that's fine, but please just drop it and give it a rest.

It's just basic first year computer science, if even that. A program should check for malformed input.

(In reply to Kip from comment #36)
> > Yes, it appears to hang. It waits for input on STDIN. The message spamc is
> > supposed to pass along to spamd to check. 
> 
> How is it "supposed to pass along to spamd to check" if the socket file is
> not sane and it can already determine that before even trying to connect,
> let alone have the user or mail client wait "indefinitely"?

Spamc *could* check for a valid socket or listening process on the TCP port, before processing STDIN at all. That it does not is pretty much desired and documented behavior.

Spamc will not pass the message to spamd, if it exceeds a max-size threshold. To know the message's size, spamc needs to process STDIN. See man spamc:

 -s max_size, --max-size=max_size
     Set the maximum message size which will be sent to spamd -- any
     bigger than this threshold and the message will be returned
     unprocessed (default: 500 KB).  If spamc gets handed a message
     bigger than this, it won't be passed to spamd.  The maximum message
     size is 256 MB.


(In reply to Kip from comment #37)
> It's just basic first year computer science, if even that. A program should
> check for malformed input.

$ echo -n | spamc -U ""; echo $?
74
$ echo | spamc -U ""; echo $?
69

$ man spamc | egrep '(69|74)'
           EX_UNAVAILABLE  69  service unavailable
           EX_IOERR        74  input/output error


The recent arguing and dissection of spamc behavior is outside the scope of this report, though, and bugzilla is not an appropriate place for discussions.

Please feel free to move the more general discussion parts to the dev list.

(In reply to Karsten Bräckelmann from comment #38)
> Spamc *could* check for a valid socket or listening process on the TCP port,
> before processing STDIN at all. 

Yes, that's what the patch solves in the case of a UNIX socket.

> That it does not is pretty much desired and documented behavior.

Desired? Speak for yourself.

> Spamc will not pass the message to spamd, if it exceeds a max-size
> threshold. To know the message's size, spamc needs to process STDIN. See man
> spamc:

That has nothing to do with the substance of my comment. I'm not concerned with the max-size.


> $ echo -n | spamc -U ""; echo $?
> 74

Great, now model the problem accurately by repeating the same test again, but without piping the newline into it.

> Please feel free to move the more general discussion parts to the dev list.

I'm afraid you don't seem to understand how an issue tracker works. Sometimes people disagree with one another, and we don't restort to stasis of place out of desperation when it is convenient.

Hey guys,

I tested `spamc -U ""` on the command line, and as Kip pointed out, it hangs on STDIN. I understand the motivation for this, but I don't think it should wait at all if the path to the daemon is broken.

I don't understand why this is not an issue in SA. In my novice opinion, let's say I try to "cat" a non-existent file, using `cat blah.txt`. If the file is not found, cat politely tells me instead of barking out an error code.

From what I can see, spamc is doing something similar, except it passes the message onto spamd with whatever is passed into STDIN. If that message has nowhere to go (i.e. "" is an invalid path), spamc should inform the calling process (in this case, Evolution and its developers). 

I've tested Kip's patch and can confirm that it works and resolves this issue, and will probably give the Evolution devs a much easier time in fixing their bug. Let's try to help the Evolution guys a little. I think well-documented and helpful exit codes/messages will only serve to help future developers.

If this means rewriting error codes and messages so they're more verbose, then so be it, but I think this case clearly illustrates that the current method is not completely intuitive for developers trying to integrate spamassassin into their work.

My two cents. Thank you for reading.

VR

(In reply to Karsten Bräckelmann from comment #38)
> Spamc *could* check for a valid socket or listening process on the TCP port,
> before processing STDIN at all. 

One other thing, it doesn't seem to check for a valid socket at all in some cases, as I've already illustrated. So it the point on whether it could check before or after consuming some of stdin is irrelevant.

(In reply to VR from comment #40)
> I tested `spamc -U ""` on the command line, and as Kip pointed out, it hangs
> on STDIN. I understand the motivation for this, but I don't think it should
> wait at all if the path to the daemon is broken.

Thanks for replicating and testing the patch, VR. =)

$ echo -n | spamc -U ""; echo $?
0

Also, I just ran `echo -n | spamc -U ""; echo $?`. The exit code was 0. This is a bug, because it implies there is no error, when there is.

VR

(In reply to VR from comment #43)
> Also, I just ran `echo -n | spamc -U ""; echo $?`. The exit code was 0. This
> is a bug, because it implies there is no error, when there is.
> 
> VR

I've managed to replicate this on a mipsel machine separate from my workstation. It's definitely a bug, but thankfully it's been patched.

I get the same result, Arch Linux 64-bit install. spamc compiled from the AUR.

[zeroedout@zeroarch ~]$ echo -n | spamc -U ""; echo $?
0

Seems the "Status" on this issue is incorrect - this is indeed a bug.

(In reply to Kip from comment #39)
> > Please feel free to move the more general discussion parts to the dev list.
> 
> I'm afraid you don't seem to understand how an issue tracker works.

Quite a bold statement without any check on my background.


(In reply to VR from comment #43)
> Also, I just ran `echo -n | spamc -U ""; echo $?`. The exit code was 0. This
> is a bug, because it implies there is no error, when there is.

Nope, it is not. *sigh*  It seems some of the "test-cases" added here by all of us are partially using spamc global configuration that affects the test.

From the spamc man-page about EXIT CODES:

  "By default, spamc will use the 'safe fallback' error recovery method.
   That means, it will always exit with an exit code if 0, even if an
   error was encountered.  If any error occurrs, it will simply pass
   through the unaltered message."

With the -x, --no-safe-fallback option set, for the given test-cases, spamc will return 0 *only* with at least one byte on STDIN and without the erroneous -U "". It will complain with exit code 69 or 74 in any other mentioned test-case.

This is the documented and desired (read: agreed upon to be default) behavior. If anyone desires something else, the man-page advices him how to set global spamc configuration and change it to his heart's content.

Test-cases provided to show spamc falsely reporting "no error", whereas it allegedly should have, are based on not taking the complete documentation and default settings into account.

Please feel free to not simply take my word for it, but actually try it yourself.


The original report comment 0 does not describe a SA bug.
This bug report is correctly RESOLVED INVALID.

Please refrain from adding further comments here, unless you have something substantial and new to add to this report.

Feel free to move any further discussion to the dev list. Bugzilla is not an appropriate place for this type of discussions and arguing.


VR, with all due respect: Your bugzilla account has been created on 2013-05-29 22:36:50 UTC, just minutes before your first comment 40 at 22:50:57 UTC. You have never been subscribed with this account's address to the SA dev@ list, either, so you didn't get the on-list bug mail.

Please, enlighten us who you are, or how you noticed this report.

(In reply to Karsten Bräckelmann from comment #46)
> Nope, it is not. *sigh*  It seems some of the "test-cases" added here by all
> of us are partially using spamc global configuration that affects the test.

Sounds like another bug.

> From the spamc man-page about EXIT CODES:
> 
>   "By default, spamc will use the 'safe fallback' error recovery method.
>    That means, it will always exit with an exit code if 0, even if an
>    error was encountered.  If any error occurrs, it will simply pass
>    through the unaltered message."

Since you hadn't realized that until after the fact that three other people replicated the same issue, it drives my point home again that the error control is not adequate.

> Please refrain from adding further comments here, unless you have something
> substantial and new to add to this report.

I've asked you the same, and while I've supplied a patch, and you've already encouraged us that you would stop trolling (comment #5), you're still here.

> Feel free to move any further discussion to the dev list. Bugzilla is not an
> appropriate place for this type of discussions and arguing.

Neither the issue tracker, nor the mailing list, is an appropriate place to troll people who are helping you fix and improve your software.

> VR, with all due respect: Your bugzilla account has been created on
> 2013-05-29 22:36:50 UTC, just minutes before your first comment 40 at
> 22:50:57 UTC. You have never been subscribed with this account's address to
> the SA dev@ list, either, so you didn't get the on-list bug mail.
> 
> Please, enlighten us who you are, or how you noticed this report.

It's the distro's fault. It's Evolution's fault. It's the patch writer's fault. Now the the other end users who replicated the same issue are suspect. But SA is infallible. 

It doesn't matter who he is and he doesn't need to be trolled either. There is no requirement that you monitor this ticket. Just leave us be.

(In reply to Karsten Bräckelmann from comment #46)
> Nope, it is not. *sigh*  It seems some of the "test-cases" added here by all
> of us are partially using spamc global configuration that affects the test.

Shouldn't the test cases be processed during a make check target and not as the default setting of a released binary?

The volatility of the discussion starts with this:

(In reply to Karsten Bräckelmann from comment #1)

> Kip, please do *not* report a single problem to multiple bug-trackers.
> Usually, there is only one bug, not many.
> Keep in mind this is reporting a bug. If you are unsure whether something is
> a bug or not, or if you need help finding the root cause, please tend to the
> various support ways (mailing lists, forums) of your distro, and possibly
> also the products involved. A bugtracker is not a place for generic support.
> 
> Closing RESOLVED INVALID, not a SA bug.

First, it's ironic that Kip was right to report to multiple bug-trackers, considering the conclusion of the problem having contributions from multiple parties.

Second, this reply is patronizing and unhelpful. It seems prematurely naive to immediately assume that a bug reporter is asking for "generic support" without actually looking into the bug or attempting to reproduce. I make this assumption because it isn't until comment #5 that we get something remotely helpful:

(In reply to Karsten Bräckelmann from comment #5)
> The error logged by syslog states failing to create a UNIX socket. By
> default, both spamc and spamd are using TCP port 783, not a UNIX socket.
> 
> The ps output in comment 0 shows the system-wide running spamd (pid 18498)
> to listen on the default TCP 783, whereas the per-user spamd daemon (pid
> 2789) is indeed using custom options to use a UNIX socket.
> 
> What are the owner, group and mode of that socket? What group(s) is the user
> 'kip' a member of?  

If this had been comment #2 instead of comment #5, this thread would probably not have gone in the direction it did.

Another piece of irony here:

(In reply to Karsten Bräckelmann from comment #46)
> (In reply to Kip from comment #39)
> > > Please feel free to move the more general discussion parts to the dev list.
> > 
> > I'm afraid you don't seem to understand how an issue tracker works.
> 
> Quite a bold statement without any check on my background.

The interesting part here is how Karsten didn't check Kip's background first before making the initial condescending assumption in comment #2. As the rest of the discussion will tell, Kip knows exactly what he's doing, to the point that he's provided a patch. If Karsten had done a background check on Kip before comment #2 or shown him enough respect to look into the problem, there would be no volatility in this discussion and the bug would have been quickly resolved. 

Pardon the repetition of my vocabulary, but as it stands, Karsten was the first to be dismissive, condescending/patronizing and unhelpful. I see no attitude in any of Kip's initial responses, and definitely find the tone of his later ones justified, if not too polite, considering Karsten's condescension. Is there a language barrier somewhere that is causing misinterpretation?

It's pretty clear that Kip is just trying to contribute to improving SA. What I don't understand is the complete and total opposition to accepting that. Ego problems in real life need to be kept out of this. Nobody is saying you are less than you are. As Kip has mentioned multiple times, this is just code. Don't take it personally. Get over it. It's not about you, it's about working together to improve the software. Anything other than that is just a hindrance to progress in general. What's the point of having a bug tracker if not to make progress?

 
> VR, with all due respect: Your bugzilla account has been created on
> 2013-05-29 22:36:50 UTC, just minutes before your first comment 40 at
> 22:50:57 UTC. You have never been subscribed with this account's address to
> the SA dev@ list, either, so you didn't get the on-list bug mail.
> 
> Please, enlighten us who you are, or how you noticed this report.

I am not a spam bot or anything automatic. I am a real person who is trying to help replicate the problem that Kip saw. I'm afraid that's all the information I am comfortable parting with to you. I created the account in order to post a reply, but it doesn't mean I wasn't trying to replicate the problem hours or even days before. 

I'm not trying to attack you in any way, Karsten. I'm genuinely trying to use spamassassin and get it working with Evolution. Having many people reply to you is a sign that your software is well-liked and well-used. We wouldn't waste our time if we did not like the product. Nobody is out to get you or upset you in any way. We're only offering suggestions, at the end of the day.

I note your comment about the exit status defaulting to 0 and that it is desired behaviour for your team. At this point it is a matter of my philosophy against SA's. I can agree to disagree with you, but I really think this is something that should get reconsidered by your team because it will avoid a lot of headache for downstream developers and integrated applications.

(In reply to VR from comment #50)
> I note your comment about the exit status defaulting to 0 and that it is
> desired behaviour for your team. At this point it is a matter of my
> philosophy against SA's. I can agree to disagree with you, but I really
> think this is something that should get reconsidered by your team because it
> will avoid a lot of headache for downstream developers and integrated
> applications.

The default is that when something goes wrong the mail is still delivered. Many people will prefer to get their mail with spam than have it delayed. To break this useful and long-established behaviour would be absurd. Most project that rely on SpamAssassin don't use spamc anyway.

(In reply to RW from comment #51)
> Most project that rely on SpamAssassin don't use spamc anyway.

There are over ten million people worldwide using Ubuntu. Of that, for a significant portion of its life, most people using a native mail client were probably using Evolution. Many still continue to do so, even though Thunderbird has replaced it as its default mail client. KMail, Evolution, and many other mail clients will pull or suggest to the user a distro packaged SA dependency either during installation time, or during user configuration. Evolution needs the spamc binary present in order to pass the email to the daemon for inspection. That's probably why people not on the dev mailing list, including myself, are simply googling for this issue and finding that others are experiencing the same issue.

(In reply to Kip from comment #52)
> (In reply to RW from comment #51)
> > Most project that rely on SpamAssassin don't use spamc anyway.
> 
> There are over ten million people worldwide using Ubuntu. ... 
> Evolution needs the spamc binary present in order to pass the
> email to the daemon for inspection. 

None of those people, other that the plug-in maintainer, need to understand spamc. The only bug is in evolution; there is no bug in SpamAssassin.

(In reply to RW from comment #53)
> None of those people, other that the plug-in maintainer, need to understand
> spamc. 

Apparently they do if spamc deviates from decades long well established conventions.

> The only bug is in evolution; there is no bug in SpamAssassin.

Correct, withstanding the patch.

$ ls "" &> /dev/null ; echo $?
2

$ gcc -c "" &> /dev/null ; echo $?
4

$ ln "" &> /dev/null ; echo $?
1

$ php "" &> /dev/null ; echo $?
127

$ xdg-open "" &> /dev/null ; echo $?
1

$ curl "" &> /dev/null ; echo $?
3

$ ssh "" &> /dev/null ; echo $?
255

$ gpg "" &> /dev/null ; echo $?
2

$ python "" &> /dev/null ; echo $?
1

$ echo -n | spamc -U "" &> /dev/null ; echo $?
0

$ egrep -nr "EXIT_SUCCESS" /usr/include/
/usr/include/stdlib.h:134:#define	EXIT_SUCCESS	0	/* Successful exit status.  */

You can keep repeating yourself until the cows come home, it doesn't change anything. There is a bug in Evolution and there was the aforementioned one in SpamAssassin up until recently.

(In reply to VR from comment #40)
> I tested `spamc -U ""` on the command line, and as Kip pointed out, it hangs
> on STDIN. I understand the motivation for this, but I don't think it should
> wait at all if the path to the daemon is broken.

I just checked out of curiousity to find out how many instances of spamc were just sitting there hanging as a result of Evolution's incorrect invocation and spamc's buggy error control:

$ ps aux | grep "[spamc]" | wc -l
494

From Evolution's NEWS:

   ...
   #560420: spamc and spamassassin use error codes >= 64 to denote 
   execution errors. Positive error codes < 64 means the message was
   identified as Spam (hp@syntomax.com)
   ...

It turns out Evolution is checking the return codes correctly from what I can see. But knowing which return code to check for is pointless when spamc hangs indefinitely.

Evolution is specifying -x / --no-safe-fallback in the argv.

https://git.gnome.org/browse/evolution/tree/modules/spamassassin/evolution-spamassassin.c#n417

(In reply to Kip from comment #56)
> From Evolution's NEWS:

> It turns out Evolution is checking the return codes correctly from what I
> can see. But knowing which return code to check for is pointless when spamc
> hangs indefinitely.

echo -n  | spamc  -x  -U ""

doesn't hang. What is Evolution doing differently?

(In reply to RW from comment #58)
> echo -n  | spamc  -x  -U ""
> 
> doesn't hang. What is Evolution doing differently?

I'm having difficulty getting the dependencies for Evolution head, so I can't debug and patch it easily to answer your question. However, it looks as though it might have already been patched upstream which means that, of the two projects, only SA's head still needs to be patched.

Evolution's faulty invocation probably originated in this routine:

  <https://git.gnome.org/browse/evolution/tree/modules/spamassassin/evolution-spamassassin.c#n596>

Some comments from Evolution's developers regarding SpamAssassin which may help the latter's developers improve their program's interface and avoid issues like this to begin with:

	/* XXX SpamAssassin could really benefit from a D-Bus interface
	 *     these days.  These tests are just needlessly painful for
	 *     clients trying to talk to an already-running spamd. */

I agree completely with them. DBus interfaces are not difficult to write with glib's GIO interface. It is well written and I've implemented such things before.

I think that's about as likely as Sendmail getting a dbus interface - SpamAssassin is server software.

Closing RESOLVED INVALID, not a SA bug.

(In reply to RW from comment #60)
> I think that's about as likely as Sendmail getting a dbus interface -
> SpamAssassin is server software.

There is no shortage of server software far more complex than SpamAssassin in the field that relies on DBus for IPC. But don't worry about it, we can patch this downstream.

> It turns out Evolution is checking the return codes correctly from what I
> can see. But knowing which return code to check for is pointless when spamc
> hangs indefinitely.

There are no hanging spamc processes. You might want to re-evaluate your command in comment 55.

> Evolution is specifying -x / --no-safe-fallback in the argv.

Nope. The referenced code is merely testing for a running spamd daemon. Evolution is not using --no-safe-fallback, but the --check option with spamc. The default of safe-fallback still is in effect.


(In reply to Kip Warner from comment #62)
Another "accidental" Resolution change to FIXED, just like you tried in comment 24 before?

Resolution INVALID.

(In reply to Karsten Bräckelmann from comment #63)
> There are no hanging spamc processes. You might want to re-evaluate your
> command in comment 55.

Don't be ridiculous. There were over 400 of them. Next time it happens, I'll post a screenshot in htop.

> Nope. The referenced code is merely testing for a running spamd daemon.
> Evolution is not using --no-safe-fallback, but the --check option with
> spamc. The default of safe-fallback still is in effect.

Re-read my post. I didn't say under what conditions it was using it.

> Another "accidental" Resolution change to FIXED, just like you tried in
> comment 24 before?

I clicked Save Changes and your tracker changed the status without my having explicitly selected to do so. Since apparently this bug tracker is not the appropriate place to discuss bugs, I won't mention another one.

> > There are no hanging spamc processes. You might want to re-evaluate your
> > command in comment 55.
> 
> Don't be ridiculous. There were over 400 of them.

Well, there were more than 400... lines of 'ps aux' output, which had at least one character of the char class [acmps].

Given the current month is May and you're (presumably) in Vancouver...


> I clicked Save Changes and your tracker changed the status without my having
> explicitly selected to do so. Since apparently this bug tracker is not the
> appropriate place to discuss bugs, I won't mention another one.

Please report Bugzilla issues to upstream https://bugzilla.mozilla.org/,
Product Bugzilla.

(In reply to Karsten Bräckelmann from comment #65)
> Well, there were more than 400... lines of 'ps aux' output, which had at
> least one character of the char class [acmps].

Correct, except that I ran it before the character class regex and saw over four hundred processes of spamc. I added the character class to clean up the output and just display spamc.

> Given the current month is May and you're (presumably) in Vancouver...

You are on dairy.

> I added the character class to clean up the output and just display spamc.

Here, you have absolutely no idea what you are talking about.

> You are on dairy.

And here *I* have no idea what you are talking about. But hey, not everyone is a native English speaker.

(In reply to Karsten Bräckelmann from comment #67)
> > I added the character class to clean up the output and just display spamc.
> 
> Here, you have absolutely no idea what you are talking about.

It doesn't matter if the regexp to cleanup the output was wrong or not. You're missing the point (again). The point was that there were over four hundred spamc processes hanging. This was the command I originally ran if you insist on being a pedant...

$ ps aux | grep spamc
...
(400+ lines of spamc processes followed)
...

> You're missing the point (again).

You failed to make your point (again). You failed to provide even the tiniest bit of evidence.

> The point was that there were over four hundred spamc processes hanging.

An apparently hanging spamc process pretty much is possible only under one particular circumstance -- an open STDIN to the calling process, which does not pipe data but keep it open idle. Exactly the same as

  $ cat

That would be yet another bug not affecting SA (or coreutils in the case of 'cat'), but whatever called it.

(In reply to Kip from comment #55)
> I just checked out of curiousity to find out how many instances of spamc
> were just sitting there hanging as a result of Evolution's incorrect
> invocation and spamc's buggy error control:
> 
> $ ps aux | grep "[spamc]" | wc -l
> 494

If any of those were actually spawned by Evolution, could you pick one or two of them and dig around under /proc and get their invocation arguments?

There are embedded NUL bytes, and we're also looking for possible empty arguments, so I suggest this:
   od -c /proc/PID#/cmdline

(In reply to Karsten Bräckelmann from comment #69)
> You failed to make your point (again). You failed to provide even the
> tiniest bit of evidence.

I think there is definitely a language barrier at work here. In my usage of the language in this context, a quantity of over 400 is not small.

> An apparently hanging spamc process pretty much is possible only under one
> particular circumstance -- an open STDIN to the calling process, which does
> not pipe data but keep it open idle. Exactly the same as
> 
>   $ cat

$ cat "" ; echo $?
cat: : No such file or directory
1

(In reply to John Hardin from comment #70)
> If any of those were actually spawned by Evolution, could you pick one or
> two of them and dig around under /proc and get their invocation arguments?
> 
> There are embedded NUL bytes, and we're also looking for possible empty
> arguments, so I suggest this:
>    od -c /proc/PID#/cmdline

Thanks John. I'll give that a try next time I see them hanging.

> > You failed to make your point (again). You failed to provide even the
> > tiniest bit of evidence.
> 
> I think there is definitely a language barrier at work here. In my usage of
> the language in this context, a quantity of over 400 is not small.

There is no language barrier, if you do speak English.

In terms of hanging processes, 400 is not small. Agreed. Though that is only what you claim. You did not show even a single line of output. You snipped every single line that could stand as a hint (evidence not yet required).


> > An apparently hanging spamc process pretty much is possible only under one
> > particular circumstance -- an open STDIN to the calling process, which does
> > not pipe data but keep it open idle. Exactly the same as
> > 
> >   $ cat
> 
> $ cat "" ; echo $?

You are not seriously trying to sneak that in, are you? In your example, 'cat' does not listen on STDIN but tries to open the given file named empty string.


> (In reply to John Hardin from comment #70)
> Thanks John. I'll give that a try next time I see them hanging.

What do you mean, next time you see them hanging?

According to your claims, the patch attachment 5144 [details] fixes the issue. And you applied it locally. Thus, according to your claims, you will never see them hanging again.

But hey, even better. We got a way to reproduce the issue.

Revert your own patch. Then send yourself a message. Hanging process right there, if your claim is correct.

Extra points for 400 hanging processes.

(In reply to Karsten Bräckelmann from comment #72)
> In terms of hanging processes, 400 is not small. Agreed. Though that is only
> what you claim. You did not show even a single line of output. You snipped
> every single line that could stand as a hint (evidence not yet required).

Even if I had pasted it without cleaning it up, you would have then gone on to say that it was fabricated.

> You are not seriously trying to sneak that in, are you? In your example,
> 'cat' does not listen on STDIN but tries to open the given file named empty
> string.

Yes, just like the socket file parameter to spamc.

> What do you mean, next time you see them hanging?
> 
> According to your claims, the patch attachment 5144 [details] fixes the
> issue. And you applied it locally. Thus, according to your claims, you will
> never see them hanging again.

Correct, except I wasn't running my patched binary when I captured the output that I did.

> Extra points for 400 hanging processes.

Karsten, go play in the traffic.

> > You are not seriously trying to sneak that in, are you? In your example,
> > 'cat' does not listen on STDIN but tries to open the given file named empty
> > string.
> 
> Yes, just like the socket file parameter to spamc.

The socket will be used by spamc as STDOUT to pass the message to spamd, after STDIN has been digested.

In your command, 'cat' uses the empty string as file name to open, ignoring STDIN completely.

s/just like /nothing like /

(In reply to Karsten Bräckelmann from comment #74)
> The socket will be used by spamc as STDOUT to pass the message to spamd,
> after STDIN has been digested.
>
> In your command, 'cat' uses the empty string as file name to open, ignoring
> STDIN completely.
> 
> s/just like /nothing like /

GNU cat and spamc are not the same program. They do different things with their input. Anyone with even the most basic first year education in computer science understands that it is the programmer's responsibility to validate user input so far as is reasonable.

> GNU cat and spamc are not the same program. They do different things with
> their input.

Both swallow their input on STDIN.

And other than a minimal SPAMD protocol header based on unrelated command line arguments, both are doing the same with STDIN -- pipe it to STDOUT.


> Anyone with even the most basic first year education in
> computer science understands that it is the programmer's responsibility to
> validate user input so far as is reasonable.

Validating input comes *after* receiving input. Cannot validate while still waiting.

Anyone with even the most basic first year education... Sorry, my mistake.

(In reply to Karsten Bräckelmann from comment #76)
> Both swallow their input on STDIN.

Without a parameter to GNU cat, yes, it consumes input on stdin. With an empty string as a parameter, it bails with EXIT_FAILURE as all programs are expected to.

> Validating input comes *after* receiving input. Cannot validate while still
> waiting.

The socket path is provided and available as input before the stdin stream needs to be accessed, therefore your argument is not valid. There is no excuse for not reasonably validating user input other than sheer laziness.

> > Both swallow their input on STDIN.
> 
> Without a parameter to GNU cat, yes, it consumes input on stdin. With an
> empty string as a parameter, it bails with EXIT_FAILURE as all programs are
> expected to.

An empty string as a parameter (argument) to an option requiring an argument, is an entirely valid case. Similar to this heavily perl-ish

  s/ anything Kip says / /x

> > Validating input comes *after* receiving input. Cannot validate while still
> > waiting.
> 
> The socket path is provided and available as input before the stdin stream
> needs to be accessed, therefore your argument is not valid.

Wrong. STDIN needs to be accessed, to decide whether to contact spamd at all. See comment 38 for the max-size option you outright dismissed before. However, this is entirely irrelevant.

 * Your recent arguing is about apparently hanging spamc processes, which
   can only happen with stalled STDIN.
 * Your patch does nothing in this regards.

So, is your bug report about idling on STDIN, while other options like a possibly bad -U socket is not checked?

  Your patch does NOT fix this.

Or is your bug report about spamc not treating a bad -U socket as an error?

  It does.


Maybe there is a language barrier after all, between en_CA and en_DE.

Kip, can you coherently and in a single statement (unlike the squirming in the previous 70+ comments) explain the issue again, please?

(In reply to Karsten Bräckelmann from comment #78)
> An empty string as a parameter (argument) to an option requiring an
> argument, is an entirely valid case. 

No, you're mistaken in the general sense. That depends entirely on what a program is intended to do with it. But regardless, even in the context of spamc, it's still wrong as I've already demonstrated.

> Wrong. STDIN needs to be accessed, to decide whether to contact spamd at
> all. See comment 38 for the max-size option you outright dismissed before.
> However, this is entirely irrelevant.

Wrong again. spamc calls read_args()'s before setting up transport.

>  * Your recent arguing is about apparently hanging spamc processes, which
>    can only happen with stalled STDIN.

No, we tried it even with input (see above), and it still hanged.

>  * Your patch does nothing in this regards.

You might start with reading it.

> Kip, can you coherently and in a single statement (unlike the squirming in
> the previous 70+ comments) explain the issue again, please?

It wouldn't matter what I said, besides the fact that I've already met this request, you're going to continue to troll and not listen anyways. The problem has been patched now. If you don't want to use it to improve your code base, that's fine. We can patch it downstream.

> No, you're mistaken in the general sense. That depends entirely on what a
> program is intended to do with it. But regardless, even in the context of
> spamc, it's still wrong as I've already demonstrated.

So you say, the calling program should never have passed an empty string as a value to an argument that requires a file?

Agreed. Evolution should have checked that.

> > Wrong. STDIN needs to be accessed, to decide whether to contact spamd at
> > all. See comment 38 for the max-size option you outright dismissed before.
> > However, this is entirely irrelevant.
> 
> Wrong again. spamc calls read_args()'s before setting up transport.
                           ^^^^
Read.

Not the same as verify arguments' contents. And do FS check to test the argument exists, is a socket, and the apparent listener seems to be a spamd instance. Neither for TCP. So what?

> >  * Your recent arguing is about apparently hanging spamc processes, which
> >    can only happen with stalled STDIN.
> 
> No, we tried it even with input (see above), and it still hanged.

False.

If you strongly believe I am wrong, please tell us at least the comment number rather than "above" only.


> > Kip, can you coherently and in a single statement (unlike the squirming in
> > the previous 70+ comments) explain the issue again, please?
> 
> It wouldn't matter what I said, besides the fact that I've already met this
> request,

You didn't.

> you're going to continue to troll

I won't.

> and not listen anyways. The problem has been patched now.

It has not.  (Unless you refer to your private system only.)

> If you don't want to use it to improve your
> code base, that's fine. We can patch it downstream.
                          ^^
By "we" you mean...

Please add a comment to this bug report, if/when "we"/you ever patch it downstream.

(In reply to Karsten Bräckelmann from comment #80)
> So you say, the calling program should never have passed an empty string as
> a value to an argument that requires a file?

No, that's not what I said. Like I said, there's apparently a language barrier.

> Not the same as verify arguments' contents. 

I don't think you understand how GNU getopt_long processing works. That's exactly one of the things it is suppose to do, verify arguments. Otherwise there would be no reason to set the required_argument bit set.

> And do FS check to test the
> argument exists, is a socket, and the apparent listener seems to be a spamd
> instance. 

Correct.

> Neither for TCP. So what?

Test if the TCP host / port are valid as well? That's a good idea. Go for it.

> > No, we tried it even with input (see above), and it still hanged.
> 
> False.
> 
> If you strongly believe I am wrong, please tell us at least the comment
> number rather than "above" only.

I already did and your solution was to call me a liar.

> > you're going to continue to troll
> 
> I won't.

You are.

> Like I said, there's apparently a language barrier.

I am sorry. I don't speak French. Poor me learned Latin in school. Doesn't help understanding en_CA apparently.

But since you seem to have an interest in getting $(whatever your issue is) fixed upstream, maybe you can lower yourself and explain it in a poor English style I might be able to grasp?

That would be so sweet of you.

> > Neither for TCP. So what?
> Test if the TCP host / port are valid as well? That's a good idea. Go for it.

spamc -K

Did you ever read at least the man-pages? (Beware, rhetorical question.)

> > If you strongly believe I am wrong, please tell us at least the comment
> > number rather than "above" only.
> 
> I already did and your solution was to call me a liar.

I never called you a liar. Explicitly. Maybe implicitly, granted, but then again my question stands -- which comment are you referring to?


> > > you're going to continue to troll
> > 
> > I won't.
> 
> You are.

I like to keep you around. :)

(In reply to Karsten Bräckelmann from comment #82)
> I am sorry. I don't speak French. Poor me learned Latin in school...But 
> since you seem to have an interest in getting $(whatever your issue is)
> fixed upstream, maybe you can lower yourself and explain it in a poor
> English style I might be able to grasp?

It's ok, the Latin will help break the language barrier: It's been provided already, ad nauseum.

> Did you ever read at least the man-pages? (Beware, rhetorical question.)

In that case, asking me whether spamc should check the validity of the TCP connection is a rhetorical question. My OP has nothing to do with TCP connections in any case.

> I never called you a liar. Explicitly. Maybe implicitly, granted, but then
> again my question stands -- which comment are you referring to?

If I gave you a comment number (again), you'd denounce it at worst, or simply keep pushing the goal posts at best. I'm not your bibtex.

> > Did you ever read at least the man-pages? (Beware, rhetorical question.)
> 
> In that case, asking me whether spamc should check the validity of the TCP
> connection is a rhetorical question. My OP has nothing to do with TCP
> connections in any case.

Nah, I asked if you ever read the spamc man-page. Did you?

In any case, checking for the validity of the process on the other side of the pipe, is and must be done after consuming STDIN.

Your original report comment 0 has indeed nothing to do with TCP. But that quote is not about comment 0, but your comment 81, where you actively encourage checking TCP.

If that is not part of your report, then please refrain from arguing about TCP here. This report is about tracking the bug you reported. If you want to extend your report to TCP, please file a new report.


> > I never called you a liar. Explicitly. Maybe implicitly, granted, but then
> > again my question stands -- which comment are you referring to?
> 
> If I gave you a comment number (again), you'd denounce it at worst, or
> simply keep pushing the goal posts at best. I'm not your bibtex.

I asked more than once. I have more than your pet-peeve bug of the week to worry about.

I am, however, one of the few actively contributing to this project. Which you are not. So, if you are asked for a simple positive integer number less than 80, to clarify your "I mentioned it before" whining, it should be really easy for you to add here.

I'm jumping in kind of late, after 84 comments, but since I was one of the people involved in the original coding of spamc and the decision to have spamc fail safe by returning a 0 exit code under what might normally be considered an error condition I have an interest in understanding if there is something that needs to be fixed here. But I'm afraid the above comments have left me very confused.

First of all, is it the case that the original bug that was reported here turned out to be that spamc was configured to talk to a tcp port but spamd was listeneing on a unix socket or vice versa, i.e., it was a configuration error maybe compounded by Evolution not checking for errors, a bug that has since been fixed in Evolution?

Or is there a problem that was demonstrated by the 400 hanging spamc processes, not to be confused with the output of ps aux | grep "[spamc]" | wc -l which on one of my machines returns 291 even though spamc is not installed on that machine? If hanging spamc processes is the bug being reported, under what circumstances does it hang? I can see some mention in comments that if you allow STDIN to sit without giving it any input then spamc will continue to wait for it even if the argument to -U is not a valid socket path. I don't see how Evolution could have been using spamc in a way that would trigger that kind of hang. Even if Evolution passed an empty string or other invalid socket path to the -U option, that would still terminate after reading in the input.

I'm confused about what use case the proposed patch fixes. The following use cases seem to work:

 echo foo | spamc -U ; echo $?

That outputs a command line usage message (useful if a human is using spamc on the command line) and returns exit code 64 (useful for a calling process or script to notice a problem)

 echo foo | spamc -U "" ; echo $?

That outputs the passed through input of "foo" and returns an exit code 0. That is the design that allows spamc to fail safe, not losing mail if there is a transport problem talking to spamd. It is the same behavior you get if the network connection to spamd running on an external machine fails, the mail still gets delivered, though not spam filtered.

 echo foo | spamc -U "" -x ; echo $?

That has empty STDOUT output (does not fail safe) but does indicate the error with an exit code 69. The -x option is for people who would rather have spamc's caller check for the exit code and handle an error such as -U "" instead of having the failsafe behavior.

 spamc -U "" -K ; echo $?

Indicates with exit code 69 that transport to the unix socket failed, without ever trying to read from STDIN. This what a caller can use to check that transport works to talk to a spamd that is listening. It is a more complete check than a simple !strlen(spamc_optarg) || (stat(spamc_optarg, &attributes) != 0) || !S_ISSOCK(attributes.st_mode) like the proposed patch does.

The proposed patch adds an early check that the argument to the -U option is a Unix socket. If it isn't, it causes the call to fail and the caller possibly to lose the mail. (We allow for a use case in which the caller really does count on the documented fail safe behavior to deliver to stdout whatever is piped into spamc with nothing worse happening than no spam filtering). It's one thing to fail completely if the command line syntax is totally wrong, such as no argument to -U. That is an error in setting up the mail system which will be caught before going into production. But an argument to -U may work fine one day and then no longer be a valid socket path the next. The proposed patch could cause a working system to start rejecting mail (or losing it if the exit code is not properly checked) if something goes wrong and the socket path stops being valid. The current code will failsafe. If you really want the behavior that the patch provides, use the -x option, which not only will produce the same result of returning a non-zero exit code given a socket path that doesn't stat, it will do it even with a valid socket path that is not to a listening spamd, something the patch doesn't do. It does read all of stdin before it reports the error. I don't think that is a problem compared to the benefits of 1) having the default behavior be fail safe; 2) allowing to check for input too long and pass through the too long input straight to stdout without loading it on spamd. Especially since the patch gets its benefit of early detection of the problem with -U only when the socket path is so wrong that it fails the stat, and that benefit could only be safely used when the -x option is specified.

Well, maybe I did miss the description of the actual bug in the previous 84 comments, so if I misunderstood something I would like to know what needs fixing, or how the proposed patch does something more useful that the existing code does not do.

(In reply to Kevin A. McGrail from comment #13)
> Look at adding the patch to spamc.  It isn't a bad patch despite the
> volatility of the discussion surrounding it.

+1

Can we *please* put and end to this?

(In reply to John Hardin from comment #86)
> (In reply to Kevin A. McGrail from comment #13)
> > Look at adding the patch to spamc.  It isn't a bad patch despite the
> > volatility of the discussion surrounding it.
> 
> +1
> 
> Can we *please* put and end to this?

After reading Sidney's comments, I'd like to revise this: accept the part of the patch that provides meaningful error messages, but don't fail unsafe.

(In reply to Sidney Markowitz from comment #85)
> I'm jumping in kind of late, after 84 comments, but since I was one of the
> people involved in the original coding of spamc and the decision to have
> spamc fail safe by returning a 0 exit code under what might normally be
> considered an error condition I have an interest in understanding if there
> is something that needs to be fixed here. But I'm afraid the above comments
> have left me very confused.

Hey Sidney. I'm sorry about that and I had hoped the discussion thus far had been more constructive and I don't fault you for having skipped the bulk of it.

> First of all, is it the case that the original bug that was reported here
> turned out to be that spamc was configured to talk to a tcp port but spamd
> was listeneing on a unix socket or vice versa, i.e., 

No, sorry if that was not clear. The problem was in how it handled a bad unix socket parameter under certain conditions. I don't know enough about the TCP method to say if there's an issue with that transport method, but someone else more knowledgeable might be interested in looking into that. Although that's not an issue I'm having.

> Or is there a problem that was demonstrated by the 400 hanging spamc
> processes, not to be confused with the output of ps aux | grep "[spamc]" |
> wc -l which on one of my machines returns 291 even though spamc is not
> installed on that machine? 

The regexp was wrong, but I did not apply it until after running htop and seeing the number of spamc processes scrolling well off the page. But anyways, it may well have been related, but I don't know yet for sure until I can verify at least how each process was being invoked.

> If hanging spamc processes is the bug being
> reported, under what circumstances does it hang? 

My issue originally was not the hanging, although that came up later. I knew my SA was not working for some reason, but I did not know why. All I would see in mail.err were many of the following:

     spamc[7611]: connect(AF_UNIX) to spamd  failed: Connection refused
     
I checked to make sure spamd was running and it looked fine. I reinstalled it, restarted the service, rebooted, etc., and even verified permissions and it all seemed fine.

The double space after spamd eventually caught my eye and I went to investigate. It turned out the path to the UNIX socket belonging to the spamd daemon was an empty string. I did some more digging and, for whatever reason, Evolution was invoking spamc with an empty -U "" parameter. That's definitely Evolution's fault as everyone agrees thus far, but spamc's handling of this situation could have been more graceful.

> I can see some mention in
> comments that if you allow STDIN to sit without giving it any input then
> spamc will continue to wait for it even if the argument to -U is not a valid
> socket path. I don't see how Evolution could have been using spamc in a way
> that would trigger that kind of hang. 

I actually don't know definitively if or how Evolution was causing them to hang, so I'm in agreement with you. Thankfully John's given me a useful tip to track down how they were invoked so we can figure that out. I just need to wait for it to happen again.

> Even if Evolution passed an empty
> string or other invalid socket path to the -U option, that would still
> terminate after reading in the input.

If -x was provided, then yes, it should have. But for whatever reason, it did not output the following on my mail.err that I see when I manually invoked it myself on the command line.

    spamc[8213]: invalid usage

That's fine as error messages go. However, in the case where Evolution was invoking it, that was not present. Based on the debugging I did, not all code paths that I examined relied on a transport socketpath variable checked to ensure that it was non-empty. They did, however, check as far as I could see, that the string was non-null. Given that those assertions were being made, coupled with the malformed string contained in "spamd  failed", it was an indication to me that there was a problem and this was not intentional.

>  echo foo | spamc -U "" -x ; echo $?
> 
> That has empty STDOUT output (does not fail safe) but does indicate the
> error with an exit code 69. The -x option is for people who would rather
> have spamc's caller check for the exit code and handle an error such as -U
> "" instead of having the failsafe behavior.

This is probably the closest analogue to what Evolution was trying to do.

I think there are two problems with this. As you know, most usage of SA is in a daemonized environment where the user is not going to be able to see the console output. It's true, the calling application could and should handle the return code, but even in the case where it is, and Evolution appears to be doing that, based on my reading of the source (though I'm not certain), it was not immediately evident based on the mail.err log what the problem actually was.

The second problem is it seems as though the -x makes spamc behave more as a user might expect it to behave on a POSIX system. Granted, because of what it is trying to do and what it must not lose, there can be a good exception to this here. However, given that at least three or four people above may not have immediately realized this, even though the behaviour is documented in the man page, the message on mail.err could have been more helpful since there may be many others with the same expectation. Karsten and I agree to disagree on how it handles this, but that is not really the issue that I was trying to dwell on anyways.

> It's one thing to fail completely if the command line syntax is
> totally wrong, such as no argument to -U. That is an error in setting up the
> mail system which will be caught before going into production. But an
> argument to -U may work fine one day and then no longer be a valid socket
> path the next. 

Agreed. And while the patch could be improved to address that, we can both agree that an empty socket path will always be invalid on any day.

> The proposed patch could cause a working system to start
> rejecting mail (or losing it if the exit code is not properly checked) if
> something goes wrong and the socket path stops being valid. 

Yes, this is possible, and as I mentioned the patch can be improved to address that. However, we can reduce the likelihood of it being used incorrectly, even when supplemented with proper checking of exit codes, if developers and users can identify wrong usage earlier through a more meaningful error message.

I would venture to guess that probably all higher level applications that invoke spamc probably flank the -U parameter in quotations for safety reasons, and it is possible that they too might under certain conditions provide nothing within them. They should not do that, we agree, but that their code is not within the purview of SA's.

> The current code
> will failsafe. If you really want the behavior that the patch provides, use
> the -x option, which not only will produce the same result of returning a
> non-zero exit code given a socket path that doesn't stat, it will do it even
> with a valid socket path that is not to a listening spamd, something the
> patch doesn't do. 

I understand what you are saying, but bear in mind that most everyday usage of spamc is not through direct command line usage by the user, but through some other higher level application, be it a mail client, a cron job, or what have you. The standard approach that supplements what you are already trying to do is to make an adequate note in /var/log/ (e.g. mail.err) (FHS 5.10.1).

> Well, maybe I did miss the description of the actual bug in the previous 84
> comments, so if I misunderstood something I would like to know what needs
> fixing, or how the proposed patch does something more useful that the
> existing code does not do.

I hope that was helpful.

(In reply to John Hardin from comment #87)
> (In reply to John Hardin from comment #86)
> > (In reply to Kevin A. McGrail from comment #13)
> > > Look at adding the patch to spamc.  It isn't a bad patch despite the
> > > volatility of the discussion surrounding it.
> > 
> > +1
> > 
> > Can we *please* put and end to this?
> 
> After reading Sidney's comments, I'd like to revise this: accept the part of
> the patch that provides meaningful error messages, but don't fail unsafe.

Thanks John. I'm fine with that. It would have been more useful to the user with a more meaningful error message, and consequently might have helped an Evolution developer, or any other mail client, identify an issue with spamc's invocation before this became an issue in the first place.

Created attachment 5146 [details]
patch to make error log message more clear

After all that, the only actual problem that I see is the confusing error log. Here is a test command that demonstrates the actual problem by using the -l option to make the error message visible for this test:

$ echo foo | spamc -l -U "" ; echo $?
spamc: connect(AF_UNIX) to spamd  failed: Connection refused
foo
0

I've just attached a one line patch to libspamc.c that changes the result to

$ echo foo | spamc -l -U "" ; echo $?
spamc: connect(AF_UNIX) to spamd using --socket='' failed: Connection refused
foo
0

Because of the extraordinary amount of heat generated by this issue, and because I'm kind of tired at the moment, I will give this a day or two before I commit this patch even though we are in CTR mode now and I don't have to wait to commit.

(In reply to Sidney Markowitz from comment #89)
> Because of the extraordinary amount of heat generated by this issue, and
> because I'm kind of tired at the moment, I will give this a day or two
> before I commit this patch even though we are in CTR mode now and I don't
> have to wait to commit.

Hey Sidney. I'm fine with the patch. Not every problem requires a complex patch. Thanks for keeping an open mind.

Committed revision 1488696.

Thanks Sidney.