count single \r or \n as \r\n while checking line length against MAXHEADERSIZE

[Florian Schlichting]

Hi Julien,

On Tue, Aug 03, 2010 at 10:31:07PM +0200, Julien ÉLIE wrote:
> >I think the question should be, do we really need to reject articles
> >whose header line length exceeds 998 bytes?
> 
> We could accept them.
> 
> RFC 5536:
> 
>   o  Compliant software MUST NOT generate (but MAY accept) header field
>      lines of more than 998 octets.
> 
> 
> 
> >My patch made innd a little more permissive to accommodate for existing
> >articles created by broken clients. If innd really can handle header and
> >body lines of arbitrary length, we might consider dropping that kind of
> >check altogether (for innd, not for nnrpd of course). But since there
> >doesn't seem to be an actual need for this, we might rather leave things
> >as they are and thus prevent the eventual propagation of extraordinarily
> >broken articles and the bugs they might trigger in less robust servers
> >and clients.
> 
> I also agree with you.  Let's keep the check for the time being.

I'm having second thoughts on this issue. Today I noticed that 20-30
articles get rejected every day due to References: headers longer than
998 bytes (but almost always shorter than 1024 bytes) - current innd has
become more restrictive than most software in productive use! That's not good.
Perhaps we should in fact abolish the check or 
test against a larger size (BIG_BUFFER?) if we don't want to accept
anything?

Unfortunately, nnrpd currently relies on innd to check header line
length. So I've tried to implement the check in nnrpd, but I think the
error handling looks ugly...

What do you think?

Florian


--- a/nnrpd/post.c
+++ b/nnrpd/post.c
@@ -164,12 +164,21 @@ TrimSpaces(char *p)
 static char *
 NextHeader(char *p)
 {
-    for ( ; (p = strchr(p, '\n')) != NULL; p++) {
+    char *q;
+
+    for (q = p; (p = strchr(p, '\n')) != NULL; p++) {
+       if (p - q > MAXARTLINELENGTH) {
+           strlcpy(Error, "Header line too long",
+                   sizeof(Error));
+           return NULL;
+       }
        if (ISWHITE(p[1]))
            continue;
        *p = '\0';
        return p + 1;
     }
+    strlcpy(Error, "Article has no body -- just headers",
+           sizeof(Error));
     return NULL;
 }
 
@@ -226,8 +235,7 @@ StripOffHeaders(char *article)
 
        /* Get start of next header; if it's a blank line, we hit the end. */
        if ((p = NextHeader(p)) == NULL) {
-           strlcpy(Error, "Article has no body -- just headers",
-                    sizeof(Error));
+           /* Error set in NextHeader() */
            return NULL;
        }
        if (*p == '\n')


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5557 bytes
Desc: not available
URL: <https://lists.isc.org/pipermail/inn-workers/attachments/20100915/50dc5abf/attachment.bin>