Comment 4 for bug 1770532

Revision history for this message
Adam Jacobs (bllfr0g) wrote :

Correct, I am running patch (a), the same as included in Fedora, and it fixes the bug for me.

Before implementing the patch I spent a little time trying to understand what it does. Best I can tell, it is appropriate to call $msginfo->originating(c('originating')) after ANY call to load_policy_bank(). Indeed, that was the case in release 2.10.

Patch (b) (which subsumes patch (a)) looks good to me too, though I haven't tested it. Basically, there are a number of different ways in amavisd to apply a policy bank: the different places load_policy_bank() gets called correspond to different ways a policy bank can be triggered, and it just so happens that the way I'm applying the policy bank in my use case corresponds to the call from patch (a.)

All that said, I think both (a) and (b) are both poor fixes. It looks like upstream was TRYING, in 2.11, to remove the requirement to call $msginfo->originating(c('originating')) after each invocation of load_policy_bank() by moving a call to $msginfo->originating(c('originating')) to the end of the load_policy_bank() sub itself. Diffing 2.10 and 2.11, $msginfo->originating(c('originating')) is removed from after each call to load_policy_bank, and added to the very end of load_policy_bank itself. That said, the new call at the end of load_policy_bank() does not seem to be effective (this is the actual bug.) I tried to figure out why it isn't working; it seems to me that it should, since msginfo is an object reference (I think), so I'd think that calling a method on the object ought to be a persistent change that survives the return of the sub, but apparently not. My perl isn't good enough to know why this isn't working, and an hour or two of googling about perl objects didn't reveal the answer.

Someone who actually knows perl could probably propose the "right" fix in minutes, and I'd be happy to test it in that case.