From 1843f70b733127fcba3321d9d69359e05905f8cc Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 16 Oct 2021 00:12:16 +0100 Subject: [PATCH] Avoid calling gettimeofday(), select() per char for cmdline message submission. Bug 2819 Broken-by: 3c55eef240 --- doc/doc-txt/ChangeLog | 4 ++ src/src/exim.c | 7 ++- src/src/filtertest.c | 16 +++---- src/src/functions.h | 4 ++ src/src/globals.c | 21 +++++---- src/src/globals.h | 3 ++ src/src/receive.c | 78 ++++++++++++++++++++++------------ src/src/smtp_in.c | 24 ++++++++++- src/src/tls-gnu.c | 9 ++++ src/src/tls-openssl.c | 8 ++++ src/src/transports/autoreply.c | 13 +++--- 11 files changed, 133 insertions(+), 54 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 4e0d602dd..1a29ae50b 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -21,6 +21,10 @@ JH/03 Constification work in the filters module required a major version JH/04 Fix ClamAV TCP use under FreeBSD. Previously the OS-specific shim for sendfile() didi not account for the way the ClamAV driver code called it. +JH/05 Bug 2819: speed up command-line messages being read in. Previously a + time check was being done for every character; replace that with one + per buffer. + Exim version 4.95 ----------------- diff --git a/src/src/exim.c b/src/src/exim.c index 6e97981b0..f718366f6 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -5433,7 +5433,7 @@ if (smtp_input) { if (!f.is_inetd) set_process_info("accepting a local %sSMTP message from <%s>", smtp_batched_input? "batched " : "", - (sender_address!= NULL)? sender_address : originator_login); + sender_address ? sender_address : originator_login); } else { @@ -5483,7 +5483,8 @@ if (smtp_input) } } -/* Otherwise, set up the input size limit here. */ +/* Otherwise, set up the input size limit here and set no stdin stdio buffer +(we handle buferring so as to have visibility of fill level). */ else { @@ -5495,6 +5496,8 @@ else else log_write(0, LOG_MAIN|LOG_PANIC_DIE, "invalid value for " "message_size_limit: %s", expand_string_message); + + setvbuf(stdin, NULL, _IONBF, 0); } /* Loop for several messages when reading SMTP input. If we fork any child diff --git a/src/src/filtertest.c b/src/src/filtertest.c index a2a60a8dd..3b4752ea5 100644 --- a/src/src/filtertest.c +++ b/src/src/filtertest.c @@ -45,11 +45,11 @@ body_len = 0; body_linecount = 0; header_size = message_size; -if (!dot_ended && !feof(stdin)) +if (!dot_ended && !stdin_feof()) { if (!f.dot_ends) { - while ((ch = getc(stdin)) != EOF) + while ((ch = stdin_getc(GETC_BUFFER_UNLIMITED)) != EOF) { if (ch == 0) body_zerocount++; if (ch == '\n') body_linecount++; @@ -62,7 +62,7 @@ if (!dot_ended && !feof(stdin)) else { int ch_state = 1; - while ((ch = getc(stdin)) != EOF) + while ((ch = stdin_getc(GETC_BUFFER_UNLIMITED)) != EOF) { if (ch == 0) body_zerocount++; switch (ch_state) @@ -99,6 +99,7 @@ if (!dot_ended && !feof(stdin)) } if (s == message_body_end || s[-1] != '\n') body_linecount++; } +debug_printf("%s %d\n", __FUNCTION__, __LINE__); message_body[body_len] = 0; message_body_size = message_size - header_size; @@ -250,7 +251,7 @@ if (filter_type == FILTER_FORWARD) /* For a filter, set up the message_body variables and the message size if this is the first time this function has been called. */ -if (message_body == NULL) read_message_body(dot_ended); +if (!message_body) read_message_body(dot_ended); /* Now pass the filter file to the function that interprets it. Because filter_test is not FILTER_NONE, the interpreter will output comments about what @@ -269,10 +270,9 @@ if (is_system) } else { - yield = (filter_type == FILTER_SIEVE)? - sieve_interpret(filebuf, RDO_REWRITE, NULL, NULL, NULL, NULL, &generated, &error) - : - filter_interpret(filebuf, RDO_REWRITE, &generated, &error); + yield = filter_type == FILTER_SIEVE + ? sieve_interpret(filebuf, RDO_REWRITE, NULL, NULL, NULL, NULL, &generated, &error) + : filter_interpret(filebuf, RDO_REWRITE, &generated, &error); } return yield != FF_ERROR; diff --git a/src/src/functions.h b/src/src/functions.h index 3f04ec782..401300d94 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -68,6 +68,7 @@ extern void tls_free_cert(void **); extern int tls_getc(unsigned); extern uschar *tls_getbuf(unsigned *); extern void tls_get_cache(unsigned); +extern BOOL tls_hasc(void); extern BOOL tls_import_cert(const uschar *, void **); extern BOOL tls_is_name_for_cert(const uschar *, void *); # ifdef USE_OPENSSL @@ -150,6 +151,7 @@ extern uschar *b64encode_taint(const uschar *, int, BOOL); extern int b64decode(const uschar *, uschar **); extern int bdat_getc(unsigned); extern uschar *bdat_getbuf(unsigned *); +extern BOOL bdat_hasc(void); extern int bdat_ungetc(int); extern void bdat_flush_data(void); @@ -496,6 +498,7 @@ extern BOOL smtp_get_port(uschar *, address_item *, int *, uschar *); extern int smtp_getc(unsigned); extern uschar *smtp_getbuf(unsigned *); extern void smtp_get_cache(unsigned); +extern BOOL smtp_hasc(void); extern int smtp_handle_acl_fail(int, int, uschar *, uschar *); extern void smtp_log_no_mail(void); extern void smtp_message_code(uschar **, int *, uschar **, uschar **, BOOL); @@ -525,6 +528,7 @@ extern int spool_write_header(uschar *, int, uschar **); extern int stdin_getc(unsigned); extern int stdin_feof(void); extern int stdin_ferror(void); +extern BOOL stdin_hasc(void); extern int stdin_ungetc(int); extern void store_exit(void); diff --git a/src/src/globals.c b/src/src/globals.c index 1adb540c2..200b506f7 100644 --- a/src/src/globals.c +++ b/src/src/globals.c @@ -171,16 +171,19 @@ incoming TCP/IP. The defaults use stdin. We never need these for any stand-alone tests. */ #if !defined(STAND_ALONE) && !defined(MACRO_PREDEF) -int (*lwr_receive_getc)(unsigned) = stdin_getc; +int (*lwr_receive_getc)(unsigned) = stdin_getc; uschar * (*lwr_receive_getbuf)(unsigned *) = NULL; -int (*lwr_receive_ungetc)(int) = stdin_ungetc; -int (*receive_getc)(unsigned) = stdin_getc; -uschar * (*receive_getbuf)(unsigned *) = NULL; -void (*receive_get_cache)(unsigned) = NULL; -int (*receive_ungetc)(int) = stdin_ungetc; -int (*receive_feof)(void) = stdin_feof; -int (*receive_ferror)(void) = stdin_ferror; -BOOL (*receive_smtp_buffered)(void) = NULL; /* Only used for SMTP */ +int (*lwr_receive_ungetc)(int) = stdin_ungetc; +BOOL (*lwr_receive_hasc)(void) = stdin_hasc; + +int (*receive_getc)(unsigned) = stdin_getc; +uschar * (*receive_getbuf)(unsigned *) = NULL; +void (*receive_get_cache)(unsigned) = NULL; +BOOL (*receive_hasc)(void) = stdin_hasc; +int (*receive_ungetc)(int) = stdin_ungetc; +int (*receive_feof)(void) = stdin_feof; +int (*receive_ferror)(void) = stdin_ferror; +BOOL (*receive_smtp_buffered)(void) = NULL; /* Only used for SMTP */ #endif diff --git a/src/src/globals.h b/src/src/globals.h index bd88f7482..ed264f0c1 100644 --- a/src/src/globals.h +++ b/src/src/globals.h @@ -161,9 +161,12 @@ incoming TCP/IP. */ extern int (*lwr_receive_getc)(unsigned); extern uschar * (*lwr_receive_getbuf)(unsigned *); +extern BOOL (*lwr_receive_hasc)(void); extern int (*lwr_receive_ungetc)(int); + extern int (*receive_getc)(unsigned); extern uschar * (*receive_getbuf)(unsigned *); +extern BOOL (*receive_hasc)(void); extern void (*receive_get_cache)(unsigned); extern int (*receive_ungetc)(int); extern int (*receive_feof)(void); diff --git a/src/src/receive.c b/src/src/receive.c index 3a9a91a81..d2e556e32 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -44,42 +44,71 @@ receive_getc initially. They just call the standard functions, passing stdin as the file. (When SMTP input is occurring, different functions are used by changing the pointer variables.) */ -int -stdin_getc(unsigned lim) -{ -int c = getc(stdin); +uschar stdin_buf[4096]; +uschar * stdin_inptr = stdin_buf; +uschar * stdin_inend = stdin_buf; -if (had_data_timeout) - { - fprintf(stderr, "exim: timed out while reading - message abandoned\n"); - log_write(L_lost_incoming_connection, - LOG_MAIN, "timed out while reading local message"); - receive_bomb_out(US"data-timeout", NULL); /* Does not return */ - } -if (had_data_sigint) +static BOOL +stdin_refill(void) +{ +size_t rc = fread(stdin_buf, 1, sizeof(stdin_buf), stdin); +if (rc <= 0) { - if (filter_test == FTEST_NONE) + if (had_data_timeout) { - fprintf(stderr, "\nexim: %s received - message abandoned\n", - had_data_sigint == SIGTERM ? "SIGTERM" : "SIGINT"); - log_write(0, LOG_MAIN, "%s received while reading local message", - had_data_sigint == SIGTERM ? "SIGTERM" : "SIGINT"); + fprintf(stderr, "exim: timed out while reading - message abandoned\n"); + log_write(L_lost_incoming_connection, + LOG_MAIN, "timed out while reading local message"); + receive_bomb_out(US"data-timeout", NULL); /* Does not return */ } - receive_bomb_out(US"signal-exit", NULL); /* Does not return */ + if (had_data_sigint) + { + if (filter_test == FTEST_NONE) + { + fprintf(stderr, "\nexim: %s received - message abandoned\n", + had_data_sigint == SIGTERM ? "SIGTERM" : "SIGINT"); + log_write(0, LOG_MAIN, "%s received while reading local message", + had_data_sigint == SIGTERM ? "SIGTERM" : "SIGINT"); + } + receive_bomb_out(US"signal-exit", NULL); /* Does not return */ + } + return FALSE; } -return c; +stdin_inend = stdin_buf + rc; +stdin_inptr = stdin_buf; +return TRUE; +} + +int +stdin_getc(unsigned lim) +{ +if (stdin_inptr >= stdin_inend) + if (!stdin_refill()) + return EOF; +return *stdin_inptr++; +} + + +BOOL +stdin_hasc(void) +{ +return stdin_inptr < stdin_inend; } int stdin_ungetc(int c) { -return ungetc(c, stdin); +if (stdin_inptr <= stdin_buf) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "buffer underflow in stdin_ungetc"); + +*--stdin_inptr = c; +return c; } int stdin_feof(void) { -return feof(stdin); +return stdin_hasc() ? FALSE : feof(stdin); } int @@ -588,7 +617,7 @@ the file copy. */ static void log_close_chk(void) { -if (!receive_timeout) +if (!receive_timeout && !receive_hasc()) { struct timeval t; timesince(&t, &received_time); @@ -654,11 +683,6 @@ if (!f.dot_ends) { int last_ch = '\n'; -/*XXX we do a gettimeofday before checking for every received char, -which is hardly clever. The function-indirection doesn't help, but -an additional function to check for nonempty read buffer would help. -See stdin_getc() / smtp_getc() / tls_getc() / bdat_getc(). */ - for ( ; log_close_chk(), (ch = (receive_getc)(GETC_BUFFER_UNLIMITED)) != EOF; last_ch = ch) diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 41232d7cd..c1aa5867c 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -563,6 +563,12 @@ if (smtp_inptr >= smtp_inend) return *smtp_inptr++; } +BOOL +smtp_hasc(void) +{ +return smtp_inptr < smtp_inend; +} + uschar * smtp_getbuf(unsigned * len) { @@ -745,6 +751,14 @@ next_cmd: } } +BOOL +bdat_hasc(void) +{ +if (chunking_data_left > 0) + return lwr_receive_hasc(); +return TRUE; +} + uschar * bdat_getbuf(unsigned * len) { @@ -784,10 +798,11 @@ bdat_push_receive_functions(void) /* push the current receive_* function on the "stack", and replace them by bdat_getc(), which in turn will use the lwr_receive_* functions to do the dirty work. */ -if (lwr_receive_getc == NULL) +if (!lwr_receive_getc) { lwr_receive_getc = receive_getc; lwr_receive_getbuf = receive_getbuf; + lwr_receive_hasc = receive_hasc; lwr_receive_ungetc = receive_ungetc; } else @@ -797,23 +812,26 @@ else receive_getc = bdat_getc; receive_getbuf = bdat_getbuf; +receive_hasc = bdat_hasc; receive_ungetc = bdat_ungetc; } static inline void bdat_pop_receive_functions(void) { -if (lwr_receive_getc == NULL) +if (!lwr_receive_getc) { DEBUG(D_receive) debug_printf("chunking double-pop receive functions\n"); return; } receive_getc = lwr_receive_getc; receive_getbuf = lwr_receive_getbuf; +receive_hasc = lwr_receive_hasc; receive_ungetc = lwr_receive_ungetc; lwr_receive_getc = NULL; lwr_receive_getbuf = NULL; +lwr_receive_hasc = NULL; lwr_receive_ungetc = NULL; } @@ -2576,12 +2594,14 @@ smtp_inbuffer[IN_BUFFER_SIZE-1] = '\0'; receive_getc = smtp_getc; receive_getbuf = smtp_getbuf; receive_get_cache = smtp_get_cache; +receive_hasc = smtp_hasc; receive_ungetc = smtp_ungetc; receive_feof = smtp_feof; receive_ferror = smtp_ferror; receive_smtp_buffered = smtp_buffered; lwr_receive_getc = NULL; lwr_receive_getbuf = NULL; +lwr_receive_hasc = NULL; lwr_receive_ungetc = NULL; smtp_inptr = smtp_inend = smtp_inbuffer; smtp_had_eof = smtp_had_error = 0; diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index 4626dc945..c6625c060 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -3138,6 +3138,7 @@ state->xfer_buffer = store_malloc(ssl_xfer_buffer_size); receive_getc = tls_getc; receive_getbuf = tls_getbuf; receive_get_cache = tls_get_cache; +receive_hasc = tls_hasc; receive_ungetc = tls_ungetc; receive_feof = tls_feof; receive_ferror = tls_ferror; @@ -3740,6 +3741,7 @@ if (!ct_ctx) /* server */ receive_getc = smtp_getc; receive_getbuf = smtp_getbuf; receive_get_cache = smtp_get_cache; + receive_hasc = smtp_hasc; receive_ungetc = smtp_ungetc; receive_feof = smtp_feof; receive_ferror = smtp_ferror; @@ -3854,6 +3856,13 @@ if (state->xfer_buffer_lwm >= state->xfer_buffer_hwm) return state->xfer_buffer[state->xfer_buffer_lwm++]; } +BOOL +tls_hasc(void) +{ +exim_gnutls_state_st * state = &state_server; +return state->xfer_buffer_lwm < state->xfer_buffer_hwm; +} + uschar * tls_getbuf(unsigned * len) { diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index bc454c8a1..fddad9edc 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -3350,6 +3350,7 @@ ssl_xfer_eof = ssl_xfer_error = FALSE; receive_getc = tls_getc; receive_getbuf = tls_getbuf; receive_get_cache = tls_get_cache; +receive_hasc = tls_hasc; receive_ungetc = tls_ungetc; receive_feof = tls_feof; receive_ferror = tls_ferror; @@ -4126,6 +4127,12 @@ if (ssl_xfer_buffer_lwm >= ssl_xfer_buffer_hwm) return ssl_xfer_buffer[ssl_xfer_buffer_lwm++]; } +BOOL +tls_hasc(void) +{ +return ssl_xfer_buffer_lwm < ssl_xfer_buffer_hwm; +} + uschar * tls_getbuf(unsigned * len) { @@ -4415,6 +4422,7 @@ if (!o_ctx) /* server side */ receive_getc = smtp_getc; receive_getbuf = smtp_getbuf; receive_get_cache = smtp_get_cache; + receive_hasc = smtp_hasc; receive_ungetc = smtp_ungetc; receive_feof = smtp_feof; receive_ferror = smtp_ferror; diff --git a/src/src/transports/autoreply.c b/src/src/transports/autoreply.c index 864257b46..83b7fb685 100644 --- a/src/src/transports/autoreply.c +++ b/src/src/transports/autoreply.c @@ -646,6 +646,7 @@ if (text) if (ff) { +debug_printf("%s %d: ff\n", __FUNCTION__, __LINE__); while (Ufgets(big_buffer, big_buffer_size, ff) != NULL) { if (file_expand) @@ -669,12 +670,12 @@ limit if we are returning the body. */ if (return_message) { - uschar *rubric = (tblock->headers_only)? - US"------ This is a copy of the message's header lines.\n" - : (tblock->body_only)? - US"------ This is a copy of the body of the message, without the headers.\n" - : - US"------ This is a copy of the message, including all the headers.\n"; +debug_printf("%s %d: ret msg\n", __FUNCTION__, __LINE__); + uschar *rubric = tblock->headers_only + ? US"------ This is a copy of the message's header lines.\n" + : tblock->body_only + ? US"------ This is a copy of the body of the message, without the headers.\n" + : US"------ This is a copy of the message, including all the headers.\n"; transport_ctx tctx = { .u = {.fd = fileno(fp)}, .tblock = tblock, -- 2.30.2