On Wed, 7 Feb 2007, David Woodhouse wrote:
> defer condition = ${if eq {$acl_m_greyexpiry}{} {1}}
> set acl_m_dontcare = ${lookup sqlite {GREYDB INSERT INTO greylist \
> VALUES ( '$acl_m_greyident', \
> '${eval10:$tod_epoch+300}', \
> '$sender_host_address', \
> '${quote_sqlite:$sender_helo_name}' );}}
> condition = ${if eq {$search_error_message}{} {1}}
Looking at the code, it would seem that that will work only for sqlite,
and not for any other SQL lookup. The reason is that the code for sqlite
(which originated with you, I know) is, er, "lite", and returns only
success (OK - some string is returned) or not success (FAIL - an empty
string is returned). The code for other lookup types is likely (if I've
read the code right) to return DEFER if there is a problem running the
query (e.g. insert failed). In that situation, the "condition" condition
would never be reached, because the ACL would immediately terminate with
DEFER.
In any case, the search_error_message variable that you point at with
your patch is very ephemeral ...
> ... and although it does seem to do the right thing and accept the
> message instead of greylisting it (because if the db insertion isn't
> working, it'd just keep deferring it for ever), it still seems that
> $search_error_message doesn't manage to get into the log intact.
... indeed, it is reset before each ACL verb's conditions are evaluated
(in order to forget anything that went before). You could, of course,
save it in a variable.
Given the complication here, simply pointing $search_error_message at
the search_error_message variable is not, IMO, a good idea. Some proper
thought, redesign, and a thorough going-over of the relevant code is
what is needed.
Philip
--
Philip Hazel, University of Cambridge Computing Service.
