From c128a1801bf47f62881fe50e34af1cd835d8b370 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 28 Nov 2024 17:22:36 +0100 Subject: [PATCH 1/5] 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 From 6fbe99964f3c9f18a27553e8193044a815a09a23 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Wed, 13 Aug 2025 13:22:20 +0200 Subject: [PATCH 2/5] Add CI for unit tests --- .../actions/configure-unit-tests/action.yml | 10 +++ .github/workflows/unit-tests.yml | 75 +++++++++++++++++++ tests/unit/Makefile | 1 + tests/unit/main/test_network.c | 2 +- 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 .github/actions/configure-unit-tests/action.yml create mode 100644 .github/workflows/unit-tests.yml diff --git a/.github/actions/configure-unit-tests/action.yml b/.github/actions/configure-unit-tests/action.yml new file mode 100644 index 0000000000000..ef8239b7111c7 --- /dev/null +++ b/.github/actions/configure-unit-tests/action.yml @@ -0,0 +1,10 @@ +name: ./configure (unit tests) +description: Configure PHP with minimal settings for unit testing +runs: + using: composite + steps: + - shell: bash + run: | + set -x + ./buildconf --force + ./configure --disable-all --enable-embed=static diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml new file mode 100644 index 0000000000000..dc52a152f7abd --- /dev/null +++ b/.github/workflows/unit-tests.yml @@ -0,0 +1,75 @@ +name: Unit Tests +on: + push: + paths: + - 'main/network.c' + - 'tests/unit/**' + - '.github/workflows/unit-tests.yml' + branches: + - master + pull_request: + paths: + - 'main/network.c' + - 'tests/unit/**' + - '.github/workflows/unit-tests.yml' + branches: + - '**' + workflow_dispatch: ~ + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.url || github.run_id }} + cancel-in-progress: true + +env: + CC: ccache gcc + CXX: ccache g++ + +jobs: + UNIT_TESTS: + if: github.repository == 'php/php-src' || github.event_name == 'pull_request' + name: UNIT_TESTS_LINUX_X64 + runs-on: ubuntu-24.04 + timeout-minutes: 20 + steps: + - name: git checkout + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + set -x + sudo apt-get update + sudo apt-get install -y \ + libcmocka-dev \ + autoconf \ + gcc \ + make \ + unzip \ + bison \ + re2c \ + locales \ + ccache + + - name: ccache + uses: hendrikmuhs/ccache-action@v1.2 + with: + key: "unit-tests-${{hashFiles('main/php_version.h')}}" + append-timestamp: false + save: ${{ github.event_name != 'pull_request' }} + + - name: ./configure (minimal build) + uses: ./.github/actions/configure-unit-tests + + - name: make libphp.a + run: | + set -x + make -j$(/usr/bin/nproc) >/dev/null + + - name: Run unit tests + run: | + set -x + cd tests/unit + make test + diff --git a/tests/unit/Makefile b/tests/unit/Makefile index 050b05bbd215b..ae8eeb8ab3ed9 100644 --- a/tests/unit/Makefile +++ b/tests/unit/Makefile @@ -2,6 +2,7 @@ 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 +# Update paths in .github/workflows/unit-tests.yml when adding new test to make it run in PR when such file changes 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 diff --git a/tests/unit/main/test_network.c b/tests/unit/main/test_network.c index 9ebdf098e414f..3e0e6e37ed989 100644 --- a/tests/unit/main/test_network.c +++ b/tests/unit/main/test_network.c @@ -251,4 +251,4 @@ int main(void) { cmocka_unit_test(test_php_network_connect_socket_connect_error), }; return cmocka_run_group_tests(tests, NULL, NULL); -} \ No newline at end of file +} From eacd0b5d4e4be009f3f218d60c5768a50b0454a0 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Sun, 5 Oct 2025 20:12:26 +0200 Subject: [PATCH 3/5] zend_hrtime: check posix clock once and prefer CLOCK_MONOTONIC_RAW (#19221) --- Zend/zend_hrtime.c | 21 +++++++++++++++++++++ Zend/zend_hrtime.h | 10 ++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/Zend/zend_hrtime.c b/Zend/zend_hrtime.c index 7fa36b1b654f4..773e0525cadd9 100644 --- a/Zend/zend_hrtime.c +++ b/Zend/zend_hrtime.c @@ -27,6 +27,8 @@ # include # include +ZEND_API clockid_t zend_hrtime_posix_clock_id = CLOCK_MONOTONIC; + #elif ZEND_HRTIME_PLATFORM_WINDOWS # define WIN32_LEAN_AND_MEAN @@ -66,5 +68,24 @@ void zend_startup_hrtime(void) mach_timebase_info(&zend_hrtime_timerlib_info); +#elif ZEND_HRTIME_PLATFORM_POSIX + + struct timespec ts; + +#ifdef CLOCK_MONOTONIC_RAW + if (EXPECTED(0 == clock_gettime(CLOCK_MONOTONIC_RAW, &ts))) { + zend_hrtime_posix_clock_id = CLOCK_MONOTONIC_RAW; + return; + } +#endif + + if (EXPECTED(0 == clock_gettime(zend_hrtime_posix_clock_id, &ts))) { + return; + } + + // zend_error mechanism is not initialized at that point + fprintf(stderr, "No working CLOCK_MONOTONIC* found, this should never happen\n"); + abort(); + #endif } diff --git a/Zend/zend_hrtime.h b/Zend/zend_hrtime.h index 994dd6da169ed..f3bc4deeaf502 100644 --- a/Zend/zend_hrtime.h +++ b/Zend/zend_hrtime.h @@ -72,6 +72,10 @@ ZEND_API extern double zend_hrtime_timer_scale; # include ZEND_API extern mach_timebase_info_data_t zend_hrtime_timerlib_info; +#elif ZEND_HRTIME_PLATFORM_POSIX + +ZEND_API extern clockid_t zend_hrtime_posix_clock_id; + #endif #define ZEND_NANO_IN_SEC UINT64_C(1000000000) @@ -92,10 +96,8 @@ static zend_always_inline zend_hrtime_t zend_hrtime(void) return (zend_hrtime_t)mach_absolute_time() * zend_hrtime_timerlib_info.numer / zend_hrtime_timerlib_info.denom; #elif ZEND_HRTIME_PLATFORM_POSIX struct timespec ts = { .tv_sec = 0, .tv_nsec = 0 }; - if (EXPECTED(0 == clock_gettime(CLOCK_MONOTONIC, &ts))) { - return ((zend_hrtime_t) ts.tv_sec * (zend_hrtime_t)ZEND_NANO_IN_SEC) + ts.tv_nsec; - } - return 0; + clock_gettime(zend_hrtime_posix_clock_id, &ts); + return ((zend_hrtime_t) ts.tv_sec * (zend_hrtime_t)ZEND_NANO_IN_SEC) + ts.tv_nsec; #elif ZEND_HRTIME_PLATFORM_HPUX return (zend_hrtime_t) gethrtime(); #elif ZEND_HRTIME_PLATFORM_AIX From 4fed57e7464cb4bcbf7ee2acf515156b3ab53d8f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 4 Oct 2025 14:59:27 +0200 Subject: [PATCH 4/5] Fix GH-20043: array_unique assertion failure with RC1 array causing an exception on sort The reason this happens is because the array_unique operation happens in-place because the input array is RC1. At one point during comparison an exception is thrown which will capture the arguments in the backtrace, which will increment the refcount of the RC1 array to 2. Then a modification happens after the throw on the RC2 array causing the assertion failure. We shouldn't try continue work after an exception happened during the sort. Closes GH-20059. --- NEWS | 2 ++ ext/standard/array.c | 7 +++++++ ext/standard/tests/array/gh20043.phpt | 12 ++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 ext/standard/tests/array/gh20043.phpt diff --git a/NEWS b/NEWS index 2789d02634c91..c25c1922f8b05 100644 --- a/NEWS +++ b/NEWS @@ -54,6 +54,8 @@ PHP NEWS . Fixed bug GH-19701 (Serialize/deserialize loses some data). (nielsdos) . Fixed bug GH-19801 (leaks in var_dump() and debug_zval_dump()). (alexandre-daubois) + . Fixed bug GH-20043 (array_unique assertion failure with RC1 array + causing an exception on sort). (nielsdos) - Streams: . Fixed bug GH-19248 (Use strerror_r instead of strerror in main). diff --git a/ext/standard/array.c b/ext/standard/array.c index f0a81b5ba8953..b7b5f82d61c53 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4900,6 +4900,11 @@ PHP_FUNCTION(array_unique) ZVAL_UNDEF(&arTmp[i].b.val); zend_sort((void *) arTmp, i, sizeof(struct bucketindex), (compare_func_t) cmp, (swap_func_t) array_bucketindex_swap); + + if (UNEXPECTED(EG(exception))) { + goto out; + } + /* go through the sorted array and delete duplicates from the copy */ lastkept = arTmp; for (cmpdata = arTmp + 1; Z_TYPE(cmpdata->b.val) != IS_UNDEF; cmpdata++) { @@ -4919,6 +4924,8 @@ PHP_FUNCTION(array_unique) } } } + +out: pefree(arTmp, GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); if (in_place) { diff --git a/ext/standard/tests/array/gh20043.phpt b/ext/standard/tests/array/gh20043.phpt new file mode 100644 index 0000000000000..d5c7e06417f18 --- /dev/null +++ b/ext/standard/tests/array/gh20043.phpt @@ -0,0 +1,12 @@ +--TEST-- +GH-20043 (array_unique assertion failure with RC1 array causing an exception on sort) +--FILE-- +getMessage(); +} +?> +--EXPECT-- +Object of class stdClass could not be converted to string From 66c833444cf9a52a2e95656d1116efdb8495d507 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 4 Oct 2025 12:20:49 +0200 Subject: [PATCH 5/5] phar: Fix memory leaks when creating temp file fails when applying zip signature Also fixes up the error propagation at the call site which jumped to the wrong place in the error handling code. Closes GH-20057. --- NEWS | 2 ++ ext/phar/zip.c | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index c25c1922f8b05..82b1cd77d7c32 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ PHP NEWS - Phar: . Fix memory leak and invalid continuation after tar header writing fails. (nielsdos) + . Fix memory leaks when creating temp file fails when applying zip signature. + (nielsdos) - SimpleXML: . Fixed bug GH-19988 (zend_string_init with NULL pointer in simplexml (UB)). diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 87681c69959a7..dff170dc29b48 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -1192,7 +1192,9 @@ static int phar_zip_applysignature(phar_archive_data *phar, struct _phar_zip_pas entry.fp_type = PHAR_MOD; entry.is_modified = 1; if (entry.fp == NULL) { + efree(signature); spprintf(pass->error, 0, "phar error: unable to create temporary file for signature"); + php_stream_close(newfile); return FAILURE; } @@ -1456,11 +1458,12 @@ int phar_zip_flush(phar_archive_data *phar, char *user_stub, zend_long len, int phar_metadata_tracker_try_ensure_has_serialized_data(&phar->metadata_tracker, phar->is_persistent); if (temperr) { +temperror: if (error) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: %s", phar->fname, temperr); } efree(temperr); -temperror: +notemperror: php_stream_close(pass.centralfp); nocentralerror: php_stream_close(pass.filefp); @@ -1488,7 +1491,7 @@ int phar_zip_flush(phar_archive_data *phar, char *user_stub, zend_long len, int if (error) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: unable to write central-directory", phar->fname); } - goto temperror; + goto notemperror; } }