[RFC] crypt32: fixed the base64 tests on Vista.

[Reece Dunn]

2008/7/21 Juan Lang <juan.lang at gmail.com>:
> Hi Reece,
>
> thanks for looking into failures on Vista.
>
>> To me, without understanding this in any more detail, it looks as if
>> Vista is broken here (and thus the Vista return parts should be marked
>> as broken()). However, Vista may be doing the right thing, and thus
>> the behaviour in these cases has changed between XP and Vista. The
>> latter seems more likely, but I do not have any experience in this
>> area, nor understand these tests well enough to say one way or the
>> other.
>
> I think you're probably right that Vista's changed.  By definition,
> that makes it "right."  I suspect what it's doing is guessing that
> something is base64-encoded even if it doesn't begin with the
> "-----BEGIN CERTIFICATE-----" or whatever header.  It'd be interesting
> to see what format Vista guesses the encoded data was in when the
> encoded data have no header.  I don't have access to a Vista machine
> myself, so it'd be hard for me to fix it.
>
> Would you mind having a go?  Thanks,

Ok, so I extended these tests to give more information on failure to
help see what is going on (see the crypt32/tests: be more verbose on
the failing base64 tests on Vista to help locate the failures. patch).

What I get is:

base64.c:282: Test failed: Expected !ret and last error
ERROR_INVALID_DATA, got ret=1, error=13
base64.c:293: Test failed: Expected 9 characters of "garbage
AQID
" skipped when trying format 00000001, got 0 (used format is 00000001)
base64.c:282: Test failed: Expected !ret and last error
ERROR_INVALID_DATA, got ret=1, error=13
base64.c:293: Test failed: Expected 9 characters of "garbage
AQID
" skipped when trying format 00000006, got 0 (used format is 00000001)
base64.c:282: Test failed: Expected !ret and last error
ERROR_INVALID_DATA, got ret=1, error=13
base64.c:293: Test failed: Expected 9 characters of "garbage
AQID
" skipped when trying format 00000001, got 0 (used format is 00000001)
base64.c:282: Test failed: Expected !ret and last error
ERROR_INVALID_DATA, got ret=1, error=13
base64.c:293: Test failed: Expected 9 characters of "garbage
AQID
" skipped when trying format 00000006, got 0 (used format is 00000001)
base64: 710 tests executed (0 marked as todo, 8 failures), 0 skipped.


So it is only failing on the CRYPT_STRING_BASE64 (1) and
CRYPT_STRING_BASE64_ANY (6), which both correctly select
CRYPT_STRING_BASE64 as the format to use (the tests at line 264
comparing use/used formats do not fail) as expected.

Some things of interest are:

1.  The tests @282 report that it is failing (last error ==
ERROR_INVALID_DATA) while ret is indicating it succeeded.

This is actually misleading, because this RFC patch shows (by setting
last error to some garbage value before the call), that the last error
is not being set by Vista in this case.

2.  The documentation for CRYPT_STRING_BASE64 (thanks for the link
Kai!) notes that this format does not include a header.

3.  The garbage+data test is passing on Vista for all but the "AQID"
test, where it is not skipping the "garbage\r\n" string.

The thing that is interesting about "AQID" is that it is the only case
that does not have '=' at the end of the buffer. It is this test that
is succeeding without a header and is not skipping any data.

Therefore, it would make sense from (3) that data is skipped until a
header is found, only if a header is present. This is only for Vista
and later. Indeed, if I apply:
-                ok(skipped == strlen(garbage),
+                ok((skipped == strlen(garbage)) || (skipped == 0 && !header),
the skipped characters tests pass on Vista.

So this leaves the
        if (header)
            ok(ret, "CryptStringToBinaryA failed: %d\n", GetLastError());
        else
            ok(!ret && GetLastError() == ERROR_INVALID_DATA,
test @282.

I am not sure what to do here, but the complete check looks like:
            ok((!ret && GetLastError() == ERROR_INVALID_DATA) ||
             (useFormat == CRYPT_STRING_BASE64 && ret &&
GetLastError() == 0xdeadbeef) /* Vista */ ||
             (useFormat == CRYPT_STRING_BASE64_ANY && ret &&
GetLastError() == ERROR_INVALID_DATA) /* Vista */,
             "Expected !ret and last error ERROR_INVALID_DATA when
converting \"%s\" with format %d, got ret=%d, error=%d\n",
             str, useFormat, ret, GetLastError());
This makes the remaining tests pass on Vista, but looks darn ugly!

The question here is what to do in this case. Removing the test is
loosing information in the other tests and is not very helpful - I
only want to remove this test as a last resort.

I have attached a diff that provides the above changes, fixing these
tests on Vista.

Comments?

- Reece
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: base64-vista-fix-rfc.txt
Url: http://www.winehq.org/pipermail/wine-devel/attachments/20080722/b1ce80ec/attachment-0001.txt