From: Thibault Godouet Date: Sat, 24 Aug 2024 15:49:55 +0000 (+0100) Subject: Refactor maildisplayname. X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=63fb462916a9cbdf7200dcbaa158748b2e606bb4;p=thirdparty%2Ffcron.git Refactor maildisplayname. --- diff --git a/Makefile.in b/Makefile.in index 79b1583..cfcfd95 100644 --- a/Makefile.in +++ b/Makefile.in @@ -81,7 +81,7 @@ OBJSTAB := fcrontab.o cl.o subs.o mem.o save.o temp_file.o log.o fileconf OBJSDYN := fcrondyn.o subs.o mem.o log.o allow.o read_string.o fcronconf.o filesubs.o OBJCONV := convert-fcrontab.o cl.o subs.o mem.o save.o log.o u_list.o env_list.o fcronconf.o filesubs.o OBJSIG := fcronsighup.o subs.o mem.o log.o allow.o fcronconf.o filesubs.o -HEADERSALL := config.h $(SRCDIR)/global.h $(SRCDIR)/cl.h $(SRCDIR)/log.h $(SRCDIR)/subs.h $(SRCDIR)/mem.h $(SRCDIR)/save.h $(SRCDIR)/option.h $(SRCDIR)/dyncom.h +HEADERSALL := config.h $(SRCDIR)/global.h $(SRCDIR)/cl.h $(SRCDIR)/log.h $(SRCDIR)/subs.h $(SRCDIR)/mem.h $(SRCDIR)/save.h $(SRCDIR)/option.h $(SRCDIR)/dyncom.h $(SRCDIR)/mail.h # this is a regular expression : # do not ci automaticaly generated files and doc (done by doc's Makefile) diff --git a/fcronconf.c b/fcronconf.c index a1c5a44..b5280e2 100644 --- a/fcronconf.c +++ b/fcronconf.c @@ -25,6 +25,7 @@ #include "global.h" #include "mem.h" #include "fcronconf.h" +#include "mail.h" #include "subs.h" void init_conf(void); @@ -47,7 +48,7 @@ char *editor = NULL; char *displayname = NULL; char -*format_displayname(char *conf_value) +*format_displayname(char *displayname_conf) /* Format the input string `conf_value` according to RFC5322 sec. 3.2.3. * . * Returns: either the formatted displayname (possibly unchanged or empty) @@ -56,56 +57,51 @@ char { bool need_quotes = false; char c = '\0'; - char *bpos = NULL, *dpos = NULL; - char *buf1 = NULL, *buf2 = NULL; + char *ipos = NULL; /* Input position */ + char *output = NULL, *quoted_output = NULL; - /* Shorter than max because of the option prefix "displayname = " */ - const uint buf_len = LINE_LEN - 14; - uint cwritten = 0; + const uint buf_len = MAIL_FROM_VALUE_LEN_MAX; + uint cwritten = 0; /* how many chars we have written */ - if (strlen(conf_value) == 0) return strdup2(""); + if (strlen(displayname_conf) == 0) return strdup2(""); - buf1 = (char *)alloc_safe(buf_len * sizeof(char), "1st buffer"); - buf2 = (char *)alloc_safe(buf_len * sizeof(char), "2nd buffer"); + output = (char *)alloc_safe(buf_len * sizeof(char), "output buffer"); /* walk the conf_value and rebuild it in buf1 */ - bpos = buf1; - for (dpos = conf_value; *dpos; *dpos++) { - c = *dpos; + for (ipos = displayname_conf; *ipos; ipos++) { + c = *ipos; if (strchr(SPECIAL_MBOX_CHARS, c)) { /* insert escape */ if (c == DQUOTE) { - *bpos++ = BSLASH; - ++cwritten; + output[cwritten] = BSLASH; + cwritten++; } need_quotes = true; } if (cwritten >= buf_len) { error("Formatted 'displayname' exceeds %u chars", buf_len); - Free_safe(buf1); - Free_safe(buf2); + Free_safe(output); return NULL; } - *bpos++ = c; - ++cwritten; + output[cwritten] = c; + cwritten++; } if (need_quotes) { - if (snprintf(buf2, buf_len, "\"%s\"", buf1) >= buf_len){ - error("Formatted 'displayname' exceeds %u chars", buf_len); - Free_safe(buf1); - Free_safe(buf2); + quoted_output = (char *)alloc_safe(buf_len * sizeof(char), "quoted output buffer"); + int needed_len = snprintf(quoted_output, buf_len, "\"%s\"", output); + if (needed_len >= buf_len){ + error("Formatted 'displayname' too long: length:%u > max:%u chars", needed_len, buf_len); + Free_safe(output); + Free_safe(quoted_output); return NULL; } - Free_safe(buf1); + Free_safe(output); - return buf2; + return quoted_output; } - /* unchanged */ - Free_safe(buf2); - - return buf1; + return output; } void diff --git a/files/fcron.conf.in b/files/fcron.conf.in index 1443613..108696a 100644 --- a/files/fcron.conf.in +++ b/files/fcron.conf.in @@ -26,10 +26,10 @@ editor = @@FCRON_EDITOR@ # Display name for the "From: " header of mails sent by us. Default is # unset/empty, which keeps the "From: " header as it was before this feature -# was added -- not RFC5322-compliant, though. A sane RFC5322-compliant setting, -# could be: +# was added -- not RFC5322-compliant, though. +# The default will soon be the sane RFC5322-compliant: # -# displayname = Fcron Deamon +# displayname = Fcron Daemon # # Please read fcron.conf(5) before setting it! displayname = @@DISPLAYNAME@ diff --git a/job.c b/job.c index 71a0ee9..5191255 100644 --- a/job.c +++ b/job.c @@ -25,6 +25,7 @@ #include "fcron.h" #include "job.h" +#include "mail.h" #include "temp_file.h" @@ -289,14 +290,14 @@ make_mailbox_addr(char *displayname_conf, char *mail_from, char *hostname) uint written = 0; bool need_anglebrackets = false; - /* Shorter than max because the header prefix "From: " are added - downstream. */ - const uint buf_len = MAIL_LINE_LEN_MAX - 6; + const uint buf_len = MAIL_FROM_VALUE_LEN_MAX+1; buf = (char *)alloc_safe(buf_len * sizeof(char), "mailbox addr buffer"); - /* == strlen(),but faster */ - need_anglebrackets = displayname_conf[0] != '\0'; + if (displayname_conf[0] != '\0') { + /* displayname_conf isn't an empty string */ + need_anglebrackets = true; + } /* no @ here, it's handled upstream */ if (need_anglebrackets) @@ -327,7 +328,7 @@ create_mail(cl_t * line, char *subject, char *content_type, char *encoding, /* hostname to add to email addresses (depending on if they have a '@') */ char *hostname_from = ""; char *hostname_to = ""; - char *mailbox_addr = ""; + char *mailbox_addr = NULL; int i = 0; if (mailf == NULL) @@ -357,21 +358,21 @@ create_mail(cl_t * line, char *subject, char *content_type, char *encoding, #endif /* HAVE_GETHOSTNAME */ /* write mail header. Global 'displayname' comes from fcronconf.h */ - if (strlen(displayname) > 0){ + if (displayname[0] != '\0'){ /* New behavior -- RFC-compliant */ mailbox_addr = make_mailbox_addr(displayname, mailfrom, hostname_from); - if (mailbox_addr) { - fprintf(mailf, "From: %s\n", mailbox_addr); - Free_safe(mailbox_addr); - } - else { - error("could not make the mailbox address"); - fprintf(mailf, "From: %s%s\n", mailfrom, hostname_from); + if (! mailbox_addr) { + warn("could not make the mailbox address"); } } - else + if (mailbox_addr) { + fprintf(mailf, FROM_HEADER_KEY"%s\n", mailbox_addr); + Free_safe(mailbox_addr); + } + else { /* Old behavior */ - fprintf(mailf, "From: %s%s (fcron)\n", mailfrom, hostname_from); + fprintf(mailf, FROM_HEADER_KEY"%s%s (fcron)\n", mailfrom, hostname_from); + } fprintf(mailf, "To: %s%s\n", line->cl_mailto, hostname_to); diff --git a/job.h b/job.h index a83c46b..046e247 100644 --- a/job.h +++ b/job.h @@ -26,6 +26,7 @@ #define __JOB_H__ #define MAIL_LINE_LEN_MAX 998 /* RFC5322's max line length */ +#define FROM_HEADER_KEY "From: " /* functions prototypes */ extern void change_user_setup_env(struct cl_t *cl, char ***sendmailenv, diff --git a/mail.h b/mail.h new file mode 100644 index 0000000..86a82c5 --- /dev/null +++ b/mail.h @@ -0,0 +1,32 @@ +/* + * FCRON - periodic command scheduler + * + * Copyright 2000-2021 Thibault Godouet + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * The GNU General Public License can also be found in the file + * `LICENSE' that comes with the fcron source distribution. + */ + + +#ifndef __MAIL_H__ +#define __MAIL_H__ + +#define MAIL_LINE_LEN_MAX 998 /* RFC5322's max line length */ +#define FROM_HEADER_KEY "From: " +#define MAIL_FROM_VALUE_LEN_MAX (MAIL_LINE_LEN_MAX - sizeof(FROM_HEADER_KEY)) + +#endif /* __MAIL_H__ */ diff --git a/test/mailbox_addr.c b/test/mailbox_addr.c index cb3cbeb..c2603b4 100644 --- a/test/mailbox_addr.c +++ b/test/mailbox_addr.c @@ -7,13 +7,20 @@ #include "fcron.h" #include "job.h" #include "fcronconf.h" +#include "mail.h" -#define dasserteq(DESCR, RETURNED, EXPECTED) \ +#define AssertEq(DESCR, RETURNED, EXPECTED) \ { \ - printf("%s: %ld ?= %ld\n", (DESCR), (long)(RETURNED), (long)(EXPECTED)); \ + printf("%s: check %ld == %ld\n", (DESCR), (long)(RETURNED), (long)(EXPECTED)); \ assert((RETURNED) == (EXPECTED)); \ } +#define AssertNeq(DESCR, RETURNED, EXPECTED) \ +{ \ + printf("%s: check %ld != %ld\n", (DESCR), (long)(RETURNED), (long)(EXPECTED)); \ + assert((RETURNED) != (EXPECTED)); \ +} + /* These globals should maybe come from a lib instead of the various exes? */ char *prog_name = NULL; pid_t daemon_pid = 0; @@ -103,22 +110,57 @@ int main(int argc, char* argv[]) "Foo Bar", "baz", "@quux", "Foo Bar "); - /* overflow tests */ + /* + * format_displayname() overflow tests + */ + displayname = (char *)realloc_safe(displayname, - LINE_LEN + 1 * sizeof(char), + MAIL_FROM_VALUE_LEN_MAX * 2 * sizeof(char), "displayname buffer"); - memset(displayname, 'a', LINE_LEN); - displayname[LINE_LEN] = '\0'; - displayname[0] = DQUOTE; + memset(displayname, 'a', MAIL_FROM_VALUE_LEN_MAX*2); + + printf("=== format_displayname: one-char overflow with no expansion...\n"); + displayname[MAIL_FROM_VALUE_LEN_MAX+1] = '\0'; + output = format_displayname(displayname); + AssertEq("format_displayname: overflow with no expansion", output, NULL); + Free_safe(output); + + + printf("=== format_displayname: max size with no special char...\n"); + displayname[MAIL_FROM_VALUE_LEN_MAX] = '\0'; output = format_displayname(displayname); - dasserteq("format_displayname: overflow", output, NULL); + _test_format_displayname("format_displayname: max size with no special chars", + output, displayname); + printf("format_displayname: max size with no special char: len(output)=%lu, MAIL_FROM_VALUE_LEN_MAX=%lu\n", strlen(output), MAIL_FROM_VALUE_LEN_MAX); Free_safe(output); + printf("=== format_displayname: overflow on max size with special char...\n"); + displayname[0] = DQUOTE; + output = format_displayname(displayname); + AssertEq("format_displayname: overflow on max size with special char", output, NULL); + Free_safe(output); + + /* + * make_mailbox_addr() overflow tests + */ + + printf("=== make_mailbox_addr: overflow on displayname at max size + 1...\n"); + /* make_mailbox_addr() adds 3 chars: space, <, > */ + displayname[MAIL_FROM_VALUE_LEN_MAX-strlen("baz")-strlen("@quux")-2] = '\0'; output = make_mailbox_addr(displayname, "baz", "@quux"); - dasserteq("make_mailbox_addr: overflow", output, NULL); + AssertEq("make_mailbox_addr: overflow", output, NULL); Free_safe(output); + printf("=== make_mailbox_addr: displayname at max size...\n"); + /* make_mailbox_addr() adds 3 chars: space, <, > */ + displayname[MAIL_FROM_VALUE_LEN_MAX-strlen("baz")-strlen("@quux")-3] = '\0'; + output = make_mailbox_addr(displayname, "baz", "@quux"); + printf("make_mailbox_addr: max size with no special char: len(output)=%lu, MAIL_FROM_VALUE_LEN_MAX=%lu\n", strlen(output), MAIL_FROM_VALUE_LEN_MAX); + AssertNeq("make_mailbox_addr: max size", output, NULL); + Free_safe(output); + + Free_safe(displayname); return 0;