count single \r or \n as \r\n while checking line length againstMAXHEADERSIZE

Julien ÉLIE julien at trigofacile.com
Thu Jun 24 17:03:51 UTC 2010


Hi Florian,

> It appears that some people like to grace their articles with Face: or
> X-Face: headers of typically 4-6k binary data. While usually put into
> nice continuation lines of around 80 characters, a few clients fail to
> create proper line breaks for this header, inserting only a single \r or
> \n instead. The nnrpd in inn2.3 doesn't reject or correct these mistakes
> for non-system headers, and offending articles are still "in the wild".
>
> innd currently counts, logs and ignores such imperfect line breaks.
> Alas, when checking for the maximum allowable 'physical' line length,
> single \r or \n are not recognized, thus making the whole 'virtual'
> (continued) header appear as a single line, easily exceeding
> MAXHEADERSIZE and thus causing the article to be rejected.
>
> This patch causes both CRwithoutLF (\r) and LFwithoutCR (\n) to be
> considered the same as CRLF (\r\n) for purposes of checking allowable
> header line length. While I have been running with this patch for
> several months without problems, I wonder if there could be another
> place where imperfectly broken virtual lines might cause a problem?

I don't see where it could cause a problem.  I therefore think your patch
is right.
Moreover, since INN 2.5.2, XOVER and XHDR have been handling lone '\r' and
'\n' characters (so I believe INN 2.3 does not give the expected answer
with such X-Face: header fields, failing to transform that character into
a space).

Incidentally, I see that your patch fixes the issue for headers.
But we may have the same problem with the body.  What if the lines end
with a single '\r' or a single '\n'?  Why shouldn't it be treated the same
way as in headers?



> NB: I believe the only place where MAXHEADERSIZE describes allowable
> line length is here in ARTparseheader, so I think it could perhaps be
> replaced by a check against 1000 in order to achieve the fix mentioned
> in include/inn/options.h:118?

That is to say using another variable for the other calls to MAXHEADERSIZE?
We have SMBUF (256 bytes) and BIG_BUFFER (8192 bytes).  We could create
MED_BUFFER (1024 bytes) for a medium buffer...

And change MAXHEADERSIZE to 1000?  (I checked that it is really 1000, and
not 998 with '\r\n' -- they are currently included in the count.)
I believe we could also change the name to MAXARTLINESIZE because it
is otherwise a bit confusing.  The size of a header field is unlimited.
And the size also applies to the lines of the body.

Is it what you meant?

-- 
Julien ÉLIE

« Un sourire coûte moins cher que l'électricité
  mais donne autant de lumière. » (Abbé Pierre) 




More information about the inn-workers mailing list