API inconsistency in HttpHeader.cc?

From: Kinkie <gkinkie_at_gmail.com>
Date: Mon, 3 Oct 2011 16:37:04 +0200

Hi all,
  while working on playground (HttpHdrCc c++-ification, mostly), I
stumbled upon something which worries me a bit, and I wonder why it is
not causing issues.

HttpHeader.cc defines a few functions which add headers, I'm zoning in
onto HttpHeader::putCc as is the one I've been looking at the most.
Here it is:

void
HttpHeader::putCc(const HttpHdrCc * cc)
{
    MemBuf mb;
    Packer p;
    assert(cc);
    /* remove old directives if any */
    delById(HDR_CACHE_CONTROL);
    /* pack into mb */
    mb.init();
    packerToMemInit(&p, &mb);
    cc->packInto(&p);
    /* put */
    addEntry(new HttpHeaderEntry(HDR_CACHE_CONTROL, NULL, mb.buf));
    /* cleanup */
    packerClean(&p);
    mb.clean();
}

The problem is that addEntry is initialized from the raw storage of a
MemBuf (filled-in via packer), expecting a NULL-terminated c-string
value.
Which may very well not be the case. For instance, if noone as written
anything to the MemBuf.

A possible solution could be to mb.terminate() just before addEntry,
but that has its own problems: if the MemBuf doesn't have the space to
grow for appending the trailing \0, it will assert (see XXX on
MemBuff::terminate() ).

putContRange, putRange, putSc all share the same blueprint and,
potentially, issue.

I wonder why this is not biting us, and how to best address this. Any
ideas or suggestions?

Thanks

-- 
    /kinkie
Received on Mon Oct 03 2011 - 14:37:11 MDT

This archive was generated by hypermail 2.2.0 : Mon Oct 03 2011 - 12:00:03 MDT