Re: wininet: Add support for SSPI authentication for HTTP.

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> +        if (strncmpiW(pszAuthValue, szBasic, sizeof(szBasic)/sizeof(szBasic[0])-1))
> +        {

When using strncmp you need to also check that you reached the end of
the first string.
> +    /* compare against last character to be set to avoid a race */
> +    if (HTTP_Base64Dec['/'] != 63)
> +    {

This won't avoid the race, you'll still get garbage if two threads get
here at the same time.

-- 
Alexandre Julliard
julliard(a)winehq.org

wininet: Add support for SSPI authentication for HTTP.

[Robert Shearman]

Alexandre Julliard wrote:
> Robert Shearman <rob(a)codeweavers.com> writes:
> > +        if (strncmpiW(pszAuthValue, szBasic, sizeof(szBasic)/sizeof(szBasic[0])-1))
> > +        {
>
> When using strncmp you need to also check that you reached the end of
> the first string.

Hmm, it seems strncmpiW already does that for me so I'm a little confused:
> int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
> {
>     int ret = 0;
>     for ( ; n > 0; n--, str1++, str2++)
>         if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
>     return ret;
> }
> > +    /* compare against last character to be set to avoid a race */
> > +    if (HTTP_Base64Dec['/'] != 63)
> > +    {
>
> This won't avoid the race, you'll still get garbage if two threads get
> here at the same time.

Good spot, I'll fix this.

-- 
Rob Shearman

wininet: Add support for SSPI authentication for HTTP.

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> Hmm, it seems strncmpiW already does that for me so I'm a little confused:
> > int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
> > {
> >     int ret = 0;
> >     for ( ; n > 0; n--, str1++, str2++)
> >         if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
> >     return ret;
> > }

It does if the string is shorter, but not if it's longer. It's OK if
you want to check that the string is a strict subset, but that's not
usually what you want.

-- 
Alexandre Julliard
julliard(a)winehq.org

wininet: Add support for SSPI authentication for HTTP.

[Robert Shearman]

Alexandre Julliard wrote:
> Robert Shearman <rob(a)codeweavers.com> writes:
> > Hmm, it seems strncmpiW already does that for me so I'm a little confused:
> > > int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
> > > {
> > >     int ret = 0;
> > >     for ( ; n > 0; n--, str1++, str2++)
> > >         if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
> > >     return ret;
> > > }
>
> It does if the string is shorter, but not if it's longer. It's OK if
> you want to check that the string is a strict subset, but that's not
> usually what you want.

In this case, it is. The "Basic" string should be followed by some 
additional data which is parsed later.

-- 
Rob Shearman

wininet: Add support for SSPI authentication for HTTP.

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> In this case, it is. The "Basic" string should be followed by some
> additional data which is parsed later.

Yes, but AFAICS it's still supposed to be a separate token, so you'd
need to check for a token separator. I don't think "Basically" should
be considered a match for Basic authentication.

-- 
Alexandre Julliard
julliard(a)winehq.org