- From: <Yusef_Badri@tertio.com>
- Date: Tue, 17 Apr 2001 12:22:05 +0100
- To: www-lib@w3.org
- Cc: yb@greybox.demon.co.uk
Hello All,
I have recently been working with version 5.3.2 of the library, and would
like to submit some bug fixes back to the list. Some of them some arcane
initialisation problems which only occur if you try shutdown and restart of
the library within the same process (NB: this still doesn't work, if any
HTTP requests have been issued), but many of them are genuinely required
bug fixes - the Cookies code in particular, didn't seem to work at all.
NB: Implementing all these fixes eliminated all Purify warnings and errors
(Platform was Solaris 7).
Patch 1
HTWWWStr:HTParseTime()
Line 421 is the start of the if statement for handling the format,
Weekday, 00-Mon-00 00:00:00 GMT. After the final strtol() call in the
body of that if statement (which is line 432, in my download), insert
this statement (NB: still within the body of the if that starts on line
421):
if ( tm.tm_year > 1900 ) {
tm.tm_year -= 1900;
}
Without this, cookies with 4-digit years in their dates (ie. all the
ones that I've seen !!) could not be parsed. This is also in line with
RFC-1123, which updated the RFC-822 year spec from 2 digits to 4, and in
any case the Unix tm_year field is defined as the number of years since
1900 - it is neither a CCYY value, nor a YY value.
Later on (around line 511), this routine makes sure that tm_year is in
the range 70-120. I recommend removing that check, since all years can
be considered equally valid (or else require 1901-2038 instead, since
that is the range mktime() can handle, but then you would have to check
months and days as well, since it can't handle the whole of those
boundary years).
Patch 2
HTAtom:HTAtom_deleteAll()
The for loop consists of a single if statement. Insert the following
statement at the end of the if statement (NB: at the end, not after it):
hash_table[i] = NULL;
Patch 3
HTLib:HTLib_setUserProfile()
Insert this statement at the start of the routine.
if (UserProfile) HTUserProfile_delete(UserProfile);
Patch 4
HTLib:HTLibTerminate()
Zero UserProfile, immediately after deleting it.
UserProfile = NULL;
Patch 5
HTHeader:HTHeader_deleteAll()
Zero ParseSet, immediately after freeing it.
ParseSet = NULL;
Patch 6
HTHeader:HTHeader_setMIMEParseSet()
Insert this statement at the start of the routine:
if (ParseSet) HTMIMEParseSet_deleteAll(ParseSet);
Patch 7
HTHeader:HTHeader_deleteParser()
Insert this statement at the start of the routine:
if (!ParseSet) return NO;
Patch 8
HTProxy:add_object()
Line 121 looks like this (Uninitialised Memory Read)
if (*(me->url+strlen(me->url)-1) != '/')
Replace it with this:
if ( (me->url[0] == 0) || (*(me->url+strlen(me->url)-1) != '/') )
Patch 9
HTCookie:HTCookieHolder_deleteAll()
Replace the entire routine, with this code:
if (cookie_holder == NULL) {
return NO;
}
while (cookie_holder->next) {
HTCookieHolder_delete
((HTCookieHolder*)cookie_holder->next>object);
}
HTList_delete(cookie_holder);
cookie_holder = NULL;
return YES;
The problem here was accesses to previously freed memory.
Patch 10
HTCookie:HTCookie_parseSetCookie()
This routine starts off as follows:
char * cookie_name = HTNextField(&value);
char * cookie_value = HTNextField(&value);
if (cookie_name && *cookie_name && cookie_value) {
HTCookie * cookie = HTCookie_new();
char * param_pair;
Replace those 5 lines with these:
char* cookie_name;
char* cookie_value;
char* param_pair;
param_pair = HTNextParam(&value);
cookie_name = HTNextField(¶m_pair);
cookie_value = param_pair;
if (cookie_name && *cookie_name && cookie_value) {
HTCookie * cookie = HTCookie_new();
... and then carry on with the body of the IF statement, as at present.
The problem here was that cookies with null values could not be parsed.
It may be that the real fix required here is to modify the leading while
loop in all the HTWWWStr.c routines, so that it just says:
while (*p && (isspace((int)*p)))
(since all it has to do is find the start of the term).
However, I presume that the other users of this module are working OK,
so it seemed safer to limit my tinkering to the HTCookie module
(although I did try those HTWWWStr fixes in my own application, with no
ill effects).
Patch 11
Not strictly a bug, but compiling with -DNODEBUG transforms WWWTRACE
from an integer variable to a #define'd zero constant (see HTUtils.h).
This seems like an unnecessary and unexpected side effect of trying to
get rid of the w3chttp.out trace file, and it did trip up
LineMode/src/HTBrowse.c (tries to assign to WWWTRACE).
It also irritated me a bit, because I prefered to set the trace level
thus:
WWWTRACE = SHOW_ALL_TRACE & ~SHOW_MEM_TRACE
rather than by calling HTSetTraceMessageMask(), and explictly
enumerating the trace types which I did want.
Patch 12
HTAssoc:HTAssocList_removeObject()
There is a memory leak here, as it doesn't free the HTAssoc members.
They are allocated by this module, so it is probably its duty to free
them (and HTAssocList_delete() already does so).
The fix is to call HT_FREE(assoc->name) and HT_FREE(assoc->value),
before the call to HT_FREE(assoc).
Tertio Limited - One Angel Square, Torrens Street, London EC1V
1PL
Tel: +44 (0)20 7843 4000 Fax: +44 (0)20 7843 4001 Web
http://www.tertio.com
Any views expressed in this message are those of the individual sender,
except where the sender specifically states them to be the views of
Tertio Ltd.
Received on Tuesday, 17 April 2001 07:15:20 UTC