The fix is in deliver.c only. The remainder is just tidying.
This means that a poorly-configured remote DNS will make it incommunicado;
but it protects against a DNS-interception attack on it.
This means that a poorly-configured remote DNS will make it incommunicado;
but it protects against a DNS-interception attack on it.
+JH/04 Bug 1810: make continued-use of an open smtp transport connection
+ non-noisy when a race steals the message being considered.
+
Exim version 4.87
-----------------
Exim version 4.87
-----------------
while queue running - another process probably completed delivery. As part of
opening the data file, message_subdir gets set. */
while queue running - another process probably completed delivery. As part of
opening the data file, message_subdir gets set. */
-if (!spool_open_datafile(id))
+if ((deliver_datafile = spool_open_datafile(id)) < 0)
return continue_closedown(); /* yields DELIVER_NOT_ATTEMPTED */
/* The value of message_size at this point has been set to the data length,
return continue_closedown(); /* yields DELIVER_NOT_ATTEMPTED */
/* The value of message_size at this point has been set to the data length,
int rc;
uschar * new_sender_address,
* save_sender_address;
int rc;
uschar * new_sender_address,
* save_sender_address;
+BOOL save_qr = queue_running;
-if (!spool_open_datafile(id))
+/* make spool_open_datafile non-noisy on fail */
+
+queue_running = TRUE;
+
+/* Side effect: message_subdir is set for the (possibly split) spool directory */
+
+deliver_datafile = spool_open_datafile(id);
+queue_running = save_qr;
+if (deliver_datafile < 0)
return NULL;
/* Save and restore the global sender_address. I'm not sure if we should
return NULL;
/* Save and restore the global sender_address. I'm not sure if we should
}
message_id = argv[msg_action_arg];
(void)string_format(spoolname, sizeof(spoolname), "%s-H", message_id);
}
message_id = argv[msg_action_arg];
(void)string_format(spoolname, sizeof(spoolname), "%s-H", message_id);
- if (!spool_open_datafile(message_id))
+ if ((deliver_datafile = spool_open_datafile(message_id)) < 0)
printf ("Failed to load message datafile %s\n", message_id);
if (spool_read_header(spoolname, TRUE, FALSE) != spool_read_OK)
printf ("Failed to load message %s\n", message_id);
printf ("Failed to load message datafile %s\n", message_id);
if (spool_read_header(spoolname, TRUE, FALSE) != spool_read_OK)
printf ("Failed to load message %s\n", message_id);
extern FILE *spool_mbox(unsigned long *, const uschar *);
#endif
extern BOOL spool_move_message(uschar *, uschar *, uschar *, uschar *);
extern FILE *spool_mbox(unsigned long *, const uschar *);
#endif
extern BOOL spool_move_message(uschar *, uschar *, uschar *, uschar *);
-extern BOOL spool_open_datafile(uschar *);
+extern int spool_open_datafile(uschar *);
extern int spool_open_temp(uschar *);
extern int spool_read_header(uschar *, BOOL, BOOL);
extern int spool_write_header(uschar *, int, uschar **);
extern int spool_open_temp(uschar *);
extern int spool_read_header(uschar *, BOOL, BOOL);
extern int spool_write_header(uschar *, int, uschar **);
only if the action is remove and the user is an admin user, to allow for
tidying up broken states. */
only if the action is remove and the user is an admin user, to allow for
tidying up broken states. */
-if (!spool_open_datafile(id))
+if ((deliver_datafile = spool_open_datafile(id)) < 0)
with append.
Argument: the id of the message
with append.
Argument: the id of the message
-Returns: TRUE if file successfully opened and locked
+Returns: fd if file successfully opened and locked, else -1
-Side effect: deliver_datafile is set to the fd of the open file.
+Side effect: message_subdir is set for the (possibly split) spool directory
spool_open_datafile(uschar *id)
{
int i;
struct stat statbuf;
flock_t lock_data;
uschar spoolname[256];
spool_open_datafile(uschar *id)
{
int i;
struct stat statbuf;
flock_t lock_data;
uschar spoolname[256];
/* If split_spool_directory is set, first look for the file in the appropriate
sub-directory of the input directory. If it is not found there, try the input
/* If split_spool_directory is set, first look for the file in the appropriate
sub-directory of the input directory. If it is not found there, try the input
int save_errno;
message_subdir[0] = (split_spool_directory == (i == 0))? id[5] : 0;
sprintf(CS spoolname, "%s/input/%s/%s-D", spool_directory, message_subdir, id);
int save_errno;
message_subdir[0] = (split_spool_directory == (i == 0))? id[5] : 0;
sprintf(CS spoolname, "%s/input/%s/%s-D", spool_directory, message_subdir, id);
- deliver_datafile = Uopen(spoolname, O_RDWR | O_APPEND, 0);
- if (deliver_datafile >= 0) break;
+ if ((fd = Uopen(spoolname, O_RDWR | O_APPEND, 0)) >= 0)
+ break;
save_errno = errno;
if (errno == ENOENT)
{
save_errno = errno;
if (errno == ENOENT)
{
else log_write(0, LOG_MAIN, "Spool error for %s: %s", spoolname,
strerror(errno));
errno = save_errno;
else log_write(0, LOG_MAIN, "Spool error for %s: %s", spoolname,
strerror(errno));
errno = save_errno;
}
/* File is open and message_subdir is set. Set the close-on-exec flag, and lock
}
/* File is open and message_subdir is set. Set the close-on-exec flag, and lock
Unix systems it doesn't make any difference as long as Exim is consistent in
what it locks. */
Unix systems it doesn't make any difference as long as Exim is consistent in
what it locks. */
-(void)fcntl(deliver_datafile, F_SETFD, fcntl(deliver_datafile, F_GETFD) |
+(void)fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) |
FD_CLOEXEC);
lock_data.l_type = F_WRLCK;
FD_CLOEXEC);
lock_data.l_type = F_WRLCK;
lock_data.l_start = 0;
lock_data.l_len = SPOOL_DATA_START_OFFSET;
lock_data.l_start = 0;
lock_data.l_len = SPOOL_DATA_START_OFFSET;
-if (fcntl(deliver_datafile, F_SETLK, &lock_data) < 0)
+if (fcntl(fd, F_SETLK, &lock_data) < 0)
{
log_write(L_skip_delivery,
LOG_MAIN,
"Spool file is locked (another process is handling this message)");
{
log_write(L_skip_delivery,
LOG_MAIN,
"Spool file is locked (another process is handling this message)");
- (void)close(deliver_datafile);
- deliver_datafile = -1;
}
/* Get the size of the data; don't include the leading filename line
in the count, but add one for the newline before the data. */
}
/* Get the size of the data; don't include the leading filename line
in the count, but add one for the newline before the data. */
-if (fstat(deliver_datafile, &statbuf) == 0)
+if (fstat(fd, &statbuf) == 0)
{
message_body_size = statbuf.st_size - SPOOL_DATA_START_OFFSET;
message_size = message_body_size + 1;
}
{
message_body_size = statbuf.st_size - SPOOL_DATA_START_OFFSET;
message_size = message_body_size + 1;
}
}
#endif /* COMPILE_UTILITY */
}
#endif /* COMPILE_UTILITY */
open_db *dbm_file;
uschar buffer[256];
open_db *dbm_file;
uschar buffer[256];
-msgq_t *msgq = NULL;
-int msgq_count = 0;
-int msgq_actual = 0;
uschar spool_dir [PATH_MAX];
uschar spool_file [PATH_MAX];
struct stat statbuf;
uschar spool_dir [PATH_MAX];
uschar spool_file [PATH_MAX];
struct stat statbuf;
-BOOL bContinuation = FALSE;
/* See if there is a record for this host; if not, there's nothing to do. */
/* See if there is a record for this host; if not, there's nothing to do. */
-host_record = dbfn_read(dbm_file, hostname);
-if (host_record == NULL)
+if (!(host_record = dbfn_read(dbm_file, hostname)))
{
dbfn_close(dbm_file);
DEBUG(D_transport) debug_printf("no messages waiting for %s\n", hostname);
{
dbfn_close(dbm_file);
DEBUG(D_transport) debug_printf("no messages waiting for %s\n", hostname);
+ msgq_t *msgq;
+ int msgq_count = 0;
+ int msgq_actual = 0;
+ BOOL bFound = FALSE;
+ BOOL bContinuation = FALSE;
+
/* create an array to read entire message queue into memory for processing */
msgq = (msgq_t*) malloc(sizeof(msgq_t) * host_record->count);
/* create an array to read entire message queue into memory for processing */
msgq = (msgq_t*) malloc(sizeof(msgq_t) * host_record->count);
/* now find the next acceptable message_id */
/* now find the next acceptable message_id */
for (i = msgq_count - 1; i >= 0; --i) if (msgq[i].bKeep)
{
if (split_spool_directory)
for (i = msgq_count - 1; i >= 0; --i) if (msgq[i].bKeep)
{
if (split_spool_directory)
msgq_actual++;
/* reassemble the host record, based on removed message ids, from in
msgq_actual++;
/* reassemble the host record, based on removed message ids, from in
/* Jeremy: check for a continuation record, this code I do not know how to
test but the code should work */
/* Jeremy: check for a continuation record, this code I do not know how to
test but the code should work */
- bContinuation = FALSE;
-
while (host_length <= 0)
{
int i;
while (host_length <= 0)
{
int i;
+ if (bFound) /* Usual exit from main loop */
+ {
+ free (msgq);
/* If host_length <= 0 we have emptied a record and not found a good message,
and there are no continuation records. Otherwise there is a continuation
/* If host_length <= 0 we have emptied a record and not found a good message,
and there are no continuation records. Otherwise there is a continuation
- Ustrcpy (new_message_id, message_id);
+ Ustrcpy(new_message_id, message_id);
dbfn_close(dbm_file);
return FALSE;
}
dbfn_close(dbm_file);
return FALSE;
}
- } /* we need to process a continuation record */
-/* clean up in memory queue */
-if (msgq)
- {
- free (msgq);
- msgq = NULL;
- msgq_count = 0;
- msgq_actual = 0;
- }
+ free(msgq);
+ } /* we need to process a continuation record */
/* Control gets here when an existing message has been encountered; its
id is in new_message_id, and host_length is the revised length of the
/* Control gets here when an existing message has been encountered; its
id is in new_message_id, and host_length is the revised length of the
- uschar msg [MESSAGE_ID_LENGTH + 1];
- int i;
-
host_record->count = host_length/MESSAGE_ID_LENGTH;
host_record->count = host_length/MESSAGE_ID_LENGTH;
- /* rebuild the host_record->text */
-
- for (i = 0; i < host_record->count; ++i)
- {
- Ustrncpy(msg, host_record->text + (i*MESSAGE_ID_LENGTH), MESSAGE_ID_LENGTH);
- msg[MESSAGE_ID_LENGTH] = 0;
- }
-
dbfn_write(dbm_file, hostname, host_record, (int)sizeof(dbdata_wait) + host_length);
*more = TRUE;
}
dbfn_write(dbm_file, hostname, host_record, (int)sizeof(dbdata_wait) + host_length);
*more = TRUE;
}
static BOOL
smtp_are_same_identities(uschar * message_id, smtp_compare_t * s_compare)
{
static BOOL
smtp_are_same_identities(uschar * message_id, smtp_compare_t * s_compare)
{
uschar * message_local_identity,
* current_local_identity,
* new_sender_address;
uschar * message_local_identity,
* current_local_identity,
* new_sender_address;