Re: WININET: implement InternetTime{From,To}SystemTime{A,W} (#4)

[Joris Huizer]

Hans Leidekker wrote:

Well, if you really want to be sure no bad string overflows occur, this 
function is still not save:
> +/***********************************************************************
> -
> -
>       InternetTimeToSystemTimeW (WININET.@)
> - */
>
> +BOOL WINAPI InternetTimeToSystemTimeW( LPCWSTR string, SYSTEMTIME* time, DWORD reserved )
> +{
> - unsigned int i;
> - WCHAR *s = (LPWSTR)string;
> -
> - TRACE( "%s %p 0x%08lx\n", debugstr_w(string), time, reserved );
> -
> - if (!string || !time || reserved != 0) return FALSE;
> -
> - /*  Convert an RFC1123 time such as 'Fri, 07 Jan 2005 12:06:35 GMT' into
> -
> *  a SYSTEMTIME structure.
> -
> */
> -
> - while (*s && !isalphaW( *s )) s++;
> - if (*s == '\0' || *(s + 1) == '\0' || *(s + 2) == '\0') return FALSE;
> - time->wDayOfWeek = 7;
> -
> - for (i = 0; i < 7; i++)
> - {
> -
>    if (toupperW( WININET_wkday[i][0] ) == toupperW( *s ) &&
> -
>        toupperW( WININET_wkday[i][1] ) == toupperW( *(s + 1) ) &&
> -
>        toupperW( WININET_wkday[i][2] ) == toupperW( *(s + 2) ) )
> -
>    {
> -
>        time->wDayOfWeek = i;
> -
>        break;
> -
>    }
> - }

What happens if, the string ends with WININET_wkday[i][0] or 
WININET_wkday[i][1] ?
> -
> - if (time->wDayOfWeek > 6) return FALSE;
> - while (*s && !isdigitW( *s )) s++;

What happens if s just became NULL in this loop ?
> - time->wDay = strtolW( s, &s, 10 );
> -
> - while (*s && !isalphaW( *s )) s++;
> - if (*s == '\0' || *(s + 1) == '\0' || *(s + 2) == '\0') return FALSE;
> - time->wMonth = 0;
> -
> - for (i = 0; i < 12; i++)
> - {
> -
>    if (toupperW( WININET_month[i][0]) == toupperW( *s ) &&
> -
>        toupperW( WININET_month[i][1]) == toupperW( *(s + 1) ) &&
> -
>        toupperW( WININET_month[i][2]) == toupperW( *(s + 2) ) )
> -
>    {
> -
>        time->wMonth = i + 1;
> -
>        break;
> -
>    }
> - }

What happens if, the string ends with WININET_month[i][0] or 
WININET_month[i][1] ?
> - if (time->wMonth == 0) return FALSE;
> -
> - while (*s && !isdigitW( *s )) s++;
> - if (*s == '\0') return FALSE;
> - time->wYear = strtolW( s, &s, 10 );
> -
> - while (*s && !isdigitW( *s )) s++;
> - if (*s == '\0') return FALSE;
> - time->wHour = strtolW( s, &s, 10 );
> -
> - while (*s && !isdigitW( *s )) s++;
> - if (*s == '\0') return FALSE;
> - time->wMinute = strtolW( s, &s, 10 );
> -
> - while (*s && !isdigitW( *s )) s++;
> - if (*s == '\0') return FALSE;
> - time->wSecond = strtolW( s, &s, 10 );
> -
> - time->wMilliseconds = 0;
> - return TRUE;
>
> +}

Yes it's a pain, but if you want to be really strict on this, this is 
not save yet

regards,

Joris

WININET: implement InternetTime{From,To}SystemTime{A,W} (#4)

[Hans Leidekker]

On Monday 17 January 2005 11:18, Joris Huizer wrote:
> > - while (*s && !isalphaW( *s )) s++;
> > - if (*s == '\0' || *(s + 1) == '\0' || *(s + 2) == '\0') return
> >
> > FALSE; +    time->wDayOfWeek = 7;
> > -
> > - for (i = 0; i < 7; i++)
> > - {
> > -
> >    if (toupperW( WININET_wkday[i][0] ) == toupperW( *s ) &&
> > -
> >        toupperW( WININET_wkday[i][1] ) == toupperW( *(s + 1) ) &&
> > -
> >        toupperW( WININET_wkday[i][2] ) == toupperW( *(s + 2) ) )
> > -
> >    {
> > -
> >        time->wDayOfWeek = i;
> > -
> >        break;
> > -
> >    }
> > - }
>
> What happens if, the string ends with WININET_wkday[i][0] or
> WININET_wkday[i][1] ?

It can't, that's protected by the:

if (*s == '\0' || *(s + 1) == '\0' || *(s + 2) == '\0') return FALSE

above. The third check prevents that.
> > - if (time->wDayOfWeek > 6) return FALSE;
> > - while (*s && !isdigitW( *s )) s++;
>
> What happens if s just became NULL in this loop ?

Huh?
> What happens if, the string ends with WININET_month[i][0] or
> WININET_month[i][1] ?

Same as above.

-Hans

WININET: implement InternetTime{From,To}SystemTime{A,W} (#4)

[Joris Huizer]

Hans Leidekker wrote:
> On Monday 17 January 2005 11:18, Joris Huizer wrote:
> > > - if (time->wDayOfWeek > 6) return FALSE;
> > > - while (*s && !isdigitW( *s )) s++;
> >
> > What happens if s just became NULL in this loop ?
>
> Huh?

Sorry, I meant, what if the string ends with digits there?
If I find out how, I may add a testcase for that;
(Sorry about the other cases, I didn't read carefull enough)

regards,

Joris

WININET: implement InternetTime{From,To}SystemTime{A,W} (#4)

[Hans Leidekker]

On Monday 17 January 2005 16:13, Joris Huizer wrote:
> Sorry, I meant, what if the string ends with digits there?
> If I find out how, I may add a testcase for that;

The next line is this one:

if (*s == '\0') return FALSE;

So if the string were to end with a digit we bail out right away.
Just tested that here as well and it's ok.

-Hans

WININET: implement InternetTime{From,To}SystemTime{A,W} (#4)

[Mike McCormack]

Hans Leidekker wrote:
> > What happens if, the string ends with WININET_wkday[i][0] or
> > WININET_wkday[i][1] ?
>
> It can't, that's protected by the:

The best way to prove these things is to write a test cases.  If others 
have doubts about your code, then they can add to the test case and 
check the code still works.

Mike