[PATCH] wininet/tests: Don't check certificate when secure flag is not set

[André Hentschel]

Signed-off-by: André Hentschel <nerv(a)dawncrow.de>
---
 dlls/wininet/tests/http.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index ace8512399..8bc06b9166 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -6025,7 +6025,8 @@ static void test_secure_connection(void)
     ok(ret, "InternetQueryOption failed: %d\n", GetLastError());
     ok(flags & SECURITY_FLAG_SECURE, "expected secure flag to be set\n");
 
-    test_cert_struct(req, &test_winehq_org_cert);
+    if (flags & SECURITY_FLAG_SECURE)
+        test_cert_struct(req, &test_winehq_org_cert);
 
     /* Querying the same option through InternetQueryOptionW still results in
      * ASCII strings being returned.
-- 
2.17.1

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=42060

Your paranoid android.


=== w2003std (32 bit Windows report) ===

wininet:
http.c:6015: Test failed: HttpSendRequest failed: 12045
http.c:6817: Test failed: HttpSendRequest failed with error 12045

[Jacek Caban]

Hi André,

On 09/15/2018 02:47 PM, André Hentschel wrote:
> Signed-off-by: André Hentschel <nerv(a)dawncrow.de>
> ---
>  dlls/wininet/tests/http.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
> index ace8512399..8bc06b9166 100644
> --- a/dlls/wininet/tests/http.c
> +++ b/dlls/wininet/tests/http.c
> @@ -6025,7 +6025,8 @@ static void test_secure_connection(void)
>      ok(ret, "InternetQueryOption failed: %d\n", GetLastError());
>      ok(flags & SECURITY_FLAG_SECURE, "expected secure flag to be set\n");
>
> -    test_cert_struct(req, &test_winehq_org_cert);
> +    if (flags & SECURITY_FLAG_SECURE)
> +        test_cert_struct(req, &test_winehq_org_cert);

What do you want to achieve with this patch? Note that we test the flag
just before test_cert_struct call, so tests will fail with your patch
anyway.

Jacek

[André Hentschel]

On 17.09.2018 16:55, Jacek Caban wrote:
> Hi André,
>
> On 09/15/2018 02:47 PM, André Hentschel wrote:
> > Signed-off-by: André Hentschel <nerv(a)dawncrow.de>
> > ---
> >   dlls/wininet/tests/http.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
> > index ace8512399..8bc06b9166 100644
> > --- a/dlls/wininet/tests/http.c
> > +++ b/dlls/wininet/tests/http.c
> > @@ -6025,7 +6025,8 @@ static void test_secure_connection(void)
> >       ok(ret, "InternetQueryOption failed: %d\n", GetLastError());
> >       ok(flags & SECURITY_FLAG_SECURE, "expected secure flag to be set\n");
> >
> > -    test_cert_struct(req, &test_winehq_org_cert);
> > +    if (flags & SECURITY_FLAG_SECURE)
> > +        test_cert_struct(req, &test_winehq_org_cert);
>
> What do you want to achieve with this patch? Note that we test the flag
> just before test_cert_struct call, so tests will fail with your patch
> anyway.
>
> Jacek

Hi Jacek,

I should have mentioned that it's supposed to fix the crash in newer 
win10 versions:
https://test.winehq.org/data/8a985174ebc1b041d9a4f1abbdc184fe692d25bf/index_...

[Jacek Caban]

Hi André,

On 09/18/2018 02:09 PM, André Hentschel wrote:
> On 17.09.2018 16:55, Jacek Caban wrote:
> > Hi André,
> >
> > On 09/15/2018 02:47 PM, André Hentschel wrote:
> > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de>
> > > ---
> > >   dlls/wininet/tests/http.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
> > > index ace8512399..8bc06b9166 100644
> > > --- a/dlls/wininet/tests/http.c
> > > +++ b/dlls/wininet/tests/http.c
> > > @@ -6025,7 +6025,8 @@ static void test_secure_connection(void)
> > >       ok(ret, "InternetQueryOption failed: %d\n", GetLastError());
> > >       ok(flags & SECURITY_FLAG_SECURE, "expected secure flag to be
> > > set\n");
> > >   -    test_cert_struct(req, &test_winehq_org_cert);
> > > +    if (flags & SECURITY_FLAG_SECURE)
> > > +        test_cert_struct(req, &test_winehq_org_cert);
> >
> > What do you want to achieve with this patch? Note that we test the flag
> > just before test_cert_struct call, so tests will fail with your patch
> > anyway.
> >
> > Jacek
>
> Hi Jacek,
>
> I should have mentioned that it's supposed to fix the crash in newer
> win10 versions:
> https://test.winehq.org/data/8a985174ebc1b041d9a4f1abbdc184fe692d25bf/index_...

Still, there will be failures, so it's not much better. Ideally we'd
find why it fails. Those are not testbot machines, so it's hard to
experiment with them... I can see a value in avoiding the crash. This
way we'd see how other https-related tests behave and maybe have more
information that would help us find the actual problem. For that,
however, I'd suggest to return early from test_cert_struct when
InternetQueryOptionA fails.


Thanks,

Jacek