jstedfast / gmime

A C/C++ MIME creation and parser library with support for S/MIME, PGP, and Unix mbox spools.
GNU Lesser General Public License v2.1
113 stars 36 forks source link

Blank line in header body invalid for netnews message #112

Closed dod38fr closed 2 years ago

dod38fr commented 2 years ago

Hi

Pan newsreader uses GMIME to format messages sent to news servers.

Unfortunatelty, the RFC5536 is somewhat more restrictive than RFC5322 regarding header format (see pan #115).

For instance this header generated by GMIME may be considered as invalid by news server because the first line of the body of the header field is empty:

X-Face: 
 3Pm8h]rEU1CZAmh[6y.keR3p<1l="@P5.slpH~`p}N0zK}n''9AxpX>\o*4skjn~M-On+Z:
 =?us-ascii?Q?G##q=5Db!7cxm=29eZ!T=285RH=3ENe=2FGIsAkCVe=3DOR=3BTRUU=3D\k~3X4wD=5F=2EQ#*V-ZgGOw0?=
 =?us-ascii?Q?=3D&#S~!6tj=22$tGFa&?=
 =?us-ascii?Q?_3=3BWnrdj=5D=3D=2C=3C=3BlS9m=2C#G=3BDzQxA69=229i=2Fq#=2E=3Bd{lfUS\$5j\85=40=2F!7|=2C=29-D=5D0|j=3C?=
 =?us-ascii?Q?e*=5By=5F~amu}tEAc=28=5B?= "5Etism-^xgy)9uh3tiZz7dg}:<d(/i)'

Is there a way to get GMIME to output header field without blank line in header body ?

All the best

jstedfast commented 2 years ago

As far as I can remember, there's no config option for this, but I would definitely accept a fix for this.

FoxMcCloud45 commented 2 years ago

Just leaving this as information, a merge request was made 5 months ago to fix the issue but was submitted on GNOME's GitLab repository.

The contributor there should most likely send a pull request here instead in the future, but I'm simply informing that there's a fix awaiting its integration.

jstedfast commented 2 years ago

Looks like that fix is for the References header only, but yes, if he submits it here, I would be able to approve & merge.

jstedfast commented 2 years ago

FWIW, it looks like the place to fix this (in the general case, for e.g. the X-Face header) would be in gmime-utils.c header_fold_tokens()

jstedfast commented 2 years ago

From a quick analysis, it looks like the fix would be something like this (untested):

diff --git a/gmime/gmime-utils.c b/gmime/gmime-utils.c
index 2ac2958c..b9a4c1cf 100644
--- a/gmime/gmime-utils.c
+++ b/gmime/gmime-utils.c
@@ -2528,7 +2528,7 @@ header_fold_tokens (GMimeFormatOptions *options, const char *field, const char *
                } else if (token->encoding != 0) {
                        n = strlen (token->charset) + 7 + (encoded ? 1 : 0);

-                       if (len + token->length + n > GMIME_FOLD_LEN) {
+                       if (token != tokens && len + token->length + n > GMIME_FOLD_LEN) {
                                if (tab != 0) {
                                        /* tabs are the perfect breaking opportunity... */
                                        g_string_insert_c (output, tab, '\n');
@@ -2559,7 +2559,7 @@ header_fold_tokens (GMimeFormatOptions *options, const char *field, const char *
                        encoded = TRUE;
                        lwsp = 0;
                        tab = 0;
-               } else if (len + token->length > GMIME_FOLD_LEN) {
+               } else if (token != tokens && len + token->length > GMIME_FOLD_LEN) {
                        if (tab != 0) {
                                /* tabs are the perfect breaking opportunity... */
                                g_string_insert_c (output, tab, '\n');
ThomasNoll commented 2 years ago

Looks like that fix is for the References header only, but yes, if he submits it here, I would be able to approve & merge.

True, it is only for the References header. That is where the problem raised for me... Pull requested ##113

FWIW, it looks like the place to fix this (in the general case, for e.g. the X-Face header) would be in gmime-utils.c header_fold_tokens()

I agree. And all the other special cases look like they are not relevant for netnews or usually short enough.

jstedfast commented 2 years ago

Has anyone had a chance to test my proposed patch? Unfortunately I'm stuck with Windows hardware and I don't think I have the space for a Linux VM due to all my installations of Visual Studio 😂

I wonder if gmime would build under WSL...

dod38fr commented 2 years ago

Hi

Sorry for the delay. Here's the X-Face header using Debian's gmime with your patch:

X-Face: 3Pm8h]rEU1CZAmh[6y.keR3p<1l="@P5.slpH~`p}N0zK}n''9AxpX>\o*4skjn~
 M-On+Z: G##q]b!7cxm)eZ!T(5RH>Ne/GIsAkCVe=OR;TRUU=\k~3X4wD_.Q#*V-ZgGOw0=
 &#S~!6tj"$tGFa& 3;Wnrdj]=,<;lS9m,#G;DzQxA69"9i/q#.;d{lfUS\$5j\85@/!7|,)
 -D]0|j<e*[y_~amu}tEAc([ "5Etism-^xgy)9uh3tiZz7dg}:<d(/i)'

The good news is that there's no blank line after X-Face:.

On the other hand, the X-Face data no longer shows the =?us-ascii? strings. I do not know if this is expected or a problem.

All the best

jstedfast commented 2 years ago

@dod38fr Thanks for testing this. Unfortunately, I don't think that's the correct behavior so I'll need to rethink this.

jstedfast commented 2 years ago

Actually, I think it's fine.