https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6939
Sidney Markowitz <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #85 from Sidney Markowitz <[email protected]> --- 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. -- You are receiving this mail because: You are the assignee for the bug.
