cant store article: bogus Xref: header in INN 2.5 ?

[Julien ÉLIE]

Hi Russ,

>> It would be better to find out why two spaces are inserted, instead of
>> modifying CrackXref to handle that weirdness.
> 
> It would be nice to fix both, but in general we shouldn't be this picky
> about whitespace.  It's pretty easy to skip over leading whitespace in
> that header, and in general RFC 5322 headers are not supposed to care
> about whitespace.  The RFC 5536 grammar also explicitly allows leading
> whitespace:
> 
>     The Xref header field indicates where an article was filed by the last
>     news server to process it.  User agents often use the information in
>     the Xref header field to avoid multiple processing of crossposted
>     articles.
> 
>     xref            =  "Xref:" SP *WSP server-name
>                        1*( FWS location ) *WSP CRLF

Yes, you're totally right.  We should fix how the Xref: header is parsed
by tradspool.
A slave INN server could be fed by another news server so we cannot assume any
restrictive syntax for the Xref: header field.
Besides, even when the upstream server is INN, there is an issue with the Xref:
parsing because of folding whitespace that is not taken into account (innd
properly folds the Xref: header it generates in ARTassignnumbers).




Suggested fix for INN 2.5.3:


--- storage/tradspool/tradspool.c	(r?vision 9364)
+++ storage/tradspool/tradspool.c	(copie de travail)
@@ -578,18 +578,21 @@
     xrefsize = 5;
     xrefs = xmalloc(xrefsize * sizeof(char *));
 
-    /* no path element should exist, nor heading white spaces exist */
     p = xref;
+
+    /* Skip leading whitespaces.  As no path element should exist,
+     * p will point to the beginning of the server name. */
+    for ( ; ISWHITE(*p) ; p++);
+
     while (true) {
 	/* check for EOL */
-	/* shouldn't ever hit null w/o hitting a \r\n first, but best to be paranoid */
-	if (*p == '\n' || *p == '\r' || *p == 0) {
+	if (*p == '\n' || *p == '\r' || *p == '\0') {
 	    /* hit EOL, return. */
 	    *lenp = len;
 	    return xrefs;
 	}
 	/* skip to next space or EOL */
-	for (q=p; *q && *q != ' ' && *q != '\n' && *q != '\r' ; ++q) ;
+	for (q=p; *q && (! ISWHITE(*q)) && *q != '\n' && *q != '\r' ; ++q) ;
 
         xrefs[len] = xstrndup(p, q - p);
 
@@ -599,9 +602,8 @@
             xrefs = xrealloc(xrefs, xrefsize * sizeof(char *));
 	}
 
- 	p = q;
-	/* skip spaces */
-	for ( ; *p == ' ' ; p++) ;
+	/* Skip whitespaces, possibly folded. */
+        p = (char *) skip_fws(q);
     }
 }



That is to say, the whole function would be:


/*
**  Crack an Xref: line apart into separate strings, each of the form "ng:artnum".
**  Return in "num" the number of newsgroups found.
*/
static char **
CrackXref(char *xref, unsigned int *lenp) {
    char *p;
    char **xrefs;
    char *q;
    unsigned int len, xrefsize;

    len = 0;
    xrefsize = 5;
    xrefs = xmalloc(xrefsize * sizeof(char *));

    p = xref;

    /* Skip leading whitespaces.  As no path element should exist,
     * p will point to the beginning of the server name. */
    for ( ; ISWHITE(*p) ; p++);

    while (true) {
        /* check for EOL */
        if (*p == '\n' || *p == '\r' || *p == '\0') {
            /* hit EOL, return. */
            *lenp = len;
            return xrefs;
        }
        /* skip to next space or EOL */
        for (q=p; *q && (! ISWHITE(*q)) && *q != '\n' && *q != '\r' ; ++q) ;

        xrefs[len] = xstrndup(p, q - p);

        if (++len == xrefsize) {
            /* grow xrefs if needed. */
            xrefsize *= 2;
            xrefs = xrealloc(xrefs, xrefsize * sizeof(char *));
        }

        /* Skip whitespaces, possibly folded. */
        p = (char *) skip_fws(q);
    }
}





It seems to work fine.  (I successfully stored articles posted to 1, 2
and 3 newsgroups in tradspool.)

-- 
Julien ÉLIE

« Un voyage de mille lieues commence toujours par un premier pas. »
  (Lao Zi)