You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Norman Maurer <nm...@byteaction.de> on 2006/07/08 16:52:30 UTC

RE: svn commit: r420100 - /james/server/trunk/src/java/org/apache/james/smtpserver/SpamAssassinHandler.java

Am Samstag, den 08.07.2006, 10:37 -0400 schrieb Noel J. Bergman:
> Several comments inline:
> 
> > + // Not scan the message if relaying allowed
> 
> Should that be configurable?
Yes it should like in other filters i wrote ;-) Just forgot this ..

> 
> > + Iterator headers = sa.getHeaders().keySet().iterator();
> > + // Add the headers
> > + while (headers.hasNext()) {
> > +   String key = headers.next().toString();
> > +   message.setHeader(key, (String) sa.getHeaders().get(key));
> > + }
> 
> Do we want to add the header's now, or set attributes that we can map to headers later?  Personally, I'm leaning towards the latter.  We might even have a generic mailet along the lines:

>   <mailet match="All" class="Attributes2Headers">
>     <attribute name="org.apache.james.SpamAssassin-Status>
>       <header>X-Spam-Status</header>
>       <mapping> <!-- optional (see AbstractVirtualUser.regexMap for idea) -->
>         <regex>...</regex> <!-- regex to match against attribute value -->
>         <value>...</value> <!-- parameterized string to build header   -->
>       </mapping>
>     </attribute>
>     <attribute name="...">
>       ...
>     </attribute>
>   </mailet
> 
> This would give us more control over header names and content (if there were no mapping element, the value would be copied directly).  For example, the ASF version of the header looks like:
> 
>   X-ASF-Spam-Status: No, hits=-9.4 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME
> 
> And it would also be more efficient, since we could modify the message just once, rather than each time we want to add meta-data.
> 
Good idea.. Feel free to change it.. Or i will have a look at this
later.

> > + String responseString = "554 "
> > +        + DSNStatus.getStatus(DSNStatus.PERMANENT,
> > +                              DSNStatus.SECURITY_OTHER)
> > +        + " This message smells like SPAM. Message rejected";
> 
> Please use more useful and less "cutsie" replies.  This reply is not useful to a sender who might want to resolve the problem.  And once again, I would turn to a generalized version of the regexMap code to allow the administrator to configure precisely the content of the string portion of the response.  This approach to message strings, possibly combined with XMLResources, could become pervasive in JAMES v3 and later.
> 
Yes now i remember that we talked about that on apacheCon (too much
input). We can do this .. I used this replay cause it was in my mind
(too long used qmail + qmail-scanner)


> 	--- Noel

BTW, i commit it for show all what we can do and how. I have no problems
with refactor it ;-)

Bye
Norman