Milter-greylist

Hi,

Attached is a patch which supercedes my previous patches for the Received-SPF header and nodkim.

On the nodkim feature: I dropped it as I generally don't find it useful. I was really after suppressing the DKIM log messages which the attached patch still achieves. 

On the Received-SPF aspect:  I've extended the string formats to also include SPF and DKIM results, and Authentication-Results.

Currently running this patch (with my postfix queue ID patch) with libopendkim and libspf2; all seems to be working well so far.
--
Steven Hiscocks

Sorry. Previous patch had significant bug due to me misinterpreting the case statement in spf.c, setting the result string to test value as set in configuration.  New patch attached. 

On 20 September 2015 20:47:37 BST, "Steven Hiscocks steven@... [milter-greylist]" <milter-greylist@yahoogroups.com> wrote:
>Hi,
>
>Attached is a patch which supercedes my previous patches for the
>Received-SPF header and nodkim.
>
>On the nodkim feature: I dropped it as I generally don't find it
>useful. I was really after suppressing the DKIM log messages which the
>attached patch still achieves. 
>
>On the Received-SPF aspect:  I've extended the string formats to also
>include SPF and DKIM results, and Authentication-Results.
>
>Currently running this patch (with my postfix queue ID patch) with
>libopendkim and libspf2; all seems to be working well so far.
>--
>Steven Hiscocks

--
Steven Hiscocks

Steven Hiscocks steven@... [milter-greylist]
<milter-greylist@yahoogroups.com> wrote:

> Sorry. Previous patch had significant bug due to me misinterpreting the
> case statement in spf.c, setting the result string to test value as set
> in configuration.  New patch attached.

Sorry for not replying earlier, I had trouble with a dying computer at
home. A few comments:

- you introduced priv->priv_sr.sr_addheader_ins  and ap.ap_addheader_ins
the _ins suffix is not very self-explaining. What about _index ?

- Why EXF_DKIM changed value?

- in conf_yacc.y, milter-greylist.c, spf.c, please stick to 80
char/lines maximum. (tab is 8 spaces)

Otherwise, this seems nice.
-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 27 September 2015 17:37:07 BST, "manu@... [milter-greylist]"  wrote:

Steven Hiscocks steven@... [milter-greylist]
 wrote:

 Sorry. Previous patch had significant bug due to me misinterpreting
the
 case statement in spf.c, setting the result string to test value as
set
 in configuration.  New patch attached.

Sorry
for not replying earlier, I had trouble with a dying computer at
home. A few comments:
No worries.


- you introduced priv->priv_sr.sr_addheader_ins  and
ap.ap_addheader_ins
the _ins suffix is not very self-explaining. What about _index ?
Changed to index in attached patch.


- Why EXF_DKIM changed value?
If I remember  correctly, I added the DKIM reason message, but it kept triggering when 'nolog' was set. Maybe I misinterpreted it, but it just seemed strange for two constants to have the value.


- in conf_yacc.y,
milter-greylist.c, spf.c, please stick to 80
char/lines maximum. (tab is 8 spaces)
Hopefully sorted now in attached patch.


Otherwise, this seems nice.
Thanks.

Also changed SPF "hardfail" to "fail" to comply with SPF header standards.

--
Steven Hiscocks

Steven Hiscocks steven@... [milter-greylist]
<milter-greylist@yahoogroups.com> wrote:

> >Otherwise, this seems nice.
> Thanks.

I was about to commit it, but I realized you patched against 4.4.3, and
the patch does not apply on latest code. There is a conflict obvious to
resolve in conf_yacc.y, but also many patch offsets that may require a
careful look and a bit of testing. 

Could you update the contribution for 4.5.14?

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

On 04/10/15 05:07, manu@... [milter-greylist] wrote:
>  
> 
> I was about to commit it, but I realized you patched against 4.4.3, and
> the patch does not apply on latest code. There is a conflict obvious to
> resolve in conf_yacc.y, but also many patch offsets that may require a
> careful look and a bit of testing.
> 
> Could you update the contribution for 4.5.14?
> 
I believe that patch should be against 4.5.14. I've just tried the patch
against 4.5.14 source and the latest pull from CVS, and it appears to
patch cleanly (no offsets or conflicts).

Is the CVS repository (one list on the website) the latest?

-- 
Steven Hiscocks

Steven Hiscocks steven@... [milter-greylist]
<milter-greylist@yahoogroups.com> wrote:

> I believe that patch should be against 4.5.14. I've just tried the patch
> against 4.5.14 source and the latest pull from CVS, and it appears to
> patch cleanly (no offsets or conflicts).

I double checked it and it seems I am the one that screwed it, sorry: it
applies nicely.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...