Re: Bug fixes

Hi Yusef, Hi all.

Thank for the patches Yusef, it looks clean.
Have you worked on the released 5.3.2 version?
Or did you included the last new patches?

> 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),

I agree. Actually at InfoVista I work on this.
I have some patches to propose I hope it could help.

Michel Philip.

> Bug fixes
> 
> From: Yusef_Badri@tertio.com
> Date: Tue, Apr 17 2001
> 
> *Next message: korri@igd.fhg.de: "Entity-loss on HTTP-500"
> 
>    * Previous message: Steinar Bang: "anyone working on WebDAV support for libwww?"
>    * Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
>    * Other mail archives: [this mailing list] [other W3C mailing lists]
>    * Mail actions: [ respond to this message ] [ mail a new topic ]
> 
>   ------------------------------------------------------------------------------
> 
> From: Yusef_Badri@tertio.com
> To: www-lib@w3.org
> Cc: yb@greybox.demon.co.uk
> Date: Tue, 17 Apr 2001 12:22:05 +0100
> Message-ID: <OF696BF1FE.F8589D03-ON80256A31.00380451@LocalDomain>
> Subject: Bug fixes
> 
> 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(&param_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.
> 
>   ------------------------------------------------------------------------------
> 
>    * Next message: korri@igd.fhg.de: "Entity-loss on HTTP-500"
>    * Previous message: Steinar Bang: "anyone working on WebDAV support for libwww?"
>    * Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
>    * Other mail archives: [this mailing list] [other W3C mailing lists]
>    * Mail actions: [ respond to this message ] [ mail a new topic ]

Received on Sunday, 22 April 2001 12:28:13 UTC