Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests on Windows #173

Closed
gvanem opened this issue Nov 26, 2021 · 3 comments
Closed

Tests on Windows #173

gvanem opened this issue Nov 26, 2021 · 3 comments

Comments

@gvanem
Copy link

gvanem commented Nov 26, 2021

The test program test-is-cookie-domain-acceptable.exe is returning:

loaded 9189 suffixes and 8 exceptions
psl_is_cookie_domain_acceptable(192.1.123.2, .1.123.2)=1 (expected 0)
psl_is_cookie_domain_acceptable(::ffff:192.1.123.2, .1.123.2)=1 (expected 0)
Summary: 2 out of 23 tests failed

Which really is due to the error WSANOTINITIALISED from WSAStringToAddressW() (called from isip()).

So WSAStartup() needs to be called first. This is how I patched src/psl.c to make it work:

--- a/src/psl.c 2021-10-25 10:47:07
+++ b/src/psl.c 2021-11-26 10:28:44
@@ -1584,6 +1584,7 @@
 static int isip(const char *hostname)
 {
 #ifdef _WIN32
+       static int WSAStartup_done = 0;
        WCHAR wName[INET6_ADDRSTRLEN+1];

        struct sockaddr_in  addr  = {0};
@@ -1592,6 +1593,14 @@
        INT size  = sizeof(addr);
        INT size6 = sizeof(addr6);

+       if (!WSAStartup_done) {
+               WSADATA wsa_data;
+
+               WSAStartup(MAKEWORD(2,2), &wsa_data);
+               atexit ((void (__cdecl*)()) WSACleanup);
+               WSAStartup_done = 1;
+       }
+
        if (!MultiByteToWideChar(CP_UTF8, 0, hostname, -1, wName, countof(wName)))
                return 0;

So, now running test-is-cookie-domain-acceptable.exe gives:

loaded 9189 suffixes and 8 exceptions
Summary: All 23 tests passed
@rockdaboot
Copy link
Owner

Thanks for this, @gvanem.
The issue I see is that this code is not thread-safe. It is possible that the code is executed more than once, the WSAStartup_done state is not reliable.

One solution would be to introduce a mutex/lock/criticalsection.

Another one would be to document that the calling application needs to call WSAStartup() before using psl_is_cookie_domain_acceptable(). This requires a change in the docs and to add WSAStartup to the tests and to the psl tool.

WDYT ?

@gvanem
Copy link
Author

gvanem commented Nov 26, 2021

Another one would be to document that the calling application needs to call WSAStartup() before using
psl_is_cookie_domain_acceptable(). This requires a change in the docs and to add WSAStartup to the tests and to the psl tool.

Yeah. That would be cleaner.

rockdaboot added a commit that referenced this issue Nov 26, 2021
As reported at #173,
psl_is_cookie_domain_acceptable() doesn't work properly without
WSAStartup() on Windows.

Co-authored-by: gvanem@yahoo.no
@rockdaboot
Copy link
Owner

Feel free to comment on #174

rockdaboot added a commit that referenced this issue Nov 26, 2021
As reported at #173,
psl_is_cookie_domain_acceptable() doesn't work properly without
WSAStartup() on Windows.

Co-authored-by: gvanem@yahoo.no
rockdaboot added a commit that referenced this issue Nov 26, 2021
As reported at #173,
psl_is_cookie_domain_acceptable() doesn't work properly without
WSAStartup() on Windows.

Co-authored-by: gvanem@yahoo.no
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants