Re: mhbuild and long header fields

2023-08-27 Thread 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.

> 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 Thread Philipp
[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

2023-08-27 Thread David Levine
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: test-mhfixmsg and test-binary fails on debian stable

2023-08-27 Thread David Levine
Philipp wrote:

> [2023-08-26 17:39] David Levine 
> > Philipp wrote:
> >
> > > [2023-08-26 15:28] David Levine 
> > > >
> > > > The NUL byte is output as \x00:
> > >
> > > I found the problem: The build in printf of dash don't write a NUL.
> > > Using printf from path to generate the test and expected file works as
> > > expected.
> >
> > Thank you!  I'll update the test.
>
> An other hint for this test. Posix printf specifies an octal escape
> sequences[0]. Replacing the '\x00' with '\000' fixes the test.

Yes, much better.  I'll fix that test and test/post/test-post-basic.

Thank you!

David

> [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html



Re: mhbuild and long header fields

2023-08-27 Thread 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().

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-27 Thread Philipp
[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: test-mhfixmsg and test-binary fails on debian stable

2023-08-27 Thread Philipp
[2023-08-26 17:39] David Levine 
> Philipp wrote:
>
> > [2023-08-26 15:28] David Levine 
> > >
> > > The NUL byte is output as \x00:
> >
> > I found the problem: The build in printf of dash don't write a NUL.
> > Using printf from path to generate the test and expected file works as
> > expected.
>
> Thank you!  I'll update the test.

An other hint for this test. Posix printf specifies an octal escape
sequences[0]. Replacing the '\x00' with '\000' fixes the test.

Philipp

[0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html