HTTP parser/management changes

I've finally committed changes to Lusca which migrate the HTTP Header management routines away from individual malloc/free calls per HTTP Header entry to (hopefully!) one malloc/free for every N. (N being tunable, of course.)

A lot of incremental preparation work was required in the HTTP parser to tidy things up enough to (hopefully!) minimise any impact on stability and functionality.

There really isn't all that much noticable improvement unless you're working on small, embedded platforms with slow CPUs. It's more of a precursor to further optimisation and reorganisation work. The cumulative effect will be worthwhile.

I've released a tarball with the work - r14674 - which also includes a handful of ATF unit tests in the test-suite/atf/ subdirectory.

I'll likely next work on reorganising some more of the FTP and HTTP protocol specific code which can be migrated out of src/ . I'd like to then spend some more time writing unit tests before (hopefully!) finishing off the IPv6 related DNS changes in preparation for more IPv6 related tomfoolery.

Unit testing using ATF

I've been toying around with the NetBSD unit testing framework called "ATF". Unlike other well-known FOSS unit testing software, ATF seems to have relatively comprehensive support for writing unit tests in C - which is quite important for a piece of C software.

In fact, it seems quite a few popular bits of FOSS software handling unit tests, embedded/API documentation, etc all support C rather poorly. I wonder why.

I've not linked the ATF stubs into the main build tree just yet. The ATF code can be found in the source tree under /test-suite/atf/ . It works enough for me to use "make check" on my iBook (yes, that's a pre-intel, PPC G4 iBook) to check that the basic changes I'm making to the HTTP parser code don't introduce new bugs.

I would love some help writing more unit tests though!

The best news is sometimes no news..

Things have been quiet on the Lusca front lately. I poked some of the users who were reporting earlier issues and asked whether said issues were fixed. All of them responded "yes".

So sometimes no news is good news. But you have to sometimes make sure.

Modifying the HTTP header parser in Lusca

I've been slowly working towards a variety of medium-term goals in Lusca. I've resisted committing various bits of partially finished work partly because they're works in progress but partially because I'm not happy with how the code fits together.

One of these areas is the HTTP header parser and management routines. Among other things, the main issues I have with the parser and management code is listed below.

• Each header entry is represented by separate strings in memory;
• Each header entry has a small, separately allocated object (HttpHeaderEntry), one per header
• Parsing the header entries uses various stdio routines to iterate over characters, and these may be implemented slower (to handle unicode/wide/UTF/locale support) than what's needed here (7-bit ASCII);
• There's some sanity checks in the header parser - specifically, duplicate content-length - which is likely better once the headers have been parsed.

I've been working on the first two items in separate branches. One converts the HttpHeaderEntry items into a single allocated array, which is grown if needed. Another takes the current String API and turns it into fully reference-counted strings. Both of these work fine for me. But shoe-horning it into the current HTTP parser code - which expects individually allocated/created HttpHeaderEntry items which it can destroy on a whim before they're considered a part of the Http Header set - is overly hackish and prone to introduce bugs.

It's taking me quite a bit of time to slowly change the HTTP parser code to be ready for the new management code. Well, it's taken me about 6 months to slowly modify it in a way that doesn't require rewriting everything and potentially changing expected behaviour and/or introduce subtle bugs.

The upshoot? Things take time, but the code hopefully will be tidier, cleaner and easier to understand. Oh, and won't include bugs.

State of the Cygwin/Windows port!

A rather nice chap has been ploughing through the source and making it work under Windows/Cygwin. I've been committing bits and pieces of his work into LUSCA_HEAD as time permits.

You can find the main port details in Issue 94.

Thanks!

Hunting down method_t bugs..

It all started with Issue 99. There was a random crash in the logging code. It looked like bug in the method handling changes which made it into Squid-2.HEAD a year or two ago. I've been patching issues in the method handling - specifically with NULL and uninitialised method pointers appearing in places - but this time the method_t pointed to junk data.

A bit of digging found that the pointer value did point to a valid method_t structure instance - but something free'd it. Hm. A little further digging found what was going on:

1. A METHOD_OTHER appeared (an RTSP method) which resulted in a new method_t being malloc'ed;
2. The pointer was copied to the request_t structure;
3. The request was processed;
4. The initial method_t pointer was freed, but the request_t method pointer still pointed to it;
5. The logging code then logged the stuff said request_t method pointer pointed to - but it was already free'd. Sometimes it'd be junk, sometimes it'd be the original contents.

The original method code (and the "known" methods) all throw around pointers - and copies of pointers - to statically allocated structures which never go away. Unfortunately this logic wasn't changed when the dynamic "other" methods appeared.

So I've been quite busy tidying up the method handling code in preparation for the change in how they're handled. LUSCA_HEAD now has some code which logs potential memory leaks when handling the dynamic methods. I'm going to see if I can come up with a way (or two) to log potential risky situations when items are dereferenced after being free'd. But hopefully I can fix the issue without introducing any further bugs.

Today's fun bug: invalid swap metadata

One of the Lusca users has issues with swap file contents being invalid. It's linked to the length of the object URL - and fixing this is unfortunately more difficult than first thought.

In essence - the size of the URL, the size of the metadata and the size of the buffers being used for reading data don't match "right". The TLV encoding code doesn't put an upper limit on the size of the metadata. The TLV decoding code doesn't enforce a maximum buffer size - it tries reading the metadata until it finds the end of said metadata. All of this unfortunately results in stupid behaviour when overly long URLs are stored.

The current maximum URL is MAX_URL - 4096 bytes. Squid-3 may have redefined this to be longer. The reason I haven't done this is because the URL is included in the swap metadata - and this is only read in in SM_PAGE_SIZE chunks - again, 4096 bytes. So if the URL is say, 4000ish or so bytes long, the total length of the encoded metadata is > 4096 bytes. This is happily written out to the swapfile.

Reading the data in however is another story. An SM_PAGE_SIZE sized buffer is created and a read is issued. The incomplete metadata is read in. There's unfortunately no check to ensure the passed in buffer actually fully contains all of the metadata - so the code happily trumps in potentially uninitialised memory. The end result is at the very least an invalid object which is eventually deleted; it could be worse. I haven't yet investigated.

In any case - I'm now going to have to somehow enforce some sensible behaviour. I'd much prefer to make the code handle arbitrary (ie, configurable arbitrary) long URLs and read in the metadata as needed - but that's a bigger issue which will take some further refactoring and redesigning to solve.

This is all another example of how code works "by magic", rather than "by intent". :)