[2023-08-27 09:29] David Levine <levin...@acm.org> > Philipp wrote: > > > Do you only test the patch or have you looked at the code? It would > > be nice to get some feedback on the implementation. > > My only comment on the code itself is that I prefer functions that > just do one thing. So I would implement a fold function that just > modifies a string, and leave the fprintf/fwrite to output_headers().
I have thought about this, but this would require fold() to allocate memory. Because the plan was to use fold() in output_headers() I though this could be avoided. > There is a bigger issue, however. output_headers() gets called by > other code, such as mhfixmsg via output_message_fp(). So the change > as-is interferes badly with it. I haven't looked for a better place > to fold the header fiels, but we'll have to find one. As far as I see output_message_fp() is currently only used by mhfixmsg and mhbuild. For mhfixmsg I though it would be ok to also fold the field. For the blank lines you mentioned: Can you test the attached version of my patch. If this don't fix your problem, can you provide a sample mail? Philipp
From fb921a3fe127dade93b56b1c98c9f8cfc95da722 Mon Sep 17 00:00:00 2001 From: Philipp Takacs <philipp+...@bureaucracy.de> Date: Fri, 25 Aug 2023 09:29:42 +0200 Subject: [PATCH] mhbuild implement header folding --- Makefile.am | 2 ++ sbr/fold.c | 64 +++++++++++++++++++++++++++++++++++++++ sbr/fold.h | 7 +++++ test/mhbuild/test-mhbuild | 53 ++++++++++++++++++++++++++++++++ uip/mhoutsbr.c | 3 +- 5 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 sbr/fold.c create mode 100644 sbr/fold.h diff --git a/Makefile.am b/Makefile.am index 4fc84c1d..168d9fe6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -378,6 +378,7 @@ noinst_HEADERS = \ sbr/fmt_new.h \ sbr/fmt_rfc2047.h \ sbr/fmt_scan.h \ + sbr/fold.h \ sbr/folder_addmsg.h \ sbr/folder_delmsgs.h \ sbr/folder_free.h \ @@ -1106,6 +1107,7 @@ sbr_libmh_a_SOURCES = \ sbr/fmt_new.c \ sbr/fmt_rfc2047.c \ sbr/fmt_scan.c \ + sbr/fold.c \ sbr/folder_addmsg.c \ sbr/folder_delmsgs.c \ sbr/folder_free.c \ diff --git a/sbr/fold.c b/sbr/fold.c new file mode 100644 index 00000000..e7ae108f --- /dev/null +++ b/sbr/fold.c @@ -0,0 +1,64 @@ +/* fold.c -- fold a mail header field + * + * This code is Copyright (c), by the authors of nmh. See the + * COPYRIGHT file in the root directory of the nmh distribution for + * complete copyright information. */ + +#include "h/mh.h" +#include "h/mime.h" +#include "fold.h" +#include <stdio.h> + +void +fold(FILE *restrict stream, const char *restrict name, const char *restrict body) +{ + const char *restrict body_next; + const char *restrict wsp; + const char *restrict wsp_next; + const bool crlf = strchr(body, '\r'); + size_t namelen = fprintf(stream, "%s:", name); + + while (*body) { + body_next = strchr(body, '\n'); + if ((unsigned long) (body_next - body) <= MAXTEXTPERLN - namelen) { + fwrite(body, 1, body_next - body + 1, stream); + namelen = 0; + body = body_next + 1; + continue; + } + wsp = wsp_next = strpbrk(body, " \t"); + + /* if now whitespace is in the current line just print the curret line as is */ + if (!wsp_next || wsp_next > body_next) { + fwrite(body, 1, body_next - body + 1, stream); + namelen = 0; + body = body_next + 1; + continue; + } + + while ((unsigned long)(wsp_next - body) <= MAXTEXTPERLN - namelen) { + wsp = wsp_next; + wsp_next = strpbrk(wsp+1, " \t"); + if (!wsp_next) { + break; + } + if (wsp_next > body_next) { + break; + } + } + + fwrite(body, 1, wsp - body, stream); + if (crlf) { + fputs("\r\n", stream); + } else { + fputs("\n", stream); + } + namelen = 0; + while (*wsp == ' ' || *wsp == '\t') { + fputc(*wsp, stream); + namelen++; + wsp++; + } + body = wsp; + } +} diff --git a/sbr/fold.h b/sbr/fold.h new file mode 100644 index 00000000..185f1758 --- /dev/null +++ b/sbr/fold.h @@ -0,0 +1,7 @@ +/* fold.h -- fold a mail header field + * + * This code is Copyright (c), by the authors of nmh. See the + * COPYRIGHT file in the root directory of the nmh distribution for + * complete copyright information. */ + +void fold(FILE *restrict stream, const char *restrict name, const char *restrict body); diff --git a/test/mhbuild/test-mhbuild b/test/mhbuild/test-mhbuild index 706a804a..f8d4992b 100755 --- a/test/mhbuild/test-mhbuild +++ b/test/mhbuild/test-mhbuild @@ -221,5 +221,58 @@ run_test "mhbuild $f" \ check "$f" "$expected" +start_test "Checking for correct header folding" + +cat >"`mhpath new`" <<\E +From: Somebody <someb...@example.com> +To: Nobody <nob...@example.com> +Subject: Test message +References: <aaaaaaaaaaaaaaaaaaaaaaaa...@example.com> <aaaaaaaaaaaaaaaaaaaaaaaa...@example.com> <c...@example.com> +-------- +This is a test +E + +cat > "$expected" <<\E +From: Somebody <someb...@example.com> +To: Nobody <nob...@example.com> +Subject: Test message +References: <aaaaaaaaaaaaaaaaaaaaaaaa...@example.com> + <aaaaaaaaaaaaaaaaaaaaaaaa...@example.com> <c...@example.com> +MIME-Version: 1.0 +Content-Type: text/plain; charset="us-ascii" + +This is a test +E + +run_test "mhbuild -auto `mhpath last`" +check "`mhpath last`" "$expected" + +start_test "Checking header folding with a to long line" + +cat >"`mhpath new`" <<\E +From: Somebody <someb...@example.com> +To: Nobody <nob...@example.com> +Subject: Test message +References: <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...@example.com> <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...@example.com> +-------- +This is a test +E + +cat > "$expected" <<\E +From: Somebody <someb...@example.com> +To: Nobody <nob...@example.com> +Subject: Test message +References: + <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...@example.com> + <aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...@example.com> +MIME-Version: 1.0 +Content-Type: text/plain; charset="us-ascii" + +This is a test +E + +run_test "mhbuild -auto `mhpath last`" +check "`mhpath last`" "$expected" + finish_test exit $failed diff --git a/uip/mhoutsbr.c b/uip/mhoutsbr.c index 6033cad8..d54917b7 100644 --- a/uip/mhoutsbr.c +++ b/uip/mhoutsbr.c @@ -17,6 +17,7 @@ #include "h/mhparse.h" #include "mhoutsbr.h" #include "sbr/base64.h" +#include "sbr/fold.h" /* @@ -202,7 +203,7 @@ output_headers (CT ct, FILE *out) hp = ct->c_first_hf; while (hp) { - fprintf (out, "%s:%s", hp->name, hp->value); + fold (out, hp->name, hp->value); hp = hp->next; } } -- 2.39.2