Bug#533285: inn2: nnrpd crashes when retrieving entire thread

[Julien ÉLIE]

Hi Russ,

>> char *
>> overview_getheader(const struct cvector *vector, unsigned int element,
>>     const struct vector *extra)
>
> I think we want to change the element number to a const char *field and
> then move the call to overview_index() into overview_getheader().  Then,
> if the index number is within the mandatory headers, do what we do now,
> and if it's outside, walk the fields looking with strncasecmp for a
> header starting with the desired field followed by a colon and a space.

Then it will imply that we do twice the work (two calls to overview_index())
because we already use the index number in HDR/XHDR/XPAT to decide
whether to retrieve the article or to have a look inside the overview
database.
If we do "HDR Message-ID 1-1000", it will call overview_index() 1000 times
to see that "Message-ID" is a mandatory header, which was already known.
It is only useful when the header is not mandatory because its position
could change between articles.

/* In overview? */
Overview = overview_index(header, OVextra);

/* Not in overview, we have to fish headers out from the articles. */
if (Overview < 0 || IsBytes || IsLines) {
[...]
}

[...]

/* Overview search. */
while (OVsearch(handle, &artnum, &data, &len, &token, NULL)) {
[...]
  p = overview_getheader(vector, Overview, OVextra);
[...]
}

As we already know the index number, it could have been provided for
mandatory headers.  Well, it is just to optimize the function.
But maybe it is not useful to save these calls.


> I suspect :bytes and :lines are already handled somewhere else, since I
> don't see any code to handle them in overview_index(), but code could
> easily be added there if need be.

Yes, by the code above, where
  IsBytes = (strcasecmp(header, "Bytes") == 0);
  IsLines = (strcasecmp(header, "Lines") == 0);


> I suppose the one sanity check that one could add is to strlen the
> duplicated string and cap the size of the allocated string to the length
> of the original instead of blindly trusting the size_t argument.
> However, I suspect that change would only affect places where the caller
> was already doing something fairly wrong, and there it would probably
> just paper over a deeper bug (as it would have here).

You're right.  Better do nothing for xstrndup, then.

-- 
Julien ÉLIE

« -- Ce n'était pas ma question.
  -- C'était p'têt pas vot'question, oui, mais c'est ma réponse ! »
  (Georges Marchais répondant à Alain Duhamel)