This patch fixes the base64 tests on Vista (http://test.winehq.org/data/b3f4091b47e70681a9909bfccd19dee95657fabd/vista_A...), however to me this feels wrong.
In the call to pCryptStringToBinaryA, Vista is returning TRUE in some cases, and for those is sometimes not setting GetLastError(), and in others is setting it to ERROR_INVALID_DATA (even when it is returning TRUE!). For these cases, it is also not skipping any characters.
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.
Any ideas?
- Reece
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, --Juan
2008/7/21 Juan Lang juan.lang@gmail.com:
Hi Reece,
thanks for looking into failures on Vista.
No problem.
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.
Thanks for the pointers as to what may be going on.
Would you mind having a go? Thanks,
I'll need to take a look at the tests in more detail and the API documentation (if I can get to the non-WinCE docs on MSDN!) to see what is really going on and which ones are failing and why. I will dig deeper into this.
Thanks, - Reece
On Monday 21 July 2008 22:00:44 Reece Dunn wrote:
I'll need to take a look at the tests in more detail and the API documentation (if I can get to the non-WinCE docs on MSDN!) to see what is really going on and which ones are failing and why. I will dig deeper into this.
http://msdn.microsoft.com/en-us/library/aa380285(VS.85).aspx
It's a good idea to use another search engine like google to search MSDN. :)
Glancing over the MSDN page, there's a new dwFlag value in Vista that's CRYPT_STRING_HEXRAW, so that might be what's tried as well.
Cheers, Kai
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
-
Juan Lang -
Kai Blin -
Reece Dunn