2008/7/21 Juan Lang juan.lang@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