Felix, Thanks for both you messages. I need to digest the more hefty changes, so here comes the easy part of my reply.
> while working on a spam scanner plugin interface for amavisd-new, I > noticed some minor things which IMHO should be improved. All patches > are available at http://www.felix-schwarz.name/files/opensource/ > because I don't know your attachement policy. > > 01_mbxname.patch: When calculating the path for quarantine, the > concatenation is done manually and does not use File::Spec. This may > lead to strings like /path//filename. While this is no problem on > Unix, I like to see a more intelligent concatenation which does not > add a slash if there is already one on the end of the first variable. > My patch uses File::Spec for that. Thanks, accepted (unless I change my mind in the following month). There are many more cases like that which do a manual splicing of file name components and could benefit from File::Spec routines. Since it is highly unlikely the program could run on non-Unix platforms, my decision to avoid File::Spec module was based on keeping the program virtual memory footprint free of modules from which only a small fraction of their functionality is needed. There are also issues with loading dependent (sub)modules, which shows up when running chrooted. This is why I have to keep a list of modules in calls to fetch_modules() up to date, it all too often happens that some component does fails to get loaded until it is too late. Bth, if users adhere to the syntax prescribed in amavisd.conf(-sample), strings like /path//filename can not form. > 02_description1.patch: The source code documentation is not complete > so I added one missing case. > > 03_description2.patch: The documentation for "spam_scan" is wrong > because this method does not return a single value but a list. Accepted. > There is a piece of code in spam_scan which should IMHO be improved > although I don't know which solution you prefer: > > my($mbsl) = c('sa_mail_body_size_limit'); > > if ( defined $mbsl && > > ($msginfo->orig_body_size > $mbsl || > > $msginfo->orig_header_size + 1 + $msginfo->orig_body_size > > > 5*1024 + $mbsl) > > ) { > > do_log(1,"spam_scan: not wasting time on SA, message ". > > "longer than $mbsl bytes: ". > > $msginfo->orig_header_size .'+'. $msginfo->orig_body_size); > > } else { > > The configuration value is is called "sa_mail_body_size_limit" > therefore I would expect that the condition only uses > $msginfo->orig_body_size (first part of the condition). In fact there > is a second condition that means that the original mail should not be > bigger than sa_mail_body_size_limit + 5k. > > Either the configuration value should be renamed to sa_mail_size_limit > or the condition should be changed that it does not consider the > header size. Well, the story goes like this: the semantics of the name sa_mail_body_size_limit is correct as intended. Its primay function is to compare it with $msginfo->orig_body_size. Then Ralf came up with an example of a mail which had a huuuge header and a normal (small) body. This caused SA to process this mail for half an hour (if allowed to). At that point I added the second condition as a stop-gap solution. One can think of it as: if header is very big, count the header lines beyond first 5k as mail body for the purpose of deciding whether SA should be called or not. Mark ------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click _______________________________________________ AMaViS-user mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/amavis-user AMaViS-FAQ:http://www.amavis.org/amavis-faq.php3 AMaViS-HowTos:http://www.amavis.org/howto/
