Skip to content

Commit

Permalink
Fix blocking on PHP 7.0-7.1 ZTS
Browse files Browse the repository at this point in the history
  • Loading branch information
cataphract committed Feb 2, 2024
1 parent 679da8e commit 6965136
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion appsec/src/extension/commands/request_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data)

struct ctx ctx = {.data = data};

return dd_command_exec(conn, &_spec, &ctx);
return dd_command_exec_req_info(conn, &_spec, &ctx.req_info);
}

static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull _ctx)
Expand Down
2 changes: 1 addition & 1 deletion appsec/src/extension/commands/request_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static const dd_command_spec _spec = {
dd_result dd_request_init(
dd_conn *nonnull conn, struct req_info_init *nonnull ctx)
{
return dd_command_exec(conn, &_spec, ctx);
return dd_command_exec_req_info(conn, &_spec, &ctx->req_info);
}

static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull _ctx)
Expand Down
2 changes: 1 addition & 1 deletion appsec/src/extension/commands/request_shutdown.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static const dd_command_spec _spec = {
dd_result dd_request_shutdown(
dd_conn *nonnull conn, struct req_shutdown_info *nonnull req_info)
{
return dd_command_exec(conn, &_spec, req_info);
return dd_command_exec_req_info(conn, &_spec, &req_info->req_info);
}

static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull ctx)
Expand Down
1 change: 1 addition & 0 deletions appsec/src/extension/commands_ctx.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <php.h>

struct req_info {
const char *nullable command_name; // for logging
zend_object *nullable root_span;
zend_string *nullable client_ip;
};
12 changes: 11 additions & 1 deletion appsec/src/extension/commands_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ dd_result ATTR_WARN_UNUSED dd_command_exec(dd_conn *nonnull conn,
return _dd_command_exec(conn, false, spec, ctx);
}

dd_result ATTR_WARN_UNUSED dd_command_exec_req_info(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, struct req_info *nonnull ctx)
{
ctx->command_name = spec->name;
return _dd_command_exec(conn, false, spec, ctx);
}

dd_result ATTR_WARN_UNUSED dd_command_exec_cred(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, void *unspecnull ctx)
{
Expand Down Expand Up @@ -361,6 +368,8 @@ static void _command_process_block_parameters(mpack_node_t root)
}
}

mlog(dd_log_debug, "Blocking parameters: status_code=%d, type=%d",
status_code, type);
dd_set_block_code_and_type(status_code, type);
}

Expand Down Expand Up @@ -425,7 +434,8 @@ dd_result dd_command_proc_resp_verd_span_data(
if (verd_len > INT_MAX) {
verd_len = INT_MAX;
}
mlog(dd_log_debug, "Verdict of request_init was '%.*s'", (int)verd_len,
mlog(dd_log_debug, "Verdict of %s was '%.*s'",
ctx->command_name ? ctx->command_name : "(unknown)", (int)verd_len,
verd_str);
}

Expand Down
3 changes: 3 additions & 0 deletions appsec/src/extension/commands_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ typedef struct _dd_command_spec {
dd_result ATTR_WARN_UNUSED dd_command_exec(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, void *unspecnull ctx);

dd_result ATTR_WARN_UNUSED dd_command_exec_req_info(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, struct req_info *nonnull ctx);

dd_result ATTR_WARN_UNUSED dd_command_exec_cred(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, void *unspecnull ctx);

Expand Down
8 changes: 8 additions & 0 deletions appsec/src/extension/php_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ static zend_always_inline zend_string *zend_string_init_interned(
const char *str, size_t len, int persistent)
{
zend_string *ret = zend_string_init(str, len, persistent);
# ifdef ZTS
// believe it or not zend_new_interned_string() is an identity function
// set the interned flag manually so zend_string_release() is a no-op
GC_FLAGS(ret) |= IS_STR_INTERNED;
zend_string_hash_val(ret);
return ret;
# else
return zend_new_interned_string(ret);
# endif
}
#endif

Expand Down
20 changes: 15 additions & 5 deletions appsec/src/extension/request_abort.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ static void _set_content_type(const char *nonnull content_type)
static void _set_output(const char *nonnull output, size_t length)
{
size_t written = php_output_write(output, length);
mlog_g(dd_log_debug, "php_output_write() returned %zu", written);
if (written != length) {
mlog(dd_log_info, "could not write full response (written: %zu)",
written);
Expand All @@ -146,8 +147,8 @@ static dd_response_type _get_response_type_from_accept_header(
dd_php_get_string_elem_cstr(_server, LSTRARG("HTTP_ACCEPT"));
if (!accept_zstr) {
mlog(dd_log_info,
"Could not find Accept header, using default content-type");
goto exit;
"Could not find Accept header, using default content-type (json)");
return response_type_json;
}

const char *accept_end = ZSTR_VAL(accept_zstr) + ZSTR_LEN(accept_zstr);
Expand All @@ -172,7 +173,7 @@ static dd_response_type _get_response_type_from_accept_header(
return response_type_html;
}

exit:
mlog_g(dd_log_debug, "No recognized accept header, defaulting to json");
return response_type_json;
}

Expand Down Expand Up @@ -214,13 +215,17 @@ void dd_request_abort_redirect()
}

if (!_abort_prelude()) {
mlog(dd_log_debug, "_abort_prelude has failed");
return;
}

char *line;
uint line_len = (uint)spprintf(
&line, 0, "Location: %s", ZSTR_VAL(_redirection_location));

mlog_g(dd_log_debug, "Will forward to %s with status %d",
ZSTR_VAL(_redirection_location), _redirection_response_code);

SG(sapi_headers).http_response_code = _redirection_response_code;
int res = sapi_header_op(SAPI_HEADER_REPLACE,
&(sapi_header_line){.line = line, .line_len = line_len});
Expand All @@ -232,7 +237,7 @@ void dd_request_abort_redirect()
efree(line);

if (sapi_flush() == SUCCESS) {
mlog(dd_log_debug, "Successful call to sapi_flush()");
mlog_g(dd_log_debug, "Successful call to sapi_flush()");
} else {
mlog(dd_log_warning, "Call to sapi_flush() failed");
}
Expand Down Expand Up @@ -299,6 +304,7 @@ void _request_abort_static_page(int response_code, int type)
}

if (!_abort_prelude()) {
mlog(dd_log_debug, "_abort_prelude has failed");
zend_string_release(body);
return;
}
Expand Down Expand Up @@ -434,8 +440,10 @@ static void _suppress_error_reporting(void);
ATTR_FORMAT(1, 2)
static void _emit_error(const char *format, ...)
{
va_list args;
mlog_g(dd_log_debug, "_emit_error() called: during_request_startup: %d",
PG(during_request_startup));

va_list args;
va_start(args, format);
if (PG(during_request_startup)) {
/* if emitting error during startup, RSHUTDOWN will not run (except fpm)
Expand Down Expand Up @@ -615,6 +623,7 @@ static zend_string *nonnull _get_json_blocking_template()
return _empty_zstr;
}
if (ZSTR_LEN(body_error_json) == 0) {
zend_string_release(body_error_json);
return _body_error_json_def;
}

Expand All @@ -635,6 +644,7 @@ static zend_string *nonnull _get_html_blocking_template()
return _empty_zstr;
}
if (ZSTR_LEN(body_error_html) == 0) {
zend_string_release(body_error_html);
return _body_error_html_def;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ enable_extensions.sh
php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini
service apache2 start

exec tail -F "${LOGS_PHP[@]}" "${LOGS_APACHE[@]}"
exec tail -n +1 -F "${LOGS_PHP[@]}" "${LOGS_APACHE[@]}"
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ enable_extensions.sh

service apache2 start

exec tail -F "${LOGS_PHP[@]}" "${LOGS_APACHE[@]}"
exec tail -n +1 -F "${LOGS_PHP[@]}" "${LOGS_APACHE[@]}"
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ enable_extensions.sh
php-fpm -y /etc/php-fpm.conf -c /etc/php/php.ini
service nginx start

exec tail -F "${LOGS_PHP[@]}" "${LOGS_NGINX[@]}"
exec tail -n +1 -F "${LOGS_PHP[@]}" "${LOGS_NGINX[@]}"
2 changes: 1 addition & 1 deletion appsec/tests/integration/src/test/www/roadrunner/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ echo datadog.trace.cli_enabled=true >> /etc/php/php.ini

./rr serve 2>&1 >> /tmp/logs/rr.log &

tail -f "${LOGS_PHP[@]}"
tail -n +1 -F "${LOGS_PHP[@]}"

0 comments on commit 6965136

Please sign in to comment.