Re: [Barry-devel] [patch] message header patch

[Chris F. <cd...@fo...>]

On Sun, Jun 24, 2007 at 06:43:07AM -0600, Brian Edginton wrote:
> So, after crashing my hard drive and recreating all this info from notes 
> (backup!, backup!, backup!), here is a patch to the message databases to 
> parse out some of the information stored in the header, i.e. flags, priority, 
> time and date.

Huge thanks for documenting and coding that MessageRecord header!

I've applied the patch verbatim.  There's a couple of things of note:

One thing I noticed that was missing was a proper size check in
the new ParseHeader() member functions.  You're doing a MAKE_RECORD()
without checking that there is enough data in the buffer.  There
should be a check somewhere, like

	if( data.GetSize() < (offset + sizeof(MessageRecord)) )
		// not enough data
		return;

We might even want to throw an exception here actually.  That is what
protocol.cc:CheckSize() does, and that is used liberally through
the parsing code.  I'd recommend that.

If you need more detail than just the simple size of MessageRecord,
feel free to add #defines.  Examples are in protostructs.h.
Sometimes you only want to check the size of the header, and not the
data, etc.  Make sure the size check is accurate, so you can parse
the header if you only get the header.


Also, in the new DayToDate() function:

time_t DayToDate( unsigned short Day )
{
        struct tm *now, then;
        time_t t = time( NULL );

        now = localtime( &t ); // need this to get year
        // set to Jan 1 midnight, this year;
        then.tm_sec = 0;
        then.tm_min = 0;
        then.tm_hour = 0;
        then.tm_mday = 0;
        then.tm_mon = 0;
        then.tm_year = now->tm_year;
        then.tm_isdst = -1;
        t = mktime(&then);
        t -= 60*60;                     // need to subract an hour
        t += Day * 24 * 60 * 60;        // Add the day converted to seconds

        return t;
}

I don't understand why you need to subtract an hour.

It also bothers me that we are relying so heavily on localtime
for this.  I'd be very interested if we can avoid that, and use exact
time in UTC, or be able to convert it somehow.


And finally, all usage of multi-byte structure fields need to be
fed through the endian conversion functions, btoh() and friends.
This seems to be missing.


> In protostructs.h I've commented on most of the fields as I've been able to 
> parse them out. There are still some fields that appear interesting, but most 
> of the important data has been discovered and I hope interpreted correctly.

Thanks!


> Left on the TODO list:
>  Year information from the date field - I'll set up a test configuration that 
> will let me adjust the year on emails, or wait 6 months for the year to 
> change - whichever comes first.

Can you send yourself an email with a forged Date: header?
I can try sending you one if you need.


>  Time is 'close' but I'm not sure that getting any finer resolution is worth 
> any more effort. The way to effect this is to go to better resolution than 
> 1.77.

Pardon my timestamp-challenged brain here, but why 1.77? :-)

This would help to document the units that timeReceived is in, etc.


>  The header structure appears to be identical between Messages, Saved Email 
> Messages and PIN Messages, as are most of the functions being called. I will 
> work on some refactoring. 

Excellent.


> So here's the patch, enjoy - and yes Chris, I did 'vimify' it to check the 
> formatting ;)

Thanks for that. :-)  It helps to keep the code consistent.

- Chris