On 6/19/24 2:52 PM, Joe Orton wrote:
> On Wed, Jun 19, 2024 at 02:04:20PM +0200, Yann Ylavic wrote:
>> On Wed, Jun 19, 2024 at 1:00 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>> As far as I read the code it does not.
>>>
>>> https://github.com/apache/apr/blob/b0a08c76ceacc2308d7cf1d5a7bb3c9b4665a432/network_io/unix/sockets.c#L423-L433
>>>
>>> We copy the data (sa, salen family and port) not a pointer.
>>
>> Ah yes, I was looking at win32 code, while Joe fixed it 13 years ago
>> for unix (r983618).
>> So the pointer copy exists, but only for WIN32 and OS/2 AFAICT, what a mess.
>
> Sorry!
>
>> Let me fix that then ;)
>
> There is a test in that commit, is it not catching the bug on non-Unix?
As far as I can tell the test is never hit on Windows as apr_socket_connect(cd,
sa) returns APR_SUCCESS
despite being non blocking.
https://github.com/apache/apr/blob/b0a08c76ceacc2308d7cf1d5a7bb3c9b4665a432/test/testsock.c#L428-L435
.\testall.exe -v testsock
testsock : |Line 433: Cannot test if connect completes synchronously
SUCCESS
All tests passed.
The following patch would fix this by causing the test to executed and of
course to fail:
Index: network_io/win32/sockets.c
===================================================================
--- network_io/win32/sockets.c (revision 1917061)
+++ network_io/win32/sockets.c (working copy)
@@ -378,7 +378,7 @@
}
if (rv == APR_FROM_OS_ERROR(WSAEWOULDBLOCK)) {
- if (sock->timeout == 0) {
+ if (sock->timeout <= 0) {
/* Tell the app that the connect is in progress...
* Gotta play some games here. connect on Unix will return
* EINPROGRESS under the same circumstances that Windows
But as Windows is not my native environment I would appreciate remote eyes if
the fix is correct.
Regards
Rüdiger
