Another go at the overlong-addrs versus rewrites problem

3 files changed, 36 insertions, 42 deletions

diff --git a/src/src/match.c b/src/src/match.c
index 6415b993d..069d0f672 100644
--- a/src/src/match.c
+++ b/src/src/match.c
@@ -454,7 +454,7 @@ an unnamed list. */
 
 if (!*listptr)
   {
-  HDEBUG(D_lists) if (ot) debug_printf("%s no (option unset)\n", ot);
+  HDEBUG(D_lists) if (ot) debug_printf_indent("%s no (option unset)\n", ot);
   return FAIL;
   }
 
@@ -487,7 +487,7 @@ else
     {
     if (f.expand_string_forcedfail)
       {
-      HDEBUG(D_lists) debug_printf("expansion of \"%s\" forced failure: "
+      HDEBUG(D_lists) debug_printf_indent("expansion of \"%s\" forced failure: "
         "assume not in this list\n", *listptr);
       return FAIL;
       }
@@ -679,7 +679,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
             p->next = nb->cache_data;
             nb->cache_data = p;
             if (*valueptr)
-              DEBUG(D_lists) debug_printf("data from lookup saved for "
+              DEBUG(D_lists) debug_printf_indent("data from lookup saved for "
                 "cache for %s: key '%s' value '%s'\n", ss, p->key, *valueptr);
             }
           }
@@ -691,7 +691,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
 
       else
         {
-        DEBUG(D_lists) debug_printf("cached %s match for %s\n",
+        DEBUG(D_lists) debug_printf_indent("cached %s match for %s\n",
           (bits & (-bits)) == bits ? "yes" : "no", ss);
 
         cached = US" - cached";
@@ -705,7 +705,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
               *valueptr = p->data;
               break;
               }
-          DEBUG(D_lists) debug_printf("cached lookup data = %s\n", *valueptr);
+          DEBUG(D_lists) debug_printf_indent("cached lookup data = %s\n", *valueptr);
           }
         }
 
@@ -714,7 +714,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
 
       if ((bits & (-bits)) == bits)    /* Only one of the two bits is set */
         {
-        HDEBUG(D_lists) debug_printf("%s %s (matched \"%s\"%s)\n", ot,
+        HDEBUG(D_lists) debug_printf_indent("%s %s (matched \"%s\"%s)\n", ot,
           yield == OK ? "yes" : "no", sss, cached);
         return yield;
         }
@@ -728,7 +728,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
       switch ((func)(arg, ss, valueptr, &error))
         {
         case OK:
-	  HDEBUG(D_lists) debug_printf("%s %s (matched \"%s\")\n", ot,
+	  HDEBUG(D_lists) debug_printf_indent("%s %s (matched \"%s\")\n", ot,
 	    (yield == OK)? "yes" : "no", sss);
 	  return yield;
 
@@ -737,7 +737,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
 	    error = string_sprintf("DNS lookup of \"%s\" deferred", ss);
 	  if (ignore_defer)
 	    {
-	    HDEBUG(D_lists) debug_printf("%s: item ignored by +ignore_defer\n",
+	    HDEBUG(D_lists) debug_printf_indent("%s: item ignored by +ignore_defer\n",
 	      error);
 	    break;
 	    }
@@ -757,12 +757,12 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
         case ERROR:
 	  if (ignore_unknown)
 	    {
-	    HDEBUG(D_lists) debug_printf("%s: item ignored by +ignore_unknown\n",
+	    HDEBUG(D_lists) debug_printf_indent("%s: item ignored by +ignore_unknown\n",
 	      error);
 	    }
 	  else
 	    {
-	    HDEBUG(D_lists) debug_printf("%s %s (%s)\n", ot,
+	    HDEBUG(D_lists) debug_printf_indent("%s %s (%s)\n", ot,
 	      include_unknown? "yes":"no", error);
 	    if (!include_unknown)
 	      {
@@ -841,7 +841,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
         {
         case OK:
 	  (void)fclose(f);
-	  HDEBUG(D_lists) debug_printf("%s %s (matched \"%s\" in %s)\n", ot,
+	  HDEBUG(D_lists) debug_printf_indent("%s %s (matched \"%s\" in %s)\n", ot,
 	    yield == OK ? "yes" : "no", sss, filename);
 
 	  /* The "pattern" being matched came from the file; we use a stack-local.
@@ -855,7 +855,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
 	    error = string_sprintf("DNS lookup of %s deferred", ss);
 	  if (ignore_defer)
 	    {
-	    HDEBUG(D_lists) debug_printf("%s: item ignored by +ignore_defer\n",
+	    HDEBUG(D_lists) debug_printf_indent("%s: item ignored by +ignore_defer\n",
 	      error);
 	    break;
 	    }
@@ -870,12 +870,12 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
         case ERROR:		/* host name lookup failed - this can only */
 	  if (ignore_unknown)	/* be for an incoming host (not outgoing) */
 	    {
-	    HDEBUG(D_lists) debug_printf("%s: item ignored by +ignore_unknown\n",
+	    HDEBUG(D_lists) debug_printf_indent("%s: item ignored by +ignore_unknown\n",
 	      error);
 	    }
 	  else
 	   {
-	    HDEBUG(D_lists) debug_printf("%s %s (%s)\n", ot,
+	    HDEBUG(D_lists) debug_printf_indent("%s %s (%s)\n", ot,
 	      include_unknown? "yes":"no", error);
 	    (void)fclose(f);
 	    if (!include_unknown)
@@ -901,7 +901,7 @@ while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
 /* End of list reached: if the last item was negated yield OK, else FAIL. */
 
 HDEBUG(D_lists)
-  debug_printf("%s %s (end of list)\n", ot, yield == OK ? "no":"yes");
+  debug_printf_indent("%s %s (end of list)\n", ot, yield == OK ? "no":"yes");
 return yield == OK ? FAIL : OK;
 
 /* Something deferred */
@@ -1017,7 +1017,7 @@ uschar *subject = cb->address;
 const uschar *s;
 uschar *pdomain, *sdomain;
 
-DEBUG(D_lists) debug_printf("address match test: subject=%s pattern=%s\n",
+DEBUG(D_lists) debug_printf_indent("address match test: subject=%s pattern=%s\n",
   subject, pattern);
 
 /* Find the subject's domain */
diff --git a/src/src/receive.c b/src/src/receive.c
index f4b829659..058a74cb2 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -2835,10 +2835,13 @@ if (LOGGING(received_recipients))
 recipients will get here only if the conditions were right (allow_unqualified_
 recipient is TRUE). */
 
+DEBUG(D_rewrite)
+  { debug_printf_indent("qualify & rewrite recipients list\n"); acl_level++; }
 for (int i = 0; i < recipients_count; i++)
   recipients_list[i].address =	/* deconst ok as src was not cont */
     US rewrite_address(recipients_list[i].address, TRUE, TRUE,
       global_rewrite_rules, rewrite_existflags);
+DEBUG(D_rewrite) acl_level--;
 
 /* If there is no From: header, generate one for local (without
 suppress_local_fixups) or submission_mode messages. If there is no sender
@@ -3010,6 +3013,8 @@ if (  from_header
 /* If there are any rewriting rules, apply them to the sender address, unless
 it has already been rewritten as part of verification for SMTP input. */
 
+DEBUG(D_rewrite)
+  { debug_printf("global rewrite rules\n"); acl_level++; }
 if (global_rewrite_rules && !sender_address_unrewritten && *sender_address)
   {
   /* deconst ok as src was not const */
@@ -3018,6 +3023,7 @@ if (global_rewrite_rules && !sender_address_unrewritten && *sender_address)
   DEBUG(D_receive|D_rewrite)
     debug_printf("rewritten sender = %s\n", sender_address);
   }
+DEBUG(D_rewrite) acl_level--;
 
 
 /* The headers must be run through rewrite_header(), because it ensures that
@@ -3034,12 +3040,13 @@ We start at the second header, skipping our own Received:. This rewriting is
 documented as happening *after* recipient addresses are taken from the headers
 by the -t command line option. An added Sender: gets rewritten here. */
 
-for (header_line * h = header_list->next; h; h = h->next)
-  {
-  header_line *newh = rewrite_header(h, NULL, NULL, global_rewrite_rules,
-    rewrite_existflags, TRUE);
-  if (newh) h = newh;
-  }
+DEBUG(D_rewrite)
+  { debug_printf("rewrite headers\n"); acl_level++; }
+for (header_line * h = header_list->next, * newh; h; h = h->next)
+  if ((newh = rewrite_header(h, NULL, NULL, global_rewrite_rules,
+			      rewrite_existflags, TRUE)))
+    h = newh;
+DEBUG(D_rewrite) acl_level--;
 
 
 /* An RFC 822 (sic) message is not legal unless it has at least one of "to",
diff --git a/src/src/rewrite.c b/src/src/rewrite.c
index fbcf863c2..42a63048f 100644
--- a/src/src/rewrite.c
+++ b/src/src/rewrite.c
@@ -454,7 +454,7 @@ uschar *s = Ustrchr(h->text, ':') + 1;
 while (isspace(*s)) s++;
 
 DEBUG(D_rewrite)
-  debug_printf("rewrite_one_header: type=%c:\n  %s", h->type, h->text);
+  debug_printf_indent("rewrite_one_header: type=%c:\n  %s", h->type, h->text);
 
 f.parse_allow_group = TRUE;     /* Allow group syntax */
 
@@ -484,7 +484,7 @@ while (*s)
   recipient = parse_extract_address(s, &errmess, &start, &end, &domain, FALSE);
   *ss = terminator;
   sprev = s;
-  s = ss + (terminator ? 1 :0);
+  s = ss + (terminator ? 1 : 0);
   while (isspace(*s)) s++;
 
   /* There isn't much we can do for syntactic disasters at this stage.
@@ -493,30 +493,17 @@ while (*s)
   empty address, overlong addres. Sometimes the result matters, sometimes not.
   It seems this function is called for *any* header we see. */
 
-
   if (!recipient)
     {
-#if 0
-    /* FIXME:
-    This was(!) an attempt tho handle empty rewrits, but seemingly it
-    needs more effort to decide if the returned empty address matters.
-    Now this will now break test 471 again.
-
-    471 fails now because it uses an overlong address, for wich parse_extract_address()
-    returns an empty address (which was not expected).
-
-    Checking the output and exit if rewrite_rules or routed_old are present
-    isn't a good idea either: It's enough to have *any* rewrite rule
-    in the configuration plus "To: undisclosed recpients:;" to exit(), which
-    is not what we want.
-    */
-
-    if (rewrite_rules || routed_old)
+    /* Handle unparesable addresses in the header. Slightly ugly because a
+    null output from the extract can also result from a header without an
+    address, "To: undisclosed recpients:;" being the classic case. */
+
+    if ((rewrite_rules || routed_old) && Ustrcmp(errmess, "empty address") != 0)
       {
       log_write(0, LOG_MAIN, "rewrite: %s", errmess);
       exim_exit(EXIT_FAILURE);
       }
-#endif
     loop_reset_point = store_reset(loop_reset_point);
     continue;
     }