From 4d7418998aad6ba55975bc2922ef17eb540d253e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Thu, 28 May 2026 21:53:01 +0100 Subject: [PATCH 1/6] Prevent request_exec being issued between requests --- appsec/src/extension/ddappsec.c | 6 +++ appsec/src/extension/request_lifecycle.c | 22 ++++++++++ appsec/src/extension/request_lifecycle.h | 1 + .../php/integration/RoadRunnerTests.groovy | 44 +++++++++++++++++++ .../_handlers/src/PostRespondLfiHandler.php | 21 +++++++++ .../src/test/www/roadrunner/worker.php | 6 +++ 6 files changed, 100 insertions(+) create mode 100644 appsec/tests/integration/src/test/www/_handlers/src/PostRespondLfiHandler.php diff --git a/appsec/src/extension/ddappsec.c b/appsec/src/extension/ddappsec.c index e839b60dd95..9c2ef9c2f42 100644 --- a/appsec/src/extension/ddappsec.c +++ b/appsec/src/extension/ddappsec.c @@ -573,6 +573,12 @@ PHP_FUNCTION(datadog_appsec_push_addresses) RETURN_FALSE; } + if (!dd_req_lifecycle_is_active()) { + mlog_g(dd_log_info, + "Not running inside a tracked request; skipping push_addresses"); + RETURN_FALSE; + } + zval *addresses; zend_string *rasp_rule = NULL; zend_string *rule_variant = NULL; diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index 0e14c3de991..4d9cb27088f 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -49,6 +49,7 @@ static THREAD_LOCAL_ON_ZTS zend_object *nullable _cur_req_span; static THREAD_LOCAL_ON_ZTS zend_array *nullable _superglob_equiv; static THREAD_LOCAL_ON_ZTS zend_string *nullable _client_ip; static THREAD_LOCAL_ON_ZTS zval _blocking_function; +static THREAD_LOCAL_ON_ZTS bool _between_init_shutdown_msgs; static THREAD_LOCAL_ON_ZTS bool _shutdown_done_on_commit; static THREAD_LOCAL_ON_ZTS bool _empty_service_or_env; static THREAD_LOCAL_ON_ZTS bool _request_blocked; @@ -186,6 +187,17 @@ static bool _rem_cfg_path_changed(bool ignore_empty /* called from rinit */) return true; } +// Whether the result of dd_request_init means the helper has entered its +// in-request (inner) loop and a request_shutdown will follow for this request. +// The helper enters the inner loop for any WAF verdict (clean, record, block or +// redirect); only a communication failure or other error leaves it in the outer +// loop with no matching shutdown. +static bool _req_init_started_request(dd_result res) +{ + return res == dd_success || res == dd_should_record || + res == dd_should_block || res == dd_should_redirect; +} + static zend_array *nullable _do_request_begin( zval *nullable rbe_zv /* needs free */) { @@ -231,12 +243,18 @@ static zend_array *nullable _do_request_begin( if (res == dd_success && DDAPPSEC_G(active)) { struct timespec start = dd_monotime_start(); res = dd_request_init(conn, &req_info); + if (_req_init_started_request(res)) { + _between_init_shutdown_msgs = true; + } dd_duration_waf_ext_account(&start); } } else if (DDAPPSEC_G(active)) { // request_init struct timespec start = dd_monotime_start(); res = dd_request_init(conn, &req_info); + if (_req_init_started_request(res)) { + _between_init_shutdown_msgs = true; + } dd_duration_waf_ext_account(&start); } @@ -442,6 +460,7 @@ static void _reset_globals(void) } ZVAL_UNDEF(&_blocking_function); + _between_init_shutdown_msgs = false; _shutdown_done_on_commit = false; _request_blocked = false; dd_tags_rshutdown(); @@ -472,6 +491,8 @@ static void _set_cur_span(zend_object *nullable span) } } +bool dd_req_lifecycle_is_active(void) { return _between_init_shutdown_msgs; } + zend_object *nullable dd_req_lifecycle_get_cur_span(void) { return _cur_req_span; @@ -581,6 +602,7 @@ static zend_array *nullable _response_commit( } _shutdown_done_on_commit = true; + _between_init_shutdown_msgs = false; return res; } diff --git a/appsec/src/extension/request_lifecycle.h b/appsec/src/extension/request_lifecycle.h index 55262b6509e..6a01d27b576 100644 --- a/appsec/src/extension/request_lifecycle.h +++ b/appsec/src/extension/request_lifecycle.h @@ -22,5 +22,6 @@ enum request_stage { zend_array *nullable dd_req_lifecycle_abort(enum request_stage stage, dd_result result, struct block_params *nonnull block_params); +bool dd_req_lifecycle_is_active(void); zend_object *nullable dd_req_lifecycle_get_cur_span(void); zend_string *nullable dd_req_lifecycle_get_client_ip(void); diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy index d8d27f00eef..51ade591d40 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy @@ -3,16 +3,22 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.docker.InspectContainerHelper +import groovy.util.logging.Slf4j +import org.junit.jupiter.api.Assumptions import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.Test import org.junit.jupiter.api.condition.EnabledIf import org.testcontainers.junit.jupiter.Container import org.testcontainers.junit.jupiter.Testcontainers +import java.net.http.HttpResponse + import static com.datadog.appsec.php.integration.TestParams.getPhpVersion import static com.datadog.appsec.php.integration.TestParams.getVariant import static com.datadog.appsec.php.integration.TestParams.phpVersionAtLeast @Testcontainers +@Slf4j @EnabledIf('isExpectedVersion') class RoadRunnerTests implements WorkerStrategyTests { static boolean expectedVersion = phpVersionAtLeast('7.4') && !variant.contains('zts') @@ -46,4 +52,42 @@ class RoadRunnerTests implements WorkerStrategyTests { Thread.sleep(500) } } + + /** + * Regression test for the AppSec helper "unexpected command RequestExec" bug: + * RequestExec sent not between a request init and a request shutdown. + */ + @Test + void 'no unexpected RequestExec in outer loop after post-respond fopen'() { + Assumptions.assumeTrue(System.getProperty('USE_HELPER_RUST') != null, + 'This bug only manifests on the Rust helper (strict outer/inner loop state machine).') + + long logOffset = (CONTAINER.execInContainer('bash', '-c', + 'wc -c < /tmp/logs/helper.log').stdout.trim() as long) + + // PostRespondLfiHandler sets a callback that calls fopen('../etc/passwd') + // after respond() returns. By that point, request_shutdown has been sent + // via the response_committed hook. If push_addresses() still reaches the + // helper (socket open, active=true), it sends RequestExec into the outer loop. + CONTAINER.traceFromRequest('/post-respond-lfi') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + // Follow-up request verifies the connection is still usable. + CONTAINER.traceFromRequest('/') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + String helperLog = CONTAINER.execInContainer('bash', '-c', + "tail -c +${logOffset + 1} /tmp/logs/helper.log").stdout + + log.info("Helper log since offset:\n{}", helperLog) + + assert !helperLog.contains('unexpected command RequestExec') : + "Helper received RequestExec in outer loop. " + + "Relevant log:\n" + + helperLog.readLines().findAll { + it.contains('unexpected command') || it.contains('error in request loop') + }.join('\n') + } } diff --git a/appsec/tests/integration/src/test/www/_handlers/src/PostRespondLfiHandler.php b/appsec/tests/integration/src/test/www/_handlers/src/PostRespondLfiHandler.php new file mode 100644 index 00000000000..fd692b889f4 --- /dev/null +++ b/appsec/tests/integration/src/test/www/_handlers/src/PostRespondLfiHandler.php @@ -0,0 +1,21 @@ + 'text/plain'], 'OK'); + } +} diff --git a/appsec/tests/integration/src/test/www/roadrunner/worker.php b/appsec/tests/integration/src/test/www/roadrunner/worker.php index de9b32c3c2c..36ea37179b0 100644 --- a/appsec/tests/integration/src/test/www/roadrunner/worker.php +++ b/appsec/tests/integration/src/test/www/roadrunner/worker.php @@ -18,6 +18,7 @@ $router->addRoute('/', new \App\HomePageHandler()); $router->addRoute('/json', new \App\JsonHandler()); $router->addRoute('/xml', new \App\XmlHandler()); +$router->addRoute('/post-respond-lfi', new \App\PostRespondLfiHandler()); while ($req = $httpWorker->waitRequest()) { /** @var \Spiral\RoadRunner\Http\Request $req */ @@ -47,4 +48,9 @@ ); } // \dd_trace_close_all_spans_and_flush(); + // Post-respond hook: fires after request_shutdown has been sent inside respond(). + if (isset($GLOBALS['_rr_post_respond'])) { + ($GLOBALS['_rr_post_respond'])(); + unset($GLOBALS['_rr_post_respond']); + } } From 2eded5701475a2601d0d941a918f46aa70420ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Fri, 29 May 2026 17:47:29 +0100 Subject: [PATCH 2/6] appsec: fix bailouts during rshutdown --- appsec/src/extension/ddappsec.h | 1 + appsec/src/extension/request_lifecycle.c | 38 +++++++- appsec/src/extension/user_tracking.c | 6 ++ .../datadog/appsec/php/docker/LogFile.groovy | 49 ++++++++++ .../datadog/appsec/php/docker/PhpFpm.groovy | 97 +++++++++++++++++++ .../php/integration/Apache2FpmTests.groovy | 15 +-- .../EndpointFallbackSamplingTests.groovy | 9 +- .../php/integration/NginxFpmTests.groovy | 72 ++++++++++++++ .../php/integration/RoadRunnerTests.groovy | 52 ++++++++-- .../php/integration/SamplingTestsInFpm.groovy | 15 +-- .../php/integration/ZtsGshutdownTests.groovy | 18 ++-- .../src/PostRespondTrackUserHandler.php | 23 +++++ .../test/www/base/public/rshutdown_oom.php | 66 +++++++++++++ .../src/test/www/roadrunner/worker.php | 1 + 14 files changed, 413 insertions(+), 49 deletions(-) create mode 100644 appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/LogFile.groovy create mode 100644 appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/PhpFpm.groovy create mode 100644 appsec/tests/integration/src/test/www/_handlers/src/PostRespondTrackUserHandler.php create mode 100644 appsec/tests/integration/src/test/www/base/public/rshutdown_oom.php diff --git a/appsec/src/extension/ddappsec.h b/appsec/src/extension/ddappsec.h index c19fa82618e..17d219f5060 100644 --- a/appsec/src/extension/ddappsec.h +++ b/appsec/src/extension/ddappsec.h @@ -38,6 +38,7 @@ ZEND_BEGIN_MODULE_GLOBALS(ddappsec) bool to_be_configured : 1; bool skip_rshutdown : 1; + // used to avoid a bailout during request shutdown bool during_request_shutdown : 1; ZEND_END_MODULE_GLOBALS(ddappsec) // clang-format on diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index 4d9cb27088f..c627ac2ee0b 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -283,7 +283,39 @@ static zend_array *nullable _do_request_begin( return spec; } +static void _do_req_lifecycle_rshutdown(bool ignore_verdict, bool force); + void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) +{ + // temporarily remove the memory limit to avoid bailouts + // and as a safety net do a try-catch + __auto_type orig_mem_limit = PG(memory_limit); + zend_set_memory_limit((size_t)Z_L(-1) >> 1); + + zend_try { + _do_req_lifecycle_rshutdown(ignore_verdict, force); + } zend_catch { + if (PG(last_error_message)) { + mlog_g(dd_log_error, + "Bailout in request shutdown; disconnecting from helper: %s", + ZSTR_VAL(PG(last_error_message))); + } else { + mlog_g(dd_log_error, + "Bailout in request shutdown; disconnecting from helper"); + } + _reset_globals(); + dd_helper_close_conn(); // note: not completely bailout-safe, + // but should be fine with the raised mem limit + } zend_end_try(); + + __auto_type res = zend_set_memory_limit(orig_mem_limit); + if (res == FAILURE) { + mlog(dd_log_error, + "Failed to restore memory limit in request shutdown"); + } +} + +static void _do_req_lifecycle_rshutdown(bool ignore_verdict, bool force) { if (DDAPPSEC_G(enabled) == APPSEC_FULLY_DISABLED) { mlog_g(dd_log_debug, "Skipping all request shutdown actions because " @@ -437,6 +469,7 @@ static zend_array *_do_request_finish_user_req(bool ignore_verdict, return spec; } +// Should be bailout-safe -- means, in particular, no emalloc allocations static void _reset_globals(void) { _set_cur_span(NULL); @@ -491,7 +524,10 @@ static void _set_cur_span(zend_object *nullable span) } } -bool dd_req_lifecycle_is_active(void) { return _between_init_shutdown_msgs; } +bool dd_req_lifecycle_is_active(void) +{ + return _between_init_shutdown_msgs && DDAPPSEC_G(active); +} zend_object *nullable dd_req_lifecycle_get_cur_span(void) { diff --git a/appsec/src/extension/user_tracking.c b/appsec/src/extension/user_tracking.c index e1a865ee3d8..2ec46df104e 100644 --- a/appsec/src/extension/user_tracking.c +++ b/appsec/src/extension/user_tracking.c @@ -228,6 +228,12 @@ void dd_find_and_apply_verdict_for_user(zend_string *nullable user_id, return; } + if (!dd_req_lifecycle_is_active()) { + mlog_g(dd_log_info, + "Not running inside a tracked request; skipping user verdict"); + return; + } + dd_conn *conn = dd_helper_mgr_cur_conn(); if (conn == NULL) { mlog(dd_log_debug, "No connection; unable to check user"); diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/LogFile.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/LogFile.groovy new file mode 100644 index 00000000000..db95a1c2267 --- /dev/null +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/LogFile.groovy @@ -0,0 +1,49 @@ +package com.datadog.appsec.php.docker + +/** + * A log file inside an {@link AppSecContainer}. Supports marking the current + * end-of-file position and then reading only what was appended since the mark, + * which is the usual way a test isolates the log output of a single action. + * + * 
+ *   def lf = new LogFile(container, 'helper.log')
+ *   lf.markEndPos()
+ *   ... do something that writes to the log ...
+ *   lf.linesSinceMark().any { it.contains('boom') }
+ *
+ * + * A {@code name} without a leading {@code /} is resolved under {@link #LOG_DIR}. + */ +class LogFile { + static final String LOG_DIR = '/tmp/logs' + + private final AppSecContainer container + final String path + private long markPos = 0 + + LogFile(AppSecContainer container, String name) { + this.container = container + this.path = name.startsWith('/') ? name : "${LOG_DIR}/${name}" + } + + /** Current size of the log in bytes (0 if it does not exist yet). */ + long size() { + container.execInContainer('bash', '-c', "wc -c < ${path} 2>/dev/null || echo 0".toString()) + .stdout.trim() as long + } + + /** Record the current end position; subsequent reads start from here. */ + void markEndPos() { + markPos = size() + } + + /** Raw text appended since the last {@link #markEndPos()}. */ + String getTextSinceMark() { + container.execInContainer('bash', '-c', "tail -c +${markPos + 1} ${path}".toString()).stdout + } + + /** Lines appended since the last {@link #markEndPos()}. */ + List getLinesSinceMark() { + getTextSinceMark().readLines() + } +} diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/PhpFpm.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/PhpFpm.groovy new file mode 100644 index 00000000000..e7c768b4f89 --- /dev/null +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/PhpFpm.groovy @@ -0,0 +1,97 @@ +package com.datadog.appsec.php.docker + +import groovy.util.logging.Slf4j +import org.testcontainers.containers.Container.ExecResult + +/** + * Helpers for reconfiguring PHP-FPM inside a running {@link AppSecContainer}. + * + * Two strategies are offered: + *
    + *
  • {@link #restart} hard-kills every php-fpm process and relaunches the + * master, optionally exporting extra environment variables or loading a + * different php.ini. Use it for changes that only take effect on a cold + * start (environment variables, php.ini edits).
    • + *
  • {@link #reload} sends SIGUSR2 for a graceful reload and blocks until the + * pre-reload workers have been replaced. Use it for pool.d settings (e.g. + * {@code pm.max_children}) that FPM re-reads on USR2; pair it with + * {@link #setPoolValue} / {@link #backupPoolConfig} / {@link #restorePoolConfig}.
    • + *
+ */ +@Slf4j +class PhpFpm { + static final String FPM_CONF = '/etc/php-fpm.conf' + static final String DEFAULT_INI = '/etc/php/php.ini' + static final String POOL_CONF = '/etc/php-fpm.d/www.conf' + + private final AppSecContainer container + + PhpFpm(AppSecContainer container) { + this.container = container + } + + /** + * Hard-restart php-fpm: kill every php-fpm process and relaunch the master. + * + * @param env extra environment variables exported before launch + * @param iniPath the php.ini to load (defaults to {@link #DEFAULT_INI}) + */ + ExecResult restart(Map env = [:], String iniPath = DEFAULT_INI) { + container.flushProfilingData() + String exports = env.collect { k, v -> "export ${k}=${v};" }.join(' ') + ExecResult res = container.execInContainer('bash', '-c', + "kill -9 `pgrep php-fpm`; ${exports} php-fpm -y ${FPM_CONF} -c ${iniPath}".toString()) + assert res.exitCode == 0 : "php-fpm restart failed: ${res.stderr}" + res + } + + /** Rewrite a single pool directive (e.g. {@code pm.max_children}) in place. */ + void setPoolValue(String key, String value, String poolConf = POOL_CONF) { + container.execInContainer('sed', '-i', "s/${key} = .*/${key} = ${value}/".toString(), poolConf) + } + + /** Back up the pool config so {@link #restorePoolConfig} can revert any edits. */ + void backupPoolConfig(String poolConf = POOL_CONF) { + container.execInContainer('cp', poolConf, "${poolConf}.bak".toString()) + } + + /** Restore the pool config saved by {@link #backupPoolConfig}. */ + void restorePoolConfig(String poolConf = POOL_CONF) { + container.execInContainer('mv', "${poolConf}.bak".toString(), poolConf) + } + + /** + * Gracefully reload the FPM master (re-reads pool config without dropping the + * socket) and block until every pre-reload worker has been replaced by a + * freshly-spawned one. + */ + void reload(long timeoutMillis = 5_000) { + List old = workerPids() + // Locate the master via its pid file, falling back to the oldest php-fpm process. + container.execInContainer('bash', '-c', + 'kill -USR2 $(cat /run/php-fpm*.pid /var/run/php-fpm*.pid 2>/dev/null | head -1) ' + + '2>/dev/null || pkill -USR2 -o php-fpm || true') + waitForWorkerTurnover(old, timeoutMillis) + } + + /** PIDs of the current pool worker processes (the master is excluded). */ + List workerPids() { + container.execInContainer('bash', '-c', "pgrep -f 'php-fpm: pool' || true") + .stdout.readLines()*.trim().findAll { it } + } + + private void waitForWorkerTurnover(List old, long timeoutMillis) { + long deadline = System.currentTimeMillis() + timeoutMillis + while (true) { + List current = workerPids() + if (!current.isEmpty() && current.intersect(old).isEmpty()) { + return + } + if (System.currentTimeMillis() > deadline) { + throw new IllegalStateException( + "php-fpm workers were not reloaded in time (old=${old}, current=${current})".toString()) + } + Thread.sleep(100) + } + } +} diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Apache2FpmTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Apache2FpmTests.groovy index 9be5159b761..e92c05240b2 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Apache2FpmTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Apache2FpmTests.groovy @@ -3,6 +3,7 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.docker.InspectContainerHelper +import com.datadog.appsec.php.docker.PhpFpm import groovy.util.logging.Slf4j import org.junit.jupiter.api.Test import org.junit.jupiter.api.condition.DisabledIf @@ -79,21 +80,11 @@ class Apache2FpmTests implements CommonTests, SamplingTestsInFpm, EndpointFallba } void setRateLimit(String limit) { - flushProfilingData() - def res = container.execInContainer( - 'bash', '-c', - """kill -9 `pgrep php-fpm`; - export DD_APPSEC_TRACE_RATE_LIMIT=$limit; - php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini""") - assert res.exitCode == 0 + new PhpFpm(container).restart([DD_APPSEC_TRACE_RATE_LIMIT: limit]) } private void resetFpm() { - flushProfilingData() - container.execInContainer( - 'bash', '-c', - '''kill -9 `pgrep php-fpm`; - php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''') + new PhpFpm(container).restart() } @Test diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/EndpointFallbackSamplingTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/EndpointFallbackSamplingTests.groovy index 6c051a9ebd9..d9c32c1e6b8 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/EndpointFallbackSamplingTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/EndpointFallbackSamplingTests.groovy @@ -1,6 +1,7 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer +import com.datadog.appsec.php.docker.PhpFpm import org.junit.jupiter.api.Test import java.net.http.HttpResponse @@ -110,12 +111,6 @@ trait EndpointFallbackSamplingTests extends SamplingTestsInFpm { } void disableEndpointRenaming() { - flushProfilingData() - def res = container.execInContainer( - 'bash', '-c', - '''kill -9 `pgrep php-fpm`; - export DD_TRACE_RESOURCE_RENAMING_ENABLED=false; - php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''') - assert res.exitCode == 0 + new PhpFpm(container).restart([DD_TRACE_RESOURCE_RENAMING_ENABLED: 'false']) } } diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/NginxFpmTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/NginxFpmTests.groovy index 8b22701d5d6..606b091b8ed 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/NginxFpmTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/NginxFpmTests.groovy @@ -3,7 +3,10 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.docker.InspectContainerHelper +import com.datadog.appsec.php.docker.LogFile +import com.datadog.appsec.php.docker.PhpFpm import groovy.util.logging.Slf4j +import org.junit.jupiter.api.Assumptions import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test import org.junit.jupiter.api.condition.DisabledIf @@ -14,6 +17,7 @@ import java.net.http.HttpResponse import static com.datadog.appsec.php.integration.TestParams.getPhpVersion import static com.datadog.appsec.php.integration.TestParams.getVariant +import static java.net.http.HttpResponse.BodyHandlers.ofString @Testcontainers @Slf4j @@ -47,4 +51,72 @@ class NginxFpmTests implements CommonTests { } } + /** + * Regression test: OOM inside dd_entity_body_convert() (called from + * dd_request_shutdown() via _request_pack()) triggers zend_bailout(), which + * the per-module zend_try swallows, so the helper never gets RequestShutdown + * and sees an out-of-order RequestInit on the next request. + */ + @Test + void 'no unexpected RequestInit due to RSHUTDOWN OOM bail'() { + Assumptions.assumeTrue(System.getProperty('USE_HELPER_RUST') != null, + 'the C++ helper silently swallows out-of-order commands.') + // PHP 8.3 release only (zts already excluded at class level): the debug + // allocator's heap-protection turns the mid-allocation OOM bailout into + // a spurious "zend_mm_heap corrupted" SIGABRT that masks the real bug. + Assumptions.assumeTrue(phpVersion == '8.3' && !variant.contains('debug'), + 'requires a PHP 8.3 release build') + + // Drop the pool to a single worker so the OOM request and the follow-up + // land on the same FPM process / helper socket. + PhpFpm fpm = new PhpFpm(container) + fpm.backupPoolConfig() + + try { + fpm.setPoolValue('pm.max_children', '1') + fpm.reload() + + LogFile helperLog = new LogFile(container, 'helper.log') + helperLog.markEndPos() + + // Warm-up: establish the helper connection so it is in its outer loop + // waiting for request_init before the OOM request. + container.traceFromRequest('/hello.php') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + // Trigger Pattern B: rshutdown_oom.php sets a 32M memory_limit, pins + // ~28 MiB live in $GLOBALS, then emits ~400 KiB of JSON. During + // RSHUTDOWN, dd_request_shutdown()'s _request_pack() callback parses + // that body via dd_entity_body_convert(), overflowing the ceiling; + // zend_bailout() fires before _omsg_send(), so the socket is untouched. + container.traceFromRequest('/rshutdown_oom.php', ofString()) { + HttpResponse resp -> + // The script completes (OOM happens during RSHUTDOWN, after the + // response is on the wire). + assert resp.statusCode() == 200 + } + + // originally, the would actually fail here because the bailout during + // rshutdown would skip _reset_globals() and _cur_req_span would not be + // reset. This would either lead to a crash (502), or, if the same slot + // was used for the span in the next request, for the span to be + // prematurely deleted on the next request as _cur_req_span was being + // replaced + container.traceFromRequest('/hello.php') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + List lines = helperLog.linesSinceMark + assert !lines.any { it.contains('unexpected command RequestInit') } : + 'Error message found. Relevant helper log:\n' + + lines.findAll { + it.contains('unexpected command') || it.contains('error in request loop') + }.join('\n') + } finally { + fpm.restorePoolConfig() + fpm.reload() + } + } + } diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy index 51ade591d40..04ae7007b68 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/RoadRunnerTests.groovy @@ -3,6 +3,7 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.FailOnUnmatchedTraces import com.datadog.appsec.php.docker.InspectContainerHelper +import com.datadog.appsec.php.docker.LogFile import groovy.util.logging.Slf4j import org.junit.jupiter.api.Assumptions import org.junit.jupiter.api.BeforeAll @@ -62,8 +63,8 @@ class RoadRunnerTests implements WorkerStrategyTests { Assumptions.assumeTrue(System.getProperty('USE_HELPER_RUST') != null, 'This bug only manifests on the Rust helper (strict outer/inner loop state machine).') - long logOffset = (CONTAINER.execInContainer('bash', '-c', - 'wc -c < /tmp/logs/helper.log').stdout.trim() as long) + LogFile helperLog = new LogFile(CONTAINER, 'helper.log') + helperLog.markEndPos() // PostRespondLfiHandler sets a callback that calls fopen('../etc/passwd') // after respond() returns. By that point, request_shutdown has been sent @@ -78,15 +79,52 @@ class RoadRunnerTests implements WorkerStrategyTests { assert resp.statusCode() == 200 } - String helperLog = CONTAINER.execInContainer('bash', '-c', - "tail -c +${logOffset + 1} /tmp/logs/helper.log").stdout + List lines = helperLog.linesSinceMark + log.info("Helper log since offset:\n{}", lines.join('\n')) - log.info("Helper log since offset:\n{}", helperLog) + assert !lines.any { it.contains('unexpected command RequestExec') } : + "Helper received RequestExec in outer loop. " + + "Relevant log:\n" + + lines.findAll { + it.contains('unexpected command') || it.contains('error in request loop') + }.join('\n') + } + + /** + * Regression test for the AppSec helper "unexpected command RequestExec" bug, + * variant where the post-respond RequestExec sender is the user-tracking SDK + * (track_user_login_success -> dd_find_and_apply_verdict_for_user) rather than + * push_addresses. + */ + @Test + void 'no unexpected RequestExec in outer loop after post-respond track_user_login'() { + Assumptions.assumeTrue(System.getProperty('USE_HELPER_RUST') != null, + 'This bug only manifests on the Rust helper (strict outer/inner loop state machine).') + + LogFile helperLog = new LogFile(CONTAINER, 'helper.log') + helperLog.markEndPos() + + // PostRespondTrackUserHandler sets a callback that calls + // track_user_login_success() after respond() returns. By that point, + // request_shutdown has been sent via the response_committed hook. If + // dd_find_and_apply_verdict_for_user still reaches the helper (socket + // open, active=true), it sends RequestExec into the outer loop. + CONTAINER.traceFromRequest('/post-respond-track-user') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + // Follow-up request verifies the connection is still usable. + CONTAINER.traceFromRequest('/') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + + List lines = helperLog.linesSinceMark + log.info("Helper log since offset:\n{}", lines.join('\n')) - assert !helperLog.contains('unexpected command RequestExec') : + assert !lines.any { it.contains('unexpected command RequestExec') } : "Helper received RequestExec in outer loop. " + "Relevant log:\n" + - helperLog.readLines().findAll { + lines.findAll { it.contains('unexpected command') || it.contains('error in request loop') }.join('\n') } diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/SamplingTestsInFpm.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/SamplingTestsInFpm.groovy index 7fbea7b5ec3..6a7b8f643df 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/SamplingTestsInFpm.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/SamplingTestsInFpm.groovy @@ -1,6 +1,7 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer +import com.datadog.appsec.php.docker.PhpFpm import com.datadog.appsec.php.model.Trace import org.junit.jupiter.api.Test @@ -102,21 +103,11 @@ trait SamplingTestsInFpm { } void setSamplingPeriod(String period) { - flushProfilingData() - def res = container.execInContainer( - 'bash', '-c', - """kill -9 `pgrep php-fpm`; - export DD_API_SECURITY_SAMPLE_DELAY=$period; - php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini""") - assert res.exitCode == 0 + new PhpFpm(container).restart([DD_API_SECURITY_SAMPLE_DELAY: period]) } private void resetFpm() { - flushProfilingData() - container.execInContainer( - 'bash', '-c', - '''kill -9 `pgrep php-fpm`; - php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini''') + new PhpFpm(container).restart() } void flushProfilingData() { diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/ZtsGshutdownTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/ZtsGshutdownTests.groovy index 2552a9dfd84..a8fc83ceb00 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/ZtsGshutdownTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/ZtsGshutdownTests.groovy @@ -2,6 +2,7 @@ package com.datadog.appsec.php.integration import com.datadog.appsec.php.docker.AppSecContainer import com.datadog.appsec.php.docker.InspectContainerHelper +import com.datadog.appsec.php.docker.LogFile import groovy.util.logging.Slf4j import org.junit.jupiter.api.Test import org.junit.jupiter.api.condition.EnabledIf @@ -63,9 +64,8 @@ class ZtsGshutdownTests { @Test void 'no crash during GSHUTDOWN when MaxConnectionsPerChild 1 triggers ZTS worker lifecycle'() { - long errorLogOffset = (CONTAINER.execInContainer('sh', '-c', - 'stat -c %s /tmp/logs/apache2/error.log 2>/dev/null || echo 0') - .stdout.trim() as long) + LogFile errorLog = new LogFile(CONTAINER, 'apache2/error.log') + errorLog.markEndPos() ExecResult backupResult = CONTAINER.execInContainer('sh', '-c', 'cp /etc/apache2/mods-enabled/mpm_event.conf /etc/apache2/mods-enabled/mpm_event.conf.bak_zts') @@ -111,13 +111,11 @@ class ZtsGshutdownTests { // Additionally check Apache's error.log for crashes that generate SIGABRT // before a core dump can be written (e.g. Rust allocator panics on // poisoned memory). - ExecResult logCheck = CONTAINER.execInContainer('sh', '-c', - "tail -c +${errorLogOffset + 1} /tmp/logs/apache2/error.log") - String errorLog = logCheck.stdout ?: '' - assert !errorLog.contains('exit signal Aborted'): - "Apache worker exited via SIGABRT during GSHUTDOWN:\n" + errorLog - assert !errorLog.contains('exit signal Segmentation'): - "Apache worker segfaulted during GSHUTDOWN:\n" + errorLog + String errorLogText = errorLog.getTextSinceMark() + assert !errorLogText.contains('exit signal Aborted'): + "Apache worker exited via SIGABRT during GSHUTDOWN:\n" + errorLogText + assert !errorLogText.contains('exit signal Segmentation'): + "Apache worker segfaulted during GSHUTDOWN:\n" + errorLogText } finally { CONTAINER.execInContainer('sh', '-c', 'cp /etc/apache2/mods-enabled/mpm_event.conf.bak_zts' + diff --git a/appsec/tests/integration/src/test/www/_handlers/src/PostRespondTrackUserHandler.php b/appsec/tests/integration/src/test/www/_handlers/src/PostRespondTrackUserHandler.php new file mode 100644 index 00000000000..cfa55535915 --- /dev/null +++ b/appsec/tests/integration/src/test/www/_handlers/src/PostRespondTrackUserHandler.php @@ -0,0 +1,23 @@ + 'text/plain'], 'OK'); + } +} diff --git a/appsec/tests/integration/src/test/www/base/public/rshutdown_oom.php b/appsec/tests/integration/src/test/www/base/public/rshutdown_oom.php new file mode 100644 index 00000000000..3c565281b6f --- /dev/null +++ b/appsec/tests/integration/src/test/www/base/public/rshutdown_oom.php @@ -0,0 +1,66 @@ +addRoute('/json', new \App\JsonHandler()); $router->addRoute('/xml', new \App\XmlHandler()); $router->addRoute('/post-respond-lfi', new \App\PostRespondLfiHandler()); +$router->addRoute('/post-respond-track-user', new \App\PostRespondTrackUserHandler()); while ($req = $httpWorker->waitRequest()) { /** @var \Spiral\RoadRunner\Http\Request $req */ From 9cdff756695f62239aa0f0560a8cbbbb38f4f387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Fri, 29 May 2026 18:17:40 +0100 Subject: [PATCH 3/6] appsec: treat post-response framing errors as dd_network After a successful command exchange (e.g. request_init), the helper has entered its inner request loop and is waiting for the matching request_shutdown. If _imsg_destroy() reports a msgpack framing error at that point, the code was returning dd_error. dd_error does not trigger dd_helper_close_conn(), so the connection stays open while the helper is blocked in the inner loop. On the next request the extension sends request_init into the inner loop, which the Rust helper treats as an unexpected command and aborts. Return dd_network instead, which causes the caller to close and abandon the connection. This keeps helper and extension state in sync even in the presence of malformed trailers. Co-Authored-By: Claude Opus 4.8 --- appsec/src/extension/commands_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appsec/src/extension/commands_helpers.c b/appsec/src/extension/commands_helpers.c index f589b94d0b5..efc4eee3d5c 100644 --- a/appsec/src/extension/commands_helpers.c +++ b/appsec/src/extension/commands_helpers.c @@ -179,7 +179,7 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn, "Response message for %.*s does not have the expected form", NAME_L); - return dd_error; + return dd_network; } if (res != dd_success && res != dd_should_block && res != dd_should_redirect && res != dd_should_record) { From c263646b68eade1a295b01ec18ad763673ebe865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Tue, 9 Jun 2026 13:36:12 +0100 Subject: [PATCH 4/6] appsec: fix extension build and update tests - Link extension against pthreads (Threads::Threads) to resolve undefined symbol: pthread_once on some platforms - Fix CMake deprecation warning: BYPRODUCT -> BYPRODUCTS - Add DD_APPSEC_ENABLED=1 env to user_tracking set_user phpt tests - Update http_client_ip_generation_01 response format - Update rinit_fail_malformed_resp expected output: malformed responses now return dd_network, closing the connection (failed_count=1) --- appsec/cmake/ddtrace.cmake | 6 ++-- appsec/cmake/extension.cmake | 3 +- appsec/cmake/helper_rust.cmake | 2 +- appsec/src/extension/request_lifecycle.c | 32 +++++++++---------- .../http_client_ip_generation_01.phpt | 4 +-- .../extension/rinit_fail_malformed_resp.phpt | 14 +++----- .../user_tracking_block_from_set_user.phpt | 2 ++ ...ser_tracking_do_nothing_from_set_user.phpt | 2 ++ .../user_tracking_redirect_from_set_user.phpt | 2 ++ 9 files changed, 35 insertions(+), 32 deletions(-) diff --git a/appsec/cmake/ddtrace.cmake b/appsec/cmake/ddtrace.cmake index 24e3a093549..1b8f00985a1 100644 --- a/appsec/cmake/ddtrace.cmake +++ b/appsec/cmake/ddtrace.cmake @@ -20,14 +20,14 @@ add_custom_target(libdatadog_stamp "-DLIBDATADOG=${LIBDATADOG_DIR}" "-DSTAMP_FILE=${LIBDATADOG_STAMP_FILE}" -P "${CMAKE_CURRENT_LIST_DIR}/update_rust_stamp.cmake" - BYPRODUCT ${LIBDATADOG_STAMP_FILE} + BYPRODUCTS ${LIBDATADOG_STAMP_FILE} ) if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/ddtrace_exports.version") add_custom_target(ddtrace_exports COMMAND bash -c "{ echo -e '{\\nglobal:'; sed 's/$/;/' '${CMAKE_SOURCE_DIR}'/../ddtrace.sym; echo -e 'local:\\n*;\\n};'; } > '${EXPORTS_FILE}'" - BYPRODUCT ${EXPORTS_FILE} + BYPRODUCTS ${EXPORTS_FILE} DEPENDS ${CMAKE_SOURCE_DIR}/../ddtrace.sym VERBATIM ) @@ -35,7 +35,7 @@ elseif(APPLE) set(EXPORTS_FILE "${CMAKE_BINARY_DIR}/ddtrace_exports.sym") add_custom_target(ddtrace_exports COMMAND sed "s/^/_/" "${CMAKE_SOURCE_DIR}/../ddtrace.sym" > "${EXPORTS_FILE}" - BYPRODUCT ${EXPORTS_FILE} + BYPRODUCTS ${EXPORTS_FILE} DEPENDS ${CMAKE_SOURCE_DIR}/../ddtrace.sym ) endif() diff --git a/appsec/cmake/extension.cmake b/appsec/cmake/extension.cmake index b49af3a74b2..a3bbdf4fb97 100644 --- a/appsec/cmake/extension.cmake +++ b/appsec/cmake/extension.cmake @@ -68,7 +68,8 @@ if(ZAI_INCLUDE_DIRS) endif() target_link_libraries(extension PRIVATE zai) -target_link_libraries(extension PRIVATE mpack PhpConfig zai rapidjson_appsec libxml2_static PCRE2::pcre2) +find_package(Threads REQUIRED) +target_link_libraries(extension PRIVATE mpack PhpConfig zai rapidjson_appsec libxml2_static PCRE2::pcre2 Threads::Threads) target_include_directories(extension PRIVATE ${EXT_ROOT_INCLUDES}) # gnu unique prevents shared libraries from being unloaded from memory by dlclose diff --git a/appsec/cmake/helper_rust.cmake b/appsec/cmake/helper_rust.cmake index 678c8480ce1..5c97046c2be 100644 --- a/appsec/cmake/helper_rust.cmake +++ b/appsec/cmake/helper_rust.cmake @@ -8,7 +8,7 @@ add_custom_target(helper-rust-stamp "-DHELPER_RUST_DIR=${HELPER_RUST_DIR}" "-DSTAMP_FILE=${HELPER_RUST_STAMP_FILE}" -P "${CMAKE_CURRENT_LIST_DIR}/update_helper_rust_stamp.cmake" - BYPRODUCT ${HELPER_RUST_STAMP_FILE} + BYPRODUCTS ${HELPER_RUST_STAMP_FILE} ) set(CARGO_BUILD_CMD "cargo build") diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index c627ac2ee0b..7d2a7898ad4 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -292,13 +292,18 @@ void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) __auto_type orig_mem_limit = PG(memory_limit); zend_set_memory_limit((size_t)Z_L(-1) >> 1); - zend_try { - _do_req_lifecycle_rshutdown(ignore_verdict, force); - } zend_catch { + zend_try { _do_req_lifecycle_rshutdown(ignore_verdict, force); } + zend_catch + { if (PG(last_error_message)) { +#if PHP_VERSION_ID < 70100 + const char *last_err = PG(last_error_message); +#else + const char *last_err = ZSTR_VAL(PG(last_error_message)); +#endif mlog_g(dd_log_error, "Bailout in request shutdown; disconnecting from helper: %s", - ZSTR_VAL(PG(last_error_message))); + last_err); } else { mlog_g(dd_log_error, "Bailout in request shutdown; disconnecting from helper"); @@ -306,12 +311,13 @@ void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) _reset_globals(); dd_helper_close_conn(); // note: not completely bailout-safe, // but should be fine with the raised mem limit - } zend_end_try(); + } + zend_end_try(); __auto_type res = zend_set_memory_limit(orig_mem_limit); if (res == FAILURE) { - mlog(dd_log_error, - "Failed to restore memory limit in request shutdown"); + mlog( + dd_log_error, "Failed to restore memory limit in request shutdown"); } } @@ -525,14 +531,10 @@ static void _set_cur_span(zend_object *nullable span) } bool dd_req_lifecycle_is_active(void) -{ - return _between_init_shutdown_msgs && DDAPPSEC_G(active); -} +{ return _between_init_shutdown_msgs && DDAPPSEC_G(active); } zend_object *nullable dd_req_lifecycle_get_cur_span(void) -{ - return _cur_req_span; -} +{ return _cur_req_span; } zend_string *nullable dd_req_lifecycle_get_client_ip(void) { @@ -989,9 +991,7 @@ static inline uint64_t _hash_string( } static inline uint64_t _hash_zend_string( uint64_t hash, zend_string *nonnull str) -{ - return _hash_string(hash, ZSTR_VAL(str), ZSTR_LEN(str)); -} +{ return _hash_string(hash, ZSTR_VAL(str), ZSTR_LEN(str)); } static uint64_t _calc_sampling_key(zend_object *root_span, int status_code) { diff --git a/appsec/tests/extension/http_client_ip_generation_01.phpt b/appsec/tests/extension/http_client_ip_generation_01.phpt index d45d7f22185..2f7bdcb3088 100644 --- a/appsec/tests/extension/http_client_ip_generation_01.phpt +++ b/appsec/tests/extension/http_client_ip_generation_01.phpt @@ -22,10 +22,10 @@ include __DIR__ . '/inc/mock_helper.php'; $helper = Helper::createInitedRun([ response_list( - response_request_init(['record', ['{"found":"attack"}','{"another":"attack"}']]) + response_request_init([[['record', []]], ['{"found":"attack"}','{"another":"attack"}']]) ), response_list( - response_request_shutdown(['record', ['{"yet another":"attack"}'], ["rshutdown_tag" => "rshutdown_value"], ["rshutdown_metric" => 2.1]]) + response_request_shutdown([[['record', []]], ['{"yet another":"attack"}'], false, [], ["rshutdown_tag" => "rshutdown_value"], ["rshutdown_metric" => 2.1]]) ), ], ['continuous' => true]); diff --git a/appsec/tests/extension/rinit_fail_malformed_resp.phpt b/appsec/tests/extension/rinit_fail_malformed_resp.phpt index 79422e76aaf..753e2a5ff7e 100644 --- a/appsec/tests/extension/rinit_fail_malformed_resp.phpt +++ b/appsec/tests/extension/rinit_fail_malformed_resp.phpt @@ -8,20 +8,16 @@ use function datadog\appsec\testing\{rinit,rshutdown,backoff_status,is_connected include __DIR__ . '/inc/mock_helper.php'; -// respond correctly to client_init and request_shutdown, but not request_init -$obj = new ArrayObject(); +// The malformed request_init response triggers dd_network, closing the connection. +// rshutdown won't communicate with the helper. $helper = Helper::createInitedRun([ response_list( response_request_init([['foo' => 'ok']]) ), - response_list( - response_request_shutdown([[['ok', []]], $obj, $obj]) - ) ]); echo "rinit:\n"; var_dump(rinit()); -// connection wasn't closed because this was no dd_network error. rshutdown will succeed echo "rshutdown:\n"; var_dump(rshutdown()); echo "is connected:\n"; @@ -37,10 +33,10 @@ bool(true) rshutdown: bool(true) is connected: -bool(true) +bool(false) array(2) { ["failed_count"]=> - int(0) + int(1) ["next_retry"]=> - float(0) + float(%f) } diff --git a/appsec/tests/extension/user_tracking_block_from_set_user.phpt b/appsec/tests/extension/user_tracking_block_from_set_user.phpt index 849a9eac444..f84917c2eb8 100644 --- a/appsec/tests/extension/user_tracking_block_from_set_user.phpt +++ b/appsec/tests/extension/user_tracking_block_from_set_user.phpt @@ -2,6 +2,8 @@ Block from DDTrace\set_user --INI-- extension=ddtrace.so +--ENV-- +DD_APPSEC_ENABLED=1 --FILE-- Date: Tue, 9 Jun 2026 14:04:49 +0100 Subject: [PATCH 5/6] appsec: fix PHP version compatibility in rshutdown bailout handler - PG(last_error_message) is char* in all PHP 7.x; became zend_string* only in PHP 8.0. Fix the version guard from < 70100 to < 80000. - zend_set_memory_limit() returns void in PHP 8.0 (changed back to bool in PHP 8.1). Guard the return-value check accordingly. - Expand single-line function bodies to satisfy clang-format. Co-Authored-By: Claude Sonnet 4.6 --- appsec/src/extension/request_lifecycle.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index 39a9a6d98c3..6ec2f9abb75 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -296,7 +296,7 @@ void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) zend_catch { if (PG(last_error_message)) { -#if PHP_VERSION_ID < 70100 +#if PHP_VERSION_ID < 80000 const char *last_err = PG(last_error_message); #else const char *last_err = ZSTR_VAL(PG(last_error_message)); @@ -314,11 +314,15 @@ void dd_req_lifecycle_rshutdown(bool ignore_verdict, bool force) } zend_end_try(); - __auto_type res = zend_set_memory_limit(orig_mem_limit); - if (res == FAILURE) { +#if PHP_VERSION_ID >= 80000 && PHP_VERSION_ID < 80100 + // PHP 8.0: zend_set_memory_limit returns void + zend_set_memory_limit(orig_mem_limit); +#else + if (zend_set_memory_limit(orig_mem_limit) == FAILURE) { mlog( dd_log_error, "Failed to restore memory limit in request shutdown"); } +#endif } static void _do_req_lifecycle_rshutdown(bool ignore_verdict, bool force) From a2810b978dbc4854bde8cbe60de00e5abf11ff30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Tue, 9 Jun 2026 14:12:38 +0100 Subject: [PATCH 6/6] appsec: expand single-line function bodies to satisfy clang-format-20 CI uses clang-format-20 which does not allow single-line function bodies even when AllowShortBlocksOnASingleLine is true. Expand three offending functions to multi-line form. Co-Authored-By: Claude Sonnet 4.6 --- appsec/src/extension/request_lifecycle.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/appsec/src/extension/request_lifecycle.c b/appsec/src/extension/request_lifecycle.c index 6ec2f9abb75..9e32129b87e 100644 --- a/appsec/src/extension/request_lifecycle.c +++ b/appsec/src/extension/request_lifecycle.c @@ -535,10 +535,14 @@ static void _set_cur_span(zend_object *nullable span) } bool dd_req_lifecycle_is_active(void) -{ return _between_init_shutdown_msgs && DDAPPSEC_G(active); } +{ + return _between_init_shutdown_msgs && DDAPPSEC_G(active); +} zend_object *nullable dd_req_lifecycle_get_cur_span(void) -{ return _cur_req_span; } +{ + return _cur_req_span; +} zend_string *nullable dd_req_lifecycle_get_client_ip(void) { @@ -995,7 +999,9 @@ static inline uint64_t _hash_string( } static inline uint64_t _hash_zend_string( uint64_t hash, zend_string *nonnull str) -{ return _hash_string(hash, ZSTR_VAL(str), ZSTR_LEN(str)); } +{ + return _hash_string(hash, ZSTR_VAL(str), ZSTR_LEN(str)); +} static uint64_t _calc_sampling_key(zend_object *root_span, int status_code) {