From 34edb29e49827a8bb541b18e48dd14eb6bc5fa15 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Fri, 25 Oct 2024 23:32:07 +0200 Subject: [PATCH 1/2] Fix network connect poll interuption handling When connecting to socket, it is possible to get EINTR. In such case, there should be an another attempt to connect if we are not over the timeout. The timeout should be adjusted accordingly in that case. Closes GH-16606 --- main/network.c | 108 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 37 deletions(-) diff --git a/main/network.c b/main/network.c index d6922064561fb..021513f8ff76a 100644 --- a/main/network.c +++ b/main/network.c @@ -299,6 +299,35 @@ typedef int php_non_blocking_flags_t; fcntl(sock, F_SETFL, save) #endif +#if HAVE_GETTIMEOFDAY +/* Subtract times */ +static inline void sub_times(struct timeval a, struct timeval b, struct timeval *result) +{ + result->tv_usec = a.tv_usec - b.tv_usec; + if (result->tv_usec < 0L) { + a.tv_sec--; + result->tv_usec += 1000000L; + } + result->tv_sec = a.tv_sec - b.tv_sec; + if (result->tv_sec < 0L) { + result->tv_sec++; + result->tv_usec -= 1000000L; + } +} + +static inline void php_network_set_limit_time(struct timeval *limit_time, + struct timeval *timeout) +{ + gettimeofday(limit_time, NULL); + limit_time->tv_sec += timeout->tv_sec; + limit_time->tv_usec += timeout->tv_usec; + if (limit_time->tv_usec >= 1000000) { + limit_time->tv_usec -= 1000000; + limit_time->tv_sec++; + } +} +#endif + /* Connect to a socket using an interruptible connect with optional timeout. * Optionally, the connect can be made asynchronously, which will implicitly * enable non-blocking mode on the socket. @@ -351,25 +380,53 @@ PHPAPI int php_network_connect_socket(php_socket_t sockfd, * expected when a connection is actively refused. This way, * php_pollfd_for will return a mask with POLLOUT if the connection * is successful and with POLLPRI otherwise. */ - if ((n = php_pollfd_for(sockfd, POLLOUT|POLLPRI, timeout)) == 0) { + int events = POLLOUT|POLLPRI; #else - if ((n = php_pollfd_for(sockfd, PHP_POLLREADABLE|POLLOUT, timeout)) == 0) { + int events = PHP_POLLREADABLE|POLLOUT; +#endif + struct timeval working_timeout; +#if HAVE_GETTIMEOFDAY + struct timeval limit_time, time_now; +#endif + if (timeout) { + memcpy(&working_timeout, timeout, sizeof(working_timeout)); +#if HAVE_GETTIMEOFDAY + php_network_set_limit_time(&limit_time, &working_timeout); #endif - error = PHP_TIMEOUT_ERROR_VALUE; } - if (n > 0) { - len = sizeof(error); - /* - BSD-derived systems set errno correctly - Solaris returns -1 from getsockopt in case of error - */ - if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) != 0) { + while (true) { + n = php_pollfd_for(sockfd, events, timeout ? &working_timeout : NULL); + if (n < 0) { + if (errno == EINTR) { +#if HAVE_GETTIMEOFDAY + if (timeout) { + gettimeofday(&time_now, NULL); + + if (!timercmp(&time_now, &limit_time, <)) { + /* time limit expired; no need for another poll */ + error = PHP_TIMEOUT_ERROR_VALUE; + break; + } else { + /* work out remaining time */ + sub_times(limit_time, time_now, &working_timeout); + } + } +#endif + continue; + } ret = -1; + } else if (n == 0) { + error = PHP_TIMEOUT_ERROR_VALUE; + } else { + len = sizeof(error); + /* BSD-derived systems set errno correctly. + * Solaris returns -1 from getsockopt in case of error. */ + if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) != 0) { + ret = -1; + } } - } else { - /* whoops: sockfd has disappeared */ - ret = -1; + break; } ok: @@ -392,22 +449,6 @@ PHPAPI int php_network_connect_socket(php_socket_t sockfd, } /* }}} */ -/* {{{ sub_times */ -static inline void sub_times(struct timeval a, struct timeval b, struct timeval *result) -{ - result->tv_usec = a.tv_usec - b.tv_usec; - if (result->tv_usec < 0L) { - a.tv_sec--; - result->tv_usec += 1000000L; - } - result->tv_sec = a.tv_sec - b.tv_sec; - if (result->tv_sec < 0L) { - result->tv_sec++; - result->tv_usec -= 1000000L; - } -} -/* }}} */ - /* Bind to a local IP address. * Returns the bound socket, or -1 on failure. * */ @@ -777,7 +818,6 @@ PHPAPI php_socket_t php_network_accept_incoming(php_socket_t srvsock, } /* }}} */ - /* Connect to a remote host using an interruptible connect with optional timeout. * Optionally, the connect can be made asynchronously, which will implicitly * enable non-blocking mode on the socket. @@ -809,13 +849,7 @@ php_socket_t php_network_connect_socket_to_host(const char *host, unsigned short if (timeout) { memcpy(&working_timeout, timeout, sizeof(working_timeout)); #if HAVE_GETTIMEOFDAY - gettimeofday(&limit_time, NULL); - limit_time.tv_sec += working_timeout.tv_sec; - limit_time.tv_usec += working_timeout.tv_usec; - if (limit_time.tv_usec >= 1000000) { - limit_time.tv_usec -= 1000000; - limit_time.tv_sec++; - } + php_network_set_limit_time(&limit_time, &working_timeout); #endif } From 687b3352049a59e819862b57410b0285a0bea885 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 28 Nov 2024 17:22:36 +0100 Subject: [PATCH 2/2] Add unit test setup with php_network_connect_socket test --- tests/unit/Makefile | 31 ++++ tests/unit/main/test_network.c | 254 +++++++++++++++++++++++++++++++++ 2 files changed, 285 insertions(+) create mode 100644 tests/unit/Makefile create mode 100644 tests/unit/main/test_network.c diff --git a/tests/unit/Makefile b/tests/unit/Makefile new file mode 100644 index 0000000000000..050b05bbd215b --- /dev/null +++ b/tests/unit/Makefile @@ -0,0 +1,31 @@ +CC = gcc +CFLAGS = -g -Wall -I../../ -I../../Zend -I../../main -I../../TSRM -I. -I.. +COMMON_LDFLAGS = ../../.libs/libphp.a -lcmocka -lpthread -lm -ldl -lresolv -lutil + +TESTS = main/test_network +main/test_network_SRC = main/test_network.c +main/test_network_LDFLAGS = $(COMMON_LDFLAGS) -Wl,--wrap=connect,--wrap=poll,--wrap=getsockopt,--wrap=gettimeofday + + +# Build all tests +all: $(TESTS) + +# Build rule for each test +$(TESTS): + $(CC) $(CFLAGS) -o $@.out $($(basename $@)_SRC) $($(basename $@)_LDFLAGS) + +# Run all tests +.PHONY: test +test: $(TESTS) + @echo "Running all tests..." + @for test in $(TESTS); do \ + echo "Running $$test..."; \ + $$test.out || exit 1; \ + done + +# Clean tests +.PHONY: clean +clean: + @for test in $(TESTS); do \ + rm -f $$test.out; \ + done diff --git a/tests/unit/main/test_network.c b/tests/unit/main/test_network.c new file mode 100644 index 0000000000000..9ebdf098e414f --- /dev/null +++ b/tests/unit/main/test_network.c @@ -0,0 +1,254 @@ +#include "php.h" +#include "php_network.h" +#include + +// Mocked poll +int __wrap_poll(struct pollfd *ufds, nfds_t nfds, int timeout) +{ + function_called(); + check_expected(timeout); + + int n = mock_type(int); + if (n > 0) { + ufds->revents = 1; + } else if (n < 0) { + errno = -n; + n = -1; + } + + return n; +} + +// Mocked connect +int __wrap_connect(int sockfd, const struct sockaddr *addr, socklen_t addrlen) +{ + function_called(); + errno = mock_type(int); + return errno != 0 ? -1 : 0; +} + +// Mocked getsockopt +int __wrap_getsockopt(int fd, int level, int optname, void *optval, socklen_t *optlen) +{ + function_called(); + int *error = (int *) optval; + *error = mock_type(int); + return mock_type(int); +} + +// Mocked gettimeofday +int __wrap_gettimeofday(struct timeval *time_Info, struct timezone *timezone_Info) +{ + function_called(); + struct timeval *now = mock_ptr_type(struct timeval *); + if (now) { + time_Info->tv_sec = now->tv_sec; + time_Info->tv_usec = now->tv_usec; + } + return mock_type(int); +} + +// Test successful connection +static void test_php_network_connect_socket_immediate_success(void **state) { + struct timeval timeout = { .tv_sec = 2, .tv_usec = 500000 }; + php_socket_t sockfd = 12; + int error_code = 0; + + expect_function_call(__wrap_connect); + will_return(__wrap_connect, 0); + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout, NULL, &error_code); + + assert_int_equal(result, 0); + assert_int_equal(error_code, 0); +} + +// Test successful connection in progress followed by poll +static void test_php_network_connect_socket_progress_success(void **state) { + struct timeval timeout_tv = { .tv_sec = 2, .tv_usec = 500000 }; + php_socket_t sockfd = 12; + int error_code = 0; + + // Mock connect setting EINPROGRESS errno + expect_function_call(__wrap_connect); + will_return(__wrap_connect, EINPROGRESS); + + // Mock time setting - ignored + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, NULL); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to return success + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 2500); + will_return(__wrap_poll, 1); + + // Mock no socket error + expect_function_call(__wrap_getsockopt); + will_return(__wrap_getsockopt, 0); // optval saved result + will_return(__wrap_getsockopt, 0); // actual return value + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout_tv, NULL, &error_code); + + assert_int_equal(result, 0); + assert_int_equal(error_code, 0); +} + +static void test_php_network_connect_socket_eintr_t1(void **state) { + struct timeval timeout_tv = { .tv_sec = 2, .tv_usec = 500000 }; + struct timeval start_time = { .tv_sec = 1000, .tv_usec = 0 }; // Initial time + struct timeval retry_time = { .tv_sec = 1001, .tv_usec = 200000 }; // Time after EINTR + php_socket_t sockfd = 12; + int error_code = 0; + + // Mock connect to set EINPROGRESS + expect_function_call(__wrap_connect); + will_return(__wrap_connect, EINPROGRESS); + + // Mock gettimeofday for initial call + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &start_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to return EINTR first + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 2500); + will_return(__wrap_poll, -EINTR); + + // Mock gettimeofday after EINTR + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &retry_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to succeed on retry + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 1300); + will_return(__wrap_poll, 1); + + // Mock no socket error + expect_function_call(__wrap_getsockopt); + will_return(__wrap_getsockopt, 0); + will_return(__wrap_getsockopt, 0); + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout_tv, NULL, &error_code); + + // Ensure the function succeeds + assert_int_equal(result, 0); + assert_int_equal(error_code, 0); +} + +static void test_php_network_connect_socket_eintr_t2(void **state) { + struct timeval timeout_tv = { .tv_sec = 2, .tv_usec = 1500000 }; + struct timeval start_time = { .tv_sec = 1000, .tv_usec = 300000 }; // Initial time + struct timeval retry_time = { .tv_sec = 1001, .tv_usec = 200000 }; // Time after EINTR + php_socket_t sockfd = 12; + int error_code = 0; + + // Mock connect to set EINPROGRESS + expect_function_call(__wrap_connect); + will_return(__wrap_connect, EINPROGRESS); + + // Mock gettimeofday for initial call + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &start_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to return EINTR first + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 3500); + will_return(__wrap_poll, -EINTR); + + // Mock gettimeofday after EINTR + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &retry_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to succeed on retry + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 2600); + will_return(__wrap_poll, 1); + + // Mock no socket error + expect_function_call(__wrap_getsockopt); + will_return(__wrap_getsockopt, 0); // optval saved result + will_return(__wrap_getsockopt, 0); // actual return value + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout_tv, NULL, &error_code); + + // Ensure the function succeeds + assert_int_equal(result, 0); + assert_int_equal(error_code, 0); +} + +static void test_php_network_connect_socket_eintr_t3(void **state) { + struct timeval timeout_tv = { .tv_sec = 2, .tv_usec = 500000 }; + struct timeval start_time = { .tv_sec = 1002, .tv_usec = 300000 }; // Initial time + struct timeval retry_time = { .tv_sec = 1001, .tv_usec = 2200000 }; // Time after EINTR + php_socket_t sockfd = 12; + int error_code = 0; + + // Mock connect to set EINPROGRESS + expect_function_call(__wrap_connect); + will_return(__wrap_connect, EINPROGRESS); + + // Mock gettimeofday for initial call + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &start_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to return EINTR first + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 2500); + will_return(__wrap_poll, -EINTR); + + // Mock gettimeofday after EINTR + expect_function_call(__wrap_gettimeofday); + will_return(__wrap_gettimeofday, &retry_time); + will_return(__wrap_gettimeofday, 0); + + // Mock poll to succeed on retry + expect_function_call(__wrap_poll); + expect_value(__wrap_poll, timeout, 1600); + will_return(__wrap_poll, 1); + + // Mock no socket error + expect_function_call(__wrap_getsockopt); + will_return(__wrap_getsockopt, 0); // optval saved result + will_return(__wrap_getsockopt, 0); // actual return value + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout_tv, NULL, &error_code); + + // Ensure the function succeeds + assert_int_equal(result, 0); + assert_int_equal(error_code, 0); +} + +// Test connection error (ECONNREFUSED) +static void test_php_network_connect_socket_connect_error(void **state) { + struct timeval timeout = { .tv_sec = 2, .tv_usec = 500000 }; + php_socket_t sockfd = 12; + int error_code = 0; + + // Mock connect to set ECONNREFUSED + expect_function_call(__wrap_connect); + will_return(__wrap_connect, ECONNREFUSED); + + int result = php_network_connect_socket(sockfd, NULL, 0, 0, &timeout, NULL, &error_code); + + // Ensure the function returns an error + assert_int_equal(result, -1); + assert_int_equal(error_code, ECONNREFUSED); +} + + +int main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_php_network_connect_socket_immediate_success), + cmocka_unit_test(test_php_network_connect_socket_progress_success), + cmocka_unit_test(test_php_network_connect_socket_eintr_t1), + cmocka_unit_test(test_php_network_connect_socket_eintr_t2), + cmocka_unit_test(test_php_network_connect_socket_eintr_t3), + cmocka_unit_test(test_php_network_connect_socket_connect_error), + }; + return cmocka_run_group_tests(tests, NULL, NULL); +} \ No newline at end of file