--
On Tue, Dec 23, 2003 at 11:50:15PM +0100, Lionel Elie Mamane wrote:
> I have this setup where the I do sender callout, but the options to
> the callout depend on the _recipient_ domain (some users want
> postmaster callout, other don't).
> This interacts badly with the callout cache leading to wrong results
> (even with no_cache, due to the process-live memory cache), and that
> the only the options to the *first* callout are honoured.
> I looked a bit at the code, from reading the function do_callout
> (file verify.c), it seems like it tries to do the right thing
I looked more closely at the code. The cache _lookup_ code does the
right thing, but the cache _update_ code does not: If
- postmaster callout is activated
- the sender callout succeeds and says OK
- the postmaster callout succeeds and says "not OK"
then the cache for the sender address gets a "not OK" entry, instead
of an OK entry.
I wrote a patch that works for me. Please review it, and consider
inclusion.
> Verifying the sender address only one time is essentially a (wrong)
> optimisation, not a feature, so it could be disabled altogether as a
> quick fix.
My patch does that (in acl.c). It also changes the cache update logic
(in verify.c) to cache the answer the MX got us, not:
(sender_callout_result && (do_pm_callout ? pm_callout_result : TRUE ))
(if you think this is more readable:
(sender_callout_result && (!do_pm_callout || pm_callout_result))
)
One comment:
if (done)
{
- if (!callout_no_cache && new_domain_record.result != ccache_reject)
+ if (!callout_no_cache && new_record.result != ccache_unknown)
{
if (dbm_file == NULL)
dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE);
This is correct because if new_record.result != ccache_unknown, then
new_domain_record.result != ccache_reject . Unless I forgot some nasty
effect of the caching or some such.
Merry Christmas, Happy Hanukkah, and all that.
--
Lionel
--
--- exim4-4.30/src/acl.c Mon Dec 1 11:15:41 2003
+++ exim4-4.30.lio/src/acl.c Wed Dec 24 08:22:53 2003
@@ -806,34 +806,13 @@
{
sender_vaddr = verify_checked_sender(verify_sender_address);
- if (sender_vaddr != NULL && /* Previously checked */
- (callout <= 0 || /* No callout needed; OR */
- testflag(sender_vaddr, af_verify_callout))) /* Callout was specified */
- {
- /* If no callout is required on this check, but callout was specified
- on the check that is cached, we inspect the "routed" flag. If this is set,
- it means that routing worked, so this check can give OK (the saved return
- code value belongs to the callout). Othewise, use the saved return code. */
-
- if (callout <= 0 && testflag(sender_vaddr, af_verify_callout))
- rc = OK;
- else
- {
- rc = sender_vaddr->special_action;
- *basic_errno = sender_vaddr->basic_errno;
- }
- HDEBUG(D_acl) debug_printf("using cached sender verify result\n");
- }
-
- /* Do a new verification, and cache the result. The cache is used to avoid
- verifying the sender multiple times for multiple RCPTs. It is also used on
+ /* Do a new verification, and cache the result. The cache is used on
failure to give details in response to the first RCPT that gets bounced for
this reason. However, this can be suppressed by the no_details option, which
sets the flag that says "this detail has already been sent". The cache
normally contains just one address, but there may be more in esoteric
circumstances. */
- else
{
BOOL routed = TRUE;
sender_vaddr = deliver_make_addr(verify_sender_address, TRUE);
diff -N --recursive -u exim4-4.30/src/verify.c exim4-4.30.lio/src/verify.c
--- exim4-4.30/src/verify.c Mon Dec 1 11:15:41 2003
+++ exim4-4.30.lio/src/verify.c Wed Dec 24 11:44:25 2003
@@ -155,11 +155,13 @@
open_db dbblock;
open_db *dbm_file = NULL;
dbdata_callout_cache new_domain_record;
+dbdata_callout_cache_address new_record;
host_item *host;
new_domain_record.result = ccache_unknown;
new_domain_record.postmaster_result = ccache_unknown;
new_domain_record.random_result = ccache_unknown;
+memset(&new_record, 0, sizeof(new_record));
/* Open the callout cache database, it it exists, for reading only at this
stage, unless caching has been disabled. */
@@ -486,11 +488,16 @@
if (new_domain_record.random_result != ccache_accept && done)
{
done =
- smtp_write_command(&outblock, FALSE, "RCPT TO:<%.1000s>\r\n",
- addr->address) >= 0 &&
+ smtp_write_command(&outblock, FALSE, "RCPT TO:<%.1000s>\r\n",
+ addr->address) >= 0 &&
smtp_read_response(&inblock, responsebuffer, sizeof(responsebuffer),
'2', callout);
+ if (done)
+ new_record.result = ccache_accept;
+ else if (errno == 0 && responsebuffer[0] == '5')
+ new_record.result = ccache_reject;
+
/* Do postmaster check if requested */
if (done && callout_postmaster)
@@ -599,11 +606,11 @@
}
/* If a definite result was obtained for the callout, cache it unless caching
-is disabled, or we had a negative domain record. */
+is disabled. */
if (done)
{
- if (!callout_no_cache && new_domain_record.result != ccache_reject)
+ if (!callout_no_cache && new_record.result != ccache_unknown)
{
if (dbm_file == NULL)
dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE);
@@ -613,9 +620,6 @@
}
else
{
- dbdata_callout_cache_address new_record;
- memset(&new_record, 0, sizeof(new_record));
- new_record.result = (yield == FAIL)? ccache_reject : ccache_accept;
(void)dbfn_write(dbm_file, addr->address, &new_record,
(int)sizeof(dbdata_callout_cache_address));
HDEBUG(D_verify) debug_printf("wrote %s callout cache address record\n",
--
