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

[Julien ÉLIE]

Hi Matija,

> oh, that would be acceptable if worst case would still be just an lost
> article which would otherwise be lost anyway.  (at worst, then I would be
> exactly where I am now regarding lost articles; and there is a chance I
> might be better off, right? And my server wouldn't get throttled all the
> time which is even more important)

I totally understand.


> But what I'm afraid is what if one of those breakages could lead (for
> example) to makehistory or expire barfing out in the middle of run when it
> hits such article and leaving the rest of messages unprocessed

Don't worry; makehistory and expire will just ignore the articles and go on
with the following one.


> or crashing nnrpd if such article is opened or similar.
> That would be much much bigger problem for me.

Also do not worry with that.


> Well, it does bother me to lose those dozen or two articles a day, but it
> bothers me much less than having server throttled at random periods of day
> and night (in *addition* to losing those articles)

And I think you will like this suggested patch for INN 2.5.2:


--- innd/art.c	(révision 9364)
+++ innd/art.c	(copie de travail)
@@ -694,8 +694,13 @@
   } else {
     /* We need to remove leading and trailing spaces for
      * message-IDs; otherwise, history hashes may not be
-     * correctly computed. */
-    if (i == HDR__MESSAGE_ID || i == HDR__SUPERSEDES) {
+     * correctly computed.
+     * We also do it for Xref: header fields because
+     * a few parts of the code currently assume that no
+     * leading whitespace exists for this header (when
+     * parsing it to find the newsgroups in which the article
+     * is stored). */
+    if (i == HDR__MESSAGE_ID || i == HDR__SUPERSEDES || i == HDR__XREF) {
       for (p = colon + 1 ; (p < header + size - 2) &&
            (ISWHITE(*p)) ; p++);
       hc->Value = p;



It now reminds me that detecting and understanding what was going on
with message-IDs was a though issue two years ago:
    http://inn.eyrie.org/trac/changeset/8437/trunk/innd/art.c
    http://inn.eyrie.org/trac/changeset/8716/trunk/innd/art.c


This patch for Xref: header field fixes the regression introduced
in INN 2.5.0.  The behaviour for the Xref: header field will be
the same as it was in INN 2.4.x.

I think no other fields need being specially parsed.  NGsplit() for the
Newsgroups: header field already strips leading whitespaces.  (It is used
when storeonxref is set to false.)
Too bad I had not thought about Xref: when fixing Message-ID: and Supersedes:
headers.


> I think I'll just wait for your fix; and if the emergency forces me to
> put that 2.5.2 xrefslave server in production (hopefully not), I think I'll
> just setup a cron job to restart innd at strategic times dozen times a day
> and hope it'll hold water until more permanent fix is available.

I hope the above proposal of "fix" is fine for your needs.  INN 2.5.2 will
work fine with it in xrefslave mode.
I have just tested it.  Overview data is properly generated for the articles
with leading whitespace.  (These spaces are stripped by innd so they are absent
from the overview data -- not RFC 3977-compliant, because OVER should show these
spaces, but that's not a big deal.  It will be fixed later.  Better have a
server that does not throttle itself!)

I also believe it is the safest fix to do right now, just before an upcoming
release of INN 2.5.3.  Properly testing extensive changes in the Xref: parsing
code in an RFC 5536 way need time.  (And is no longer a blocking issue, now that
the legacy parsing of Xref: is used again.)

-- 
Julien ÉLIE

« – Ils transportent une arme secrète dans un tonneau !
  – La cervoise tiède !!!
  – Non, ça c'est une arme connue. » (Astérix)