Re: mhbuild and long header fields
Andy wrote: > Minor correction; replace "then" with "than". > > Also, RFC 5322 2.1.1 does not "require" that the lines be no more than > 78 bytes, only that it's highly recommended that they not exceed that. > The actual line limit is 998 bytes. Good catches, thank you! And thank you, Phillipp. This bug fix/enhancement is well worth your efforts. commit 542cb12b6d0646b711772ee97c1e2aacf2bada86 Author: Philipp Date: Mon Sep 18 12:50:22 2023 +0200 mhbuild now folds header field bodies to avoid lines with more than 78 bytes. This is recommended by RFC 5322 2.1.1. Line Length Limits. Signed-off-by: David Levine David
Re: mhbuild and long header fields
Thus said David Levine on Sun, 17 Sep 2023 21:51:04 -0400: > How about this? > > mhbuild now folds header field bodies to avoid lines with more > then 78 bytes. > > This is required by RFC 5322 2.1.1. Line Length Limits. > Minor correction; replace "then" with "than". Also, RFC 5322 2.1.1 does not "require" that the lines be no more than 78 bytes, only that it's highly recommended that they not exceed that. The actual line limit is 998 bytes. Thanks, Andy
Re: mhbuild and long header fields
[2023-09-17 21:51] David Levine
> > A format-patch is attached.
>
> Thanks. I think that the commit message should note that only header field
> bodies are folded. And because folding can only occur a
> t whitespace, it is possible to end up with more than 78 bytes in a line.
> How about this?
>
> mhbuild now folds header field bodies to avoid lines with more then 78
> bytes.
>
> This is required by RFC 5322 2.1.1. Line Length Limits.
>
> I'll add the first line of the commit message to docs/pending-release-notes.
> And add the year to the copyright notice in fold.[hc].
Done, updated version is attached.
Philipp
From 3157510d821db9bf62b051aeb1dcc94fac1dffcd Mon Sep 17 00:00:00 2001
From: Philipp Takacs
Date: Fri, 25 Aug 2023 09:29:42 +0200
Subject: [PATCH] mhbuild implement header folding
mhbuild now folds header field bodies to avoid lines with more then 78 bytes.
This is required by RFC 5322 2.1.1. Line Length Limits.
---
Makefile.am | 2 ++
sbr/fold.c| 63 +++
sbr/fold.h| 7 +
test/mhbuild/test-mhbuild | 53
uip/mhoutsbr.c| 7 -
5 files changed, 131 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 ..f6047703
--- /dev/null
+++ b/sbr/fold.c
@@ -0,0 +1,63 @@
+/* fold.c -- fold a mail header field
+ *
+ * This code is Copyright (c) 2023, 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 "sbr/charstring.h"
+#include "fold.h"
+
+void
+fold(charstring_t dst, size_t namelen, const char *restrict body)
+{
+ const char *restrict body_next;
+ const char *restrict wsp;
+ const char *restrict wsp_next;
+ const bool crlf = strchr(body, '\r');
+ charstring_clear(dst);
+ namelen++;
+
+ while (*body) {
+ body_next = strchr(body, '\n');
+ if ((unsigned long) (body_next - body) <= MAXTEXTPERLN - namelen) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ namelen = 0;
+ body = body_next + 1;
+ continue;
+ }
+ wsp = body;
+ while (namelen == 0 && (*wsp == ' ' || *wsp == '\t')) {
+ wsp++;
+ }
+ wsp = wsp_next = strpbrk(wsp, " \t");
+
+ /* if now whitespace is in the current line just print the curret line as is */
+ if (!wsp_next || wsp_next > body_next) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ 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;
+ }
+ }
+
+ charstring_push_back_chars(dst, body, wsp - body, wsp - body);
+ if (crlf) {
+ charstring_push_back(dst, '\r');
+ }
+ charstring_push_back(dst, '\n');
+ namelen = 0;
+ body = wsp;
+ }
+}
diff --git a/sbr/fold.h b/sbr/fold.h
new file mode 100644
index ..40bf6a49
--- /dev/null
+++ b/sbr/fold.h
@@ -0,0 +1,7 @@
+/* fold.h -- fold a mail header field
+ *
+ * This code is Copyright (c) 2023, by the authors of nmh. See the
+ * COPYRIGHT file in the root directory of the nmh distribution for
+ * complete copyright information. */
+
+void fold(charstring_t dst, size_t namelen, 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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject:
Re: mhbuild and long header fields
Philipp wrote: > A format-patch is attached. Thanks. I think that the commit message should note that only header field bodies are folded. And because folding can only occur at whitespace, it is possible to end up with more than 78 bytes in a line. How about this? mhbuild now folds header field bodies to avoid lines with more then 78 bytes. This is required by RFC 5322 2.1.1. Line Length Limits. I'll add the first line of the commit message to docs/pending-release-notes. And add the year to the copyright notice in fold.[hc]. David
Re: mhbuild and long header fields
[2023-09-17 18:32] David Levine
> Philipp wrote:
>
> > Good to hear. Do you want a format-patch of the final version?
>
> Sure, that would be great. Or, all I need is the commit message. I just
> verified that I'm using your last code submission with no
> changes.
A format-patch is attached.
Thanks for good feedback.
Philipp
From c2fce54f46b3a393457b242c65b0e0af95ecfa21 Mon Sep 17 00:00:00 2001
From: Philipp Takacs
Date: Fri, 25 Aug 2023 09:29:42 +0200
Subject: [PATCH] mhbuild implement header folding
mhbuild now folds header fields to prevent lines with more then 78
bytes. This is required by RFC 5322 2.1.1. Line Length Limits.
---
Makefile.am | 2 ++
sbr/fold.c| 63 +++
sbr/fold.h| 7 +
test/mhbuild/test-mhbuild | 53
uip/mhoutsbr.c| 7 -
5 files changed, 131 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 ..d80cfe2d
--- /dev/null
+++ b/sbr/fold.c
@@ -0,0 +1,63 @@
+/* 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 "sbr/charstring.h"
+#include "fold.h"
+
+void
+fold(charstring_t dst, size_t namelen, const char *restrict body)
+{
+ const char *restrict body_next;
+ const char *restrict wsp;
+ const char *restrict wsp_next;
+ const bool crlf = strchr(body, '\r');
+ charstring_clear(dst);
+ namelen++;
+
+ while (*body) {
+ body_next = strchr(body, '\n');
+ if ((unsigned long) (body_next - body) <= MAXTEXTPERLN - namelen) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ namelen = 0;
+ body = body_next + 1;
+ continue;
+ }
+ wsp = body;
+ while (namelen == 0 && (*wsp == ' ' || *wsp == '\t')) {
+ wsp++;
+ }
+ wsp = wsp_next = strpbrk(wsp, " \t");
+
+ /* if now whitespace is in the current line just print the curret line as is */
+ if (!wsp_next || wsp_next > body_next) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ 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;
+ }
+ }
+
+ charstring_push_back_chars(dst, body, wsp - body, wsp - body);
+ if (crlf) {
+ charstring_push_back(dst, '\r');
+ }
+ charstring_push_back(dst, '\n');
+ namelen = 0;
+ body = wsp;
+ }
+}
diff --git a/sbr/fold.h b/sbr/fold.h
new file mode 100644
index ..8d0fd6d7
--- /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(charstring_t dst, size_t namelen, 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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+
+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..6f80bafc 100644
---
Re: mhbuild and long header fields
Philipp wrote: > Good to hear. Do you want a format-patch of the final version? Sure, that would be great. Or, all I need is the commit message. I just verified that I'm using your last code submission with no changes. > > One issue to resolve first: the References header field at the end of this > > message ends with: > > <16f13de3df80f62de768420a3cc3a2fe.phil...@bureaucracy.de> > > <14298-1693614066.188834@mIBq.BRD6.6M0V> > > > > There's always an extra space in front of the Message-ID that was appended > > last. Is that a relic of repl? > > That's strange and I can't reproduce this. Also I can't find code which > would explain this in repl and the default configuration. It took me a while to find it: References is updated by replcomps. For unknown reasons, mine had an extra space. I fixed it so all is good now. > No, fold() should not change the body of the field. Depending on the > context multible white space characters might have a meaning or are used > to increase readability without a MUA. Agreed. Thanks! David
Re: mhbuild and long header fields
[2023-09-16 12:36] David Levine > Phillipp, I've been using your header field folding for a few weeks and I > really like it. I think that it should be committed, it's big step forward. Good to hear. Do you want a format-patch of the final version? > One issue to resolve first: the References header field at the end of this > message ends with: > <16f13de3df80f62de768420a3cc3a2fe.phil...@bureaucracy.de> > <14298-1693614066.188834@mIBq.BRD6.6M0V> > > There's always an extra space in front of the Message-ID that was appended > last. Is that a relic of repl? That's strange and I can't reproduce this. Also I can't find code which would explain this in repl and the default configuration. Maybe in the convert interface of mhbuild? > Or, should the folding code collapse multiple white space characters at the > beginning of a line? No, fold() should not change the body of the field. Depending on the context multible white space characters might have a meaning or are used to increase readability without a MUA. Philipp
Re: mhbuild and long header fields
Phillipp, I've been using your header field folding for a few weeks and I really like it. I think that it should be committed, it's big step forward. One issue to resolve first: the References header field at the end of this message ends with: <16f13de3df80f62de768420a3cc3a2fe.phil...@bureaucracy.de> <14298-1693614066.188834@mIBq.BRD6.6M0V> There's always an extra space in front of the Message-ID that was appended last. Is that a relic of repl? Or, should the folding code collapse multiple white space characters at the beginning of a line? David
Re: mhbuild and long header fields
Philipp wrote: > This was to vague asked by me. I was woundering about the API. Should > fold construct a string of the complete field or only the body. I think the wwy you have it currently implemented is fine: only the body but it accounts for the length of the name. David
Re: mhbuild and long header fields
[2023-08-31 22:14] David Levine > Philipp wrote: > > > I have a version with charstring_t attached. I'm unsure if it's better > > to only fold the body or include the field name. The version attached > > only fold the body. > > RFC 5322 §2.2.3 only mentions folding the body. And field names > can't have whitespace. So I think that only a body can be folded. This was to vague asked by me. I was woundering about the API. Should fold construct a string of the complete field or only the body. Philipp
Re: mhbuild and long header fields
Philipp wrote: > I have a version with charstring_t attached. I'm unsure if it's better > to only fold the body or include the field name. The version attached > only fold the body. RFC 5322 §2.2.3 only mentions folding the body. And field names can't have whitespace. So I think that only a body can be folded. I'll exercise your patch for the next week or so. Unfortunately, I'll be off line during much of that time so might not respond quickly. Thank you for doing this. As the References field in this message shows, it's badly needed. David
Re: mhbuild and long header fields
[2023-08-28 11:53] Philipp
> [2023-08-27 22:00] David Levine
> > Philipp wrote:
> >
> > > [2023-08-27 09:29] David Levine
> > > >
> > > > 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.
> >
> > I don't think that allocating memory is a drawback.
>
> I first wrote a long text about the missing string abstraction, then
> I found charstring_t. I'll adjust this a few days.
I have a version with charstring_t attached. I'm unsure if it's better
to only fold the body or include the field name. The version attached
only fold the body.
Philipp
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 ..d80cfe2d
--- /dev/null
+++ b/sbr/fold.c
@@ -0,0 +1,63 @@
+/* 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 "sbr/charstring.h"
+#include "fold.h"
+
+void
+fold(charstring_t dst, size_t namelen, const char *restrict body)
+{
+ const char *restrict body_next;
+ const char *restrict wsp;
+ const char *restrict wsp_next;
+ const bool crlf = strchr(body, '\r');
+ charstring_clear(dst);
+ namelen++;
+
+ while (*body) {
+ body_next = strchr(body, '\n');
+ if ((unsigned long) (body_next - body) <= MAXTEXTPERLN - namelen) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ namelen = 0;
+ body = body_next + 1;
+ continue;
+ }
+ wsp = body;
+ while (namelen == 0 && (*wsp == ' ' || *wsp == '\t')) {
+ wsp++;
+ }
+ wsp = wsp_next = strpbrk(wsp, " \t");
+
+ /* if now whitespace is in the current line just print the curret line as is */
+ if (!wsp_next || wsp_next > body_next) {
+ charstring_push_back_chars(dst, body, body_next - body + 1, body_next - body + 1);
+ 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;
+ }
+ }
+
+ charstring_push_back_chars(dst, body, wsp - body, wsp - body);
+ if (crlf) {
+ charstring_push_back(dst, '\r');
+ }
+ charstring_push_back(dst, '\n');
+ namelen = 0;
+ body = wsp;
+ }
+}
diff --git a/sbr/fold.h b/sbr/fold.h
new file mode 100644
index ..8d0fd6d7
--- /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(charstring_t dst, size_t namelen, 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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+
+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..6f80bafc 100644
--- a/uip/mhoutsbr.c
+++ b/uip/mhoutsbr.c
@@ -17,6 +17,8 @@
#include "h/mhparse.h"
#include "mhoutsbr.h"
#include
Re: mhbuild and long header fields
Philipp wrote: > One question about charstring_t: Why does charstring_push_back_chars() > not use memcpy? It has the side effect of advancing cur: s->cur++. David
Re: mhbuild and long header fields
Philipp wrote: > Ah, I forgott the early continue. The attached version sould work. It does. I'm sending this message using it. David
Re: mhbuild and long header fields
[2023-08-27 22:00] David Levine > Philipp wrote: > > > [2023-08-27 09:29] David Levine > > > > > > 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. > > I don't think that allocating memory is a drawback. I first wrote a long text about the missing string abstraction, then I found charstring_t. I'll adjust this a few days. One question about charstring_t: Why does charstring_push_back_chars() not use memcpy? Philipp
Re: mhbuild and long header fields
[2023-08-27 22:00] David Levine
> > 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?
>
> The patch didn't fix it. The attached sample shows the added line
> when run through mhfixmsg:
>
> $ uip/mhfixmsg -file fold_adds_blank_line.txt -out -
> From: sen...@example.com
> DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=example.com;
>
>
> b=0123456789012345678901234567890123456789012345678901234567890123456789012345
Ah, I forgott the early continue. The attached version sould work.
Philipp
From fe971fea754f79a1dfe874e64ad157f2b294076f Mon Sep 17 00:00:00 2001
From: Philipp Takacs
Date: Fri, 25 Aug 2023 09:29:42 +0200
Subject: [PATCH] mhbuild implement header folding
---
Makefile.am | 2 ++
sbr/fold.c| 63 +++
sbr/fold.h| 7 +
test/mhbuild/test-mhbuild | 53
uip/mhoutsbr.c| 3 +-
5 files changed, 127 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 ..95553782
--- /dev/null
+++ b/sbr/fold.c
@@ -0,0 +1,63 @@
+/* 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
+
+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 = body;
+ while (namelen == 0 && (*wsp == ' ' || *wsp == '\t')) {
+ wsp++;
+ }
+ wsp = wsp_next = strpbrk(wsp, " \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;
+ body = wsp;
+ }
+}
diff --git a/sbr/fold.h b/sbr/fold.h
new file mode 100644
index ..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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+
+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
Re: mhbuild and long header fields
Philipp wrote: > [2023-08-27 09:29] David Levine > > > > 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. I don't think that allocating memory is a drawback. > 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. Good point. Yes, mhfixmsg should fold also. > 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? The patch didn't fix it. The attached sample shows the added line when run through mhfixmsg: $ uip/mhfixmsg -file fold_adds_blank_line.txt -out - From: sen...@example.com DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=example.com; b=0123456789012345678901234567890123456789012345678901234567890123456789012345 To: recipi...@example.com Date: Sun, 27 Aug 2023 23:00:00 + test If the length of that long line is reduced by 1, the blank line is not created. David --- Begin Message --- test --- End Message ---
Re: mhbuild and long header fields
[2023-08-27 09:29] David Levine
> 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
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 ..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
+
+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 ..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
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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`"
Re: mhbuild and long header fields
I wrote: > 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. Alternatively, maybe the behavior of fold() could be changed. The bad behavior is caused by occasional insertion of a blank line. David
Re: mhbuild and long header fields
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(). And a very minor change would be to add the year to the copyright notice. 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. David
Re: mhbuild and long header fields
[2023-08-26 17:42] David Levine > Philipp wrote: > I agree. I'm sending this message using your patch. I'd like to > exercise it for a few days before committing. No problem, take your time. Do you only test the patch or have you looked at the code? It would be nice to get some feedback on the implementation. Philipp
Re: mhbuild and long header fields
Philipp wrote: > That said, my code detects lines without WSP (and are longer then 78 > bytes). Some rfc 2047 encoding could be added there. But first the > folding should be implemented. I agree. I'm sending this message using your patch. I'd like to exercise it for a few days before committing. Thank you for your patch! David
Re: mhbuild and long header fields
[2023-08-25 23:46] Steffen Nurpmeso
> Steffen Nurpmeso wrote in
> <20230825213130.xthnk%stef...@sdaoden.eu>:
> |Philipp wrote in
> | <93c190b84a23e620d07b9504f619b1f2.phil...@bureaucracy.de>:
> ||[2023-08-24 21:07] Philipp Takacs
> ||> [2023-08-23 14:29] Philipp
> ||>> [2023-08-20 22:14] David Levine
> ||>>> Ken Hornstein wrote:
> | ...
> ||I have tested this and fixed some bugs. A commit with a test is attached.
> |
> |I have not looked at your code (for real), but .. i have a test
> |for the MUA i maintain like
> |
> | i=$(awk 'BEGIN{for(i=0; i<92; ++i) printf "0123456789_"}')
> |
> |and then i use "$i" as a subject.
> |The problem with RFC 5322 is that artificial spaces may have to be
> |introduced to break a line without whitespace. (That is, you have
> |to break somewhere, and have to start the next line with
> |a whitespace, but that is included then, modifying the original
> |data.)
>
> That is: you should switch to MIME RFC 2047 then for the purpose
> of uniting maybe even. Or however nmh wants to deal with this.
Thanks for your feedback, but I don't think general encoding every field
with a line longer the 78 (998) bytes is the way to go. Because fields
which are to long are uncommen. Also fields which tend to be to long have
most of the time contain white space[0]. So folding is possible.
That said, my code detects lines without WSP (and are longer then 78
bytes). Some rfc 2047 encoding could be added there. But first the
folding should be implemented.
Philipp
[0] I speak of fields you would include raw in a draft file, not binary
data like a dkim signature. Such data is mostly direct rfc 2047 encoded.
Re: mhbuild and long header fields
Steffen Nurpmeso wrote in
<20230825213130.xthnk%stef...@sdaoden.eu>:
|Philipp wrote in
| <93c190b84a23e620d07b9504f619b1f2.phil...@bureaucracy.de>:
||[2023-08-24 21:07] Philipp Takacs
||> [2023-08-23 14:29] Philipp
||>> [2023-08-20 22:14] David Levine
||>>> Ken Hornstein wrote:
| ...
||I have tested this and fixed some bugs. A commit with a test is attached.
|
|I have not looked at your code (for real), but .. i have a test
|for the MUA i maintain like
|
| i=$(awk 'BEGIN{for(i=0; i<92; ++i) printf "0123456789_"}')
|
|and then i use "$i" as a subject.
|The problem with RFC 5322 is that artificial spaces may have to be
|introduced to break a line without whitespace. (That is, you have
|to break somewhere, and have to start the next line with
|a whitespace, but that is included then, modifying the original
|data.)
That is: you should switch to MIME RFC 2047 then for the purpose
of uniting maybe even. Or however nmh wants to deal with this.
--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
Re: mhbuild and long header fields
Hello.
Philipp wrote in
<93c190b84a23e620d07b9504f619b1f2.phil...@bureaucracy.de>:
|[2023-08-24 21:07] Philipp Takacs
|> [2023-08-23 14:29] Philipp
|>> [2023-08-20 22:14] David Levine
|>>> Ken Hornstein wrote:
...
|I have tested this and fixed some bugs. A commit with a test is attached.
I have not looked at your code (for real), but .. i have a test
for the MUA i maintain like
i=$(awk 'BEGIN{for(i=0; i<92; ++i) printf "0123456789_"}')
and then i use "$i" as a subject.
The problem with RFC 5322 is that artificial spaces may have to be
introduced to break a line without whitespace. (That is, you have
to break somewhere, and have to start the next line with
a whitespace, but that is included then, modifying the original
data.)
--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
Re: mhbuild and long header fields
[2023-08-24 21:07] Philipp Takacs
> [2023-08-23 14:29] Philipp
> > [2023-08-20 22:14] David Levine
> > > Ken Hornstein wrote:
> > >
> > > > [Phillip wrote:]
> > > > >Or in output_headers(). I'm not sure if there an extra options would be
> > > > >required.
> > > >
> > > > That is one option. Another is that repl(1) could do a better job; I
> > > > suppose that is the fault of mhl.
> > >
> > > [...]
> > >
> > > I think that output_headers() is the right place. It's the only
> > > place where header fields are output.
> > >
> > > [...]
> >
> > [...]
> >
> > I'll try to implement a corrent fold funktion on the weekend.
>
> I found a bit time to implement this today. Just the fold() fuction
> not the integration in output_headers().
I have tested this and fixed some bugs. A commit with a test is attached.
Philipp
From 2342446107a415e35f7895a2a1fe2d4833420e80 Mon Sep 17 00:00:00 2001
From: Philipp Takacs
Date: Fri, 25 Aug 2023 09:29:42 +0200
Subject: [PATCH] mhbuild implement header folding
---
Makefile.am | 2 ++
sbr/fold.c| 60 +++
sbr/fold.h| 7 +
test/mhbuild/test-mhbuild | 26 +
uip/mhoutsbr.c| 3 +-
5 files changed, 97 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 ..023deed8
--- /dev/null
+++ b/sbr/fold.c
@@ -0,0 +1,60 @@
+/* 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
+
+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);
+ }
+ fputc(*wsp, stream);
+ namelen = 1;
+ body = wsp + 1;
+ }
+}
diff --git a/sbr/fold.h b/sbr/fold.h
new file mode 100644
index ..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..464ad309 100755
--- a/test/mhbuild/test-mhbuild
+++ b/test/mhbuild/test-mhbuild
@@ -221,5 +221,31 @@ run_test "mhbuild $f" \
check "$f" "$expected"
+start_test "Checking for correct header folding"
+
+cat >"`mhpath new`" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+This is a test
+E
+
+cat > "$expected" <<\E
+From: Somebody
+To: Nobody
+Subject: Test message
+References:
+
+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) {
-
Re: mhbuild and long header fields
[2023-08-23 14:29] Philipp
> [2023-08-20 22:14] David Levine
> > Ken Hornstein wrote:
> >
> > > [Phillip wrote:]
> > > >Or in output_headers(). I'm not sure if there an extra options would be
> > > >required.
> > >
> > > That is one option. Another is that repl(1) could do a better job; I
> > > suppose that is the fault of mhl.
> >
> > [...]
> >
> > I think that output_headers() is the right place. It's the only
> > place where header fields are output.
> >
> > [...]
>
> [...]
>
> I'll try to implement a corrent fold funktion on the weekend.
I found a bit time to implement this today. Just the fold() fuction
not the integration in output_headers().
Philipp
> Philipp
>
> [0] I don't know if this is an issue in the context of ical
>
> > David
>
#include "h/mh.h"
#include "h/mime.h"
#include
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);
if (namelen >= MAXTEXTPERLN) {
if (crlf) {
fputs("\r\n ", stream);
} else {
fputs("\n ", stream);
}
namelen = 0;
}
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 = strpbrk(body, " \t");
/* if now whitespace is in the current line just print the curret line as is */
if (!wsp || wsp > body_next) {
fwrite(body, 1, body_next - body + 1, stream);
namelen = 0;
body = body_next + 1;
continue;
}
while ((unsigned long)(wsp - body) <= MAXTEXTPERLN - namelen) {
wsp_next = strpbrk(wsp, " \t");
if (!wsp_next) {
break;
}
if (wsp_next > body_next) {
break;
}
wsp = wsp_next;
}
fwrite(body, 1, wsp - body, stream);
if (crlf) {
fputs("\r\n", stream);
} else {
fputs("\n", stream);
}
fputc(*wsp, stream);
namelen = 1;
body = wsp + 1;
}
}
Re: mhbuild and long header fields
Philipp wrote: > I have had a quick look at fold() and just calling fold() would produce > incorrect results. fold() just insert '\n ' after 76 characters[0]. But > to correct fold a field you must insert a '\n' befor a whitespace. > > I'll try to implement a corrent fold funktion on the weekend. > > Philipp > > [0] I don't know if this is an issue in the context of ical Good catch! Yes, RFC 5322 folding for messages and RFC 5545 folding for ical are different. Thank you. This has bothered me for a long time. David
Re: mhbuild and long header fields
[2023-08-20 22:14] David Levine > Ken Hornstein wrote: > > > [Phillip wrote:] > > >Or in output_headers(). I'm not sure if there an extra options would be > > >required. > > > > That is one option. Another is that repl(1) could do a better job; I > > suppose that is the fault of mhl. > > mhl is used for display. And a user can substitute their own > filter for it. We need to fold header fields when producing a > message. > > I think that output_headers() is the right place. It's the only > place where header fields are output. > > There is a fold() function in sbr/mhical.c, maybe it could be > moved to sbr and called from output_headers() for message and > content part headers. Those shouldn't need multibyte character > support, though that wouldn't hurt. I have had a quick look at fold() and just calling fold() would produce incorrect results. fold() just insert '\n ' after 76 characters[0]. But to correct fold a field you must insert a '\n' befor a whitespace. I'll try to implement a corrent fold funktion on the weekend. Philipp [0] I don't know if this is an issue in the context of ical > David
Re: mhbuild and long header fields
Ken Hornstein wrote: > [Phillip wrote:] > >Or in output_headers(). I'm not sure if there an extra options would be > >required. > > That is one option. Another is that repl(1) could do a better job; I > suppose that is the fault of mhl. mhl is used for display. And a user can substitute their own filter for it. We need to fold header fields when producing a message. I think that output_headers() is the right place. It's the only place where header fields are output. There is a fold() function in sbr/mhical.c, maybe it could be moved to sbr and called from output_headers() for message and content part headers. Those shouldn't need multibyte character support, though that wouldn't hurt. David
Re: mhbuild and long header fields
Philipp wrote: > Thanks for pointing this out, but now I'm more confused. Whats the point > of LENERR (in contrast to FMTERR)? But this is a diffrent discussion. There isn't much point to differentiaing LENERR from FMTERR. The error messages are different when they're set, which might help the user identify the problem. After that, they are handled identically. David
Re: mhbuild and long header fields
>> Thank you for pointing that out. Header field folding does need to be >> properly implemented. It would be a great contribution if someone has >> the bandwidth. > >I have looked a bit at it. I found two places where this could be >implemented: > >Either as part of encode_rfc2047(), this function does this allready, >but only if a non ascii char is in the field body. Right, I don't think this is the right function. That is SPECIFICALLY for RFC 2047 encoding, which is actually only permitted for some headers (definitely not References). Yes, it does folding but that's really just in the context of creating RFC 2047 "phrases". >Or in output_headers(). I'm not sure if there an extra options would be >required. That is one option. Another is that repl(1) could do a better job; I suppose that is the fault of mhl. --Ken
Re: mhbuild and long header fields
[2023-08-18 21:48] David Levine
> > I have noticed that mhbuild don't implement header field folding.
> > Specialy for the References field this might cause problems, when you
> > use it to feed a mail build by mhbuild to a tool which checks the
> > line length.
>
> Thank you for pointing that out. Header field folding does need to
> be properly implemented. It would be a great contribution if someone
> has the bandwidth.
I have looked a bit at it. I found two places where this could be
implemented:
Either as part of encode_rfc2047(), this function does this allready,
but only if a non ascii char is in the field body.
Or in output_headers(). I'm not sure if there an extra options would
be required.
> > Also I don't get the code. In uip/mhbuildsbr.c:359 a LENERR is handled
> > with die(), but I can't create a testcase for this.
>
> A draft with a header field name exceeding 997 bytes in length should
> trigger LENERR. The only code that sets a LENERR is in m_getfld().
> So the draft would trigger is in any nmh command that reads a message
> header, such as scan(1):
Thanks for pointing this out, but now I'm more confused. Whats the point
of LENERR (in contrast to FMTERR)? But this is a diffrent discussion.
Philipp
> $ (printf '%.sX' {1..997}; printf ': too-long\n') | scan -file -
> scan: field name
> "XX
>
>
>
>
>
>
> XXX:"
> exceeds 997 bytes
> ??Format error (message -1) in component 1
>1 08/18/23*
>
> David
>
Re: mhbuild and long header fields
Philipp wrote:
> I have noticed that mhbuild don't implement header field folding.
> Specialy for the References field this might cause problems, when you
> use it to feed a mail build by mhbuild to a tool which checks the
> line length.
Thank you for pointing that out. Header field folding does need to
be properly implemented. It would be a great contribution if someone
has the bandwidth.
> Also I don't get the code. In uip/mhbuildsbr.c:359 a LENERR is handled
> with die(), but I can't create a testcase for this.
A draft with a header field name exceeding 997 bytes in length should
trigger LENERR. The only code that sets a LENERR is in m_getfld().
So the draft would trigger is in any nmh command that reads a message
header, such as scan(1):
$ (printf '%.sX' {1..997}; printf ': too-long\n') | scan -file -
scan: field name
"X:"
exceeds 997 bytes
??Format error (message -1) in component 1
1 08/18/23*
David
mhbuild and long header fields
Hi I have noticed that mhbuild don't implement header field folding. Specialy for the References field this might cause problems, when you use it to feed a mail build by mhbuild to a tool which checks the line length. Also I don't get the code. In uip/mhbuildsbr.c:359 a LENERR is handled with die(), but I can't create a testcase for this. Philipp
