Re: [RFC] crypt32: fixed the base64 tests on Vista.
2008/7/21 Juan Lang [EMAIL PROTECTED]: 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 0001, got 0 (used format is 0001) 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 0006, got 0 (used format is 0001) 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 0001, got 0 (used format is 0001) 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 0006, got 0 (used format is 0001) 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 diff --git a/dlls/crypt32/tests/base64.c b/dlls/crypt32/tests/base64.c index 965b3f8..9f648ac 100644 --- a/dlls/crypt32/tests/base64.c +++ b/dlls/crypt32/tests/base64.c @@ -273,14 +273,18 @@ static void decodeAndCompareBase64_A(LPCSTR toDecode, LPCSTR header, strcat(str, toDecode); if (trailer) strcat(str, trailer); +
Re: [RFC] crypt32: fixed the base64 tests on Vista.
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 -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Re: [RFC] crypt32: fixed the base64 tests on Vista.
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
Re: [RFC] crypt32: fixed the base64 tests on Vista.
2008/7/21 Juan Lang [EMAIL PROTECTED]: 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
[RFC] crypt32: fixed the base64 tests on Vista.
This patch fixes the base64 tests on Vista
(http://test.winehq.org/data/b3f4091b47e70681a9909bfccd19dee95657fabd/vista_APetacciaVistaNXSet/crypt32:base64.html),
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
From 2208d18bef5497e7b431a2b7f1cfe3be7abb0def Mon Sep 17 00:00:00 2001
From: Reece Dunn [EMAIL PROTECTED]
Date: Sun, 20 Jul 2008 15:23:07 +0100
Subject: [PATCH] crypt32: fixed the base64 tests on Vista.
---
dlls/crypt32/tests/base64.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/dlls/crypt32/tests/base64.c b/dlls/crypt32/tests/base64.c
index 4dd7c5f..89dc485 100644
--- a/dlls/crypt32/tests/base64.c
+++ b/dlls/crypt32/tests/base64.c
@@ -273,14 +273,15 @@ static void decodeAndCompareBase64_A(LPCSTR toDecode,
LPCSTR header,
strcat(str, toDecode);
if (trailer)
strcat(str, trailer);
+SetLastError(0xdeadbeef);
ret = pCryptStringToBinaryA(str, 0, useFormat, NULL, bufLen, NULL,
NULL);
/* expect failure with no header, and success with one */
if (header)
ok(ret, CryptStringToBinaryA failed: %d\n, GetLastError());
else
-ok(!ret GetLastError() == ERROR_INVALID_DATA,
- Expected ERROR_INVALID_DATA, got %d\n, GetLastError());
+ok((!ret GetLastError() == ERROR_INVALID_DATA) || ret /* Vista
*/,
+ Expected ERROR_INVALID_DATA, got ret=%d, error=%d\n, ret,
GetLastError());
if (ret)
{
buf = HeapAlloc(GetProcessHeap(), 0, bufLen);
@@ -290,8 +291,8 @@ static void decodeAndCompareBase64_A(LPCSTR toDecode,
LPCSTR header,
ret = pCryptStringToBinaryA(str, 0, useFormat, buf, bufLen,
skipped, usedFormat);
-ok(skipped == strlen(garbage),
- Expected %d characters skipped, got %d\n, lstrlenA(garbage),
+ok(skipped == strlen(garbage) || skipped == 0 /* Vista */,
+ Expected %d or 0 characters skipped, got %d\n,
lstrlenA(garbage),
skipped);
HeapFree(GetProcessHeap(), 0, buf);
}
--
1.5.6.1.1071.g76fb
