diff --git a/fuzz/fuzz_stubs.c b/fuzz/fuzz_stubs.c index a708bca1..2dfd6262 100644 --- a/fuzz/fuzz_stubs.c +++ b/fuzz/fuzz_stubs.c @@ -18,6 +18,17 @@ #include "php.h" #include "http1/http_parser.h" /* http_request_t layout + HTTP_HEADERS_INITIAL_SIZE */ +#include + +/* Parser fuzzing never prepares a PHP response. Fail if it reaches dispatch + * instead of silently satisfying the new shared helper's response hook. */ +__attribute__((weak)) void http_response_set_default_json_flags(zend_object *obj, + uint32_t flags) +{ + (void)obj; + (void)flags; + abort(); +} /* Extension class entries (normally populated at MINIT). */ zend_class_entry *http_exception_ce __attribute__((weak)) = NULL; diff --git a/include/http1/http_parser.h b/include/http1/http_parser.h index eed378f8..3f346137 100644 --- a/include/http1/http_parser.h +++ b/include/http1/http_parser.h @@ -187,7 +187,7 @@ struct http_request_t { void (*release)(struct http_request_t *req); /* W3C Trace Context. Populated by http_request_parse_trace_context - * at on_headers_complete iff server has telemetry enabled and the + * at headers-complete or dispatch iff server has telemetry enabled and the * request carried a valid traceparent. trace_id == 16 zero bytes * means "absent". traceparent_raw / tracestate_raw retain the raw * header strings for HttpRequest::getTraceparent etc. */ diff --git a/src/core/http_connection.c b/src/core/http_connection.c index 7f6b8ae0..7977e9b5 100644 --- a/src/core/http_connection.c +++ b/src/core/http_connection.c @@ -2894,21 +2894,8 @@ static void http_connection_dispatch_request(http_connection_t *conn, http_reque http_response_install_stream_ops(Z_OBJ(ctx->response_zv), &h1_stream_ops, ctx); -#ifdef HAVE_HTTP_COMPRESSION - /* Attach compression state (issue #8). conn->config is cached at - * bind time so this hot path is a single load + null-check. */ - if (conn->config != NULL) { - extern void http_compression_attach(zend_object *, - http_request_t *, http_server_config_t *); - http_compression_attach(Z_OBJ(ctx->response_zv), req, conn->config); - } -#endif - /* Wire the per-request JSON encode default — used by - * HttpResponse::json() when the per-call $flags arg is 0. */ - if (conn->config != NULL) { - http_response_set_default_json_flags( - Z_OBJ(ctx->response_zv), conn->config->json_encode_flags); - } + http_request_prepare_dispatch(req, Z_OBJ(ctx->response_zv), conn->config, + conn->view->telemetry_enabled); conn->state = CONN_STATE_PROCESSING; diff --git a/src/core/http_protocol_handlers.c b/src/core/http_protocol_handlers.c index b2a178aa..08e40a08 100644 --- a/src/core/http_protocol_handlers.c +++ b/src/core/http_protocol_handlers.c @@ -15,6 +15,29 @@ #include "http1/http_parser.h" /* http_request_t */ #include "grpc/grpc.h" /* grpc_content_type_mode */ #include "formats/form_content_type.h" +#include "log/trace_context.h" +#ifdef HAVE_HTTP_COMPRESSION +# include "compression/http_compression_response.h" +#endif + +void http_request_prepare_dispatch(http_request_t *req, + zend_object *response, + http_server_config_t *config, + bool telemetry_enabled) +{ + /* H1/H2 also parse at headers-complete so WebSocket upgrade handlers can + * read the context. Do not retain the raw header strings a second time. */ + if (telemetry_enabled && !req->has_trace) { + http_request_parse_trace_context(req); + } + + if (config != NULL) { +#ifdef HAVE_HTTP_COMPRESSION + http_compression_attach(response, req, config); +#endif + http_response_set_default_json_flags(response, config->json_encode_flags); + } +} /* {{{ http_protocol_type_to_string */ const char* http_protocol_type_to_string(http_protocol_type_t type) diff --git a/src/core/http_protocol_handlers.h b/src/core/http_protocol_handlers.h index 9ccf97ef..c0890141 100644 --- a/src/core/http_protocol_handlers.h +++ b/src/core/http_protocol_handlers.h @@ -48,6 +48,15 @@ zend_fcall_t *http_protocol_pick_handler(HashTable *handlers, * and read the body policy through the predicates below. */ void http_request_classify_protocols(struct http_request_t *req); +/* Shared preparation before a PHP handler or static handler sees the request + * and response. Use the dispatch thread's config (the worker's clone in pool + * mode). Trace context may already have been parsed at headers-complete for + * WebSocket upgrades, which bypass the normal HTTP dispatch. */ +void http_request_prepare_dispatch(struct http_request_t *req, + zend_object *response, + http_server_config_t *config, + bool telemetry_enabled); + /* Body policy derived from the stamped grpc_mode and form_kind, keeping * transports gRPC-agnostic. must_buffer: never stream (grpc-web-text decodes * the whole body, and a form getter reads the whole body). size_uncapped: diff --git a/src/core/worker_dispatch.c b/src/core/worker_dispatch.c index 609a1927..716ff290 100644 --- a/src/core/worker_dispatch.c +++ b/src/core/worker_dispatch.c @@ -924,12 +924,8 @@ bool worker_dispatch_request(http_server_object *server, * one is encoded when worker_render_response flattens it. */ http_server_config_t *const cfg = http_server_get_config(server); - if (cfg != NULL) { -#ifdef HAVE_HTTP_COMPRESSION - http_compression_attach(Z_OBJ(ctx->response_zv), req, cfg); -#endif - http_response_set_default_json_flags(Z_OBJ(ctx->response_zv), cfg->json_encode_flags); - } + http_request_prepare_dispatch(req, Z_OBJ(ctx->response_zv), cfg, + http_server_view(server)->telemetry_enabled); if (is_grpc) { grpc_call_init_response(Z_OBJ(ctx->response_zv), grpc_mode); diff --git a/src/http2/http2_strategy.c b/src/http2/http2_strategy.c index 9f86bb70..d3564dd7 100644 --- a/src/http2/http2_strategy.c +++ b/src/http2/http2_strategy.c @@ -289,21 +289,9 @@ static void http2_strategy_dispatch(struct http_request_t *request, http_response_install_stream_ops(Z_OBJ(stream->response_zv), &h2_stream_ops, stream); -#ifdef HAVE_HTTP_COMPRESSION - /* Attach compression state (issue #8). Mirror of the H1 dispatch - * hook — uses conn->config cached at bind time. */ - if (self->conn->config != NULL) { - extern void http_compression_attach(zend_object *, - http_request_t *, http_server_config_t *); - http_compression_attach(Z_OBJ(stream->response_zv), - stream->request, self->conn->config); - } -#endif - /* Per-request JSON encode default for HttpResponse::json(). */ - if (self->conn->config != NULL) { - http_response_set_default_json_flags( - Z_OBJ(stream->response_zv), self->conn->config->json_encode_flags); - } + http_request_prepare_dispatch(stream->request, Z_OBJ(stream->response_zv), + self->conn->config, + self->conn->view->telemetry_enabled); if (is_grpc) { grpc_call_init_response(Z_OBJ(stream->response_zv), grpc_mode); diff --git a/src/http3/http3_dispatch.c b/src/http3/http3_dispatch.c index b07c7f82..5ad49d28 100644 --- a/src/http3/http3_dispatch.c +++ b/src/http3/http3_dispatch.c @@ -21,7 +21,6 @@ #include "core/bailout_guard.h" #include "http3_listener.h" /* http3_listener_server_obj */ #include "http3/http3_stream.h" /* http3_stream_t */ -#include "log/trace_context.h" #include "http_connection.h" /* http_handler_log_bailout */ #include "http_send_file.h" /* http_send_file_dispatch */ #include "http_response_internal.h" /* http_response_has/take_send_file */ @@ -623,10 +622,6 @@ void http3_stream_dispatch(http3_connection_t *c, http3_stream_t *s) s->dispatched = true; s->conn = c; - if (http_server_view(server)->telemetry_enabled) { - http_request_parse_trace_context(s->request); - } - http_request_addref(s->request); zval *req_obj = http_request_create_from_parsed(s->request); @@ -656,23 +651,9 @@ void http3_stream_dispatch(http3_connection_t *c, http3_stream_t *s) grpc_call_init_response(Z_OBJ(s->response_zv), grpc_mode); } - /* Attach compression state and the JSON default. Server pointer comes - * from the listener — same pattern that http3_handler_coroutine uses - * for the request-sample bookkeeping. */ - { - http_server_object *srv = - (http_server_object *)http3_listener_server_obj(c->listener); - http_server_config_t *cfg = http_server_get_config(srv); - - if (cfg != NULL) { -#ifdef HAVE_HTTP_COMPRESSION - http_compression_attach(Z_OBJ(s->response_zv), - s->request, cfg); -#endif - http_response_set_default_json_flags( - Z_OBJ(s->response_zv), cfg->json_encode_flags); - } - } + http_request_prepare_dispatch(s->request, Z_OBJ(s->response_zv), + http_server_get_config(server), + http_server_view(server)->telemetry_enabled); /* Static-handler dispatch. Same policy as the H1/H2 * sites: diff --git a/tests/phpt/server/h3/081-h3-pool-trace-context.phpt b/tests/phpt/server/h3/081-h3-pool-trace-context.phpt new file mode 100644 index 00000000..b76432e9 --- /dev/null +++ b/tests/phpt/server/h3/081-h3-pool-trace-context.phpt @@ -0,0 +1,126 @@ +--TEST-- +HttpServer: pooled HTTP/3 preserves trace context in the handler and access log (#352) +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- + "00-$tid-$sid-01", + 'unsampled' => "00-$tid-$sid-00", + 'upper' => strtoupper("00-$tid-$sid-01"), + 'zero' => "00-00000000000000000000000000000000-$sid-01", + 'absent' => null, +]; + +foreach (['enabled' => true, 'disabled' => false] as $mode => $enabled) { + $port = tas_free_port_span(2); + $log = "$tmp/$mode.log"; + $config = (new HttpServerConfig()) + ->addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate($cert)->setPrivateKey($key) + ->setWorkers(2) + ->setTelemetryEnabled($enabled) + ->setLogSinks([ + ['type' => 'file', 'path' => $log, 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO], + ]); + $server = new HttpServer($config); + $server->addHttpHandler(function ($req, $res) { + $res->json([ + $req->getTraceParent(), $req->getTraceState(), + $req->getTraceId(), $req->getSpanId(), $req->getTraceFlags(), + ]); + }); + + spawn(function () use ($server, $port, $client, $tmp, $mode, $enabled, $cases, $tid, $sid) { + delay(600); + foreach ($cases as $name => $tp) { + $header = $tp === null ? '' : ' H3CLIENT_HEADER=' . escapeshellarg("traceparent: $tp"); + $cmd = 'H3CLIENT_DEADLINE_MS=4000' . $header . ' ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /' . $name + . ' 2>' . escapeshellarg("$tmp/stderr.txt"); + $body = json_decode((string) shell_exec($cmd), true); + $valid = $enabled && ($name === 'valid' || $name === 'unsampled'); + $want = $valid ? [$tp, null, $tid, $sid, $name === 'valid' ? 1 : 0] + : [null, null, null, null, null]; + echo "$mode $name handler: ", $body === $want ? 'ok' : 'FAIL', "\n"; + } + $server->stop(); + }); + $server->start(); + + /* stop() drains the worker access sinks before we read the records. */ + $records = []; + foreach (explode("\n", trim((string) file_get_contents($log))) as $line) { + $r = json_decode($line, true); + $path = $r['Attributes']['url.path'] ?? ''; + if (array_key_exists(ltrim($path, '/'), $cases)) { + $records[$path][] = $r; + } + } + foreach ($cases as $name => $tp) { + $rows = $records['/' . $name] ?? []; + $r = $rows[0] ?? []; + $valid = $enabled && ($name === 'valid' || $name === 'unsampled'); + $ok = count($rows) === 1 && ($r['Attributes']['network.protocol.version'] ?? '') === '3' + && ($valid ? (($r['TraceId'] ?? '') === $tid && ($r['SpanId'] ?? '') === $sid) + : (!isset($r['TraceId']) && !isset($r['SpanId']))); + echo "$mode $name log: ", $ok ? 'ok' : 'FAIL', "\n"; + } +} +echo "done\n"; +?> +--EXPECTF-- +%Aenabled valid handler: ok +enabled unsampled handler: ok +enabled upper handler: ok +enabled zero handler: ok +enabled absent handler: ok +%Aenabled valid log: ok +enabled unsampled log: ok +enabled upper log: ok +enabled zero log: ok +enabled absent log: ok +%Adisabled valid handler: ok +disabled unsampled handler: ok +disabled upper handler: ok +disabled zero handler: ok +disabled absent handler: ok +%Adisabled valid log: ok +disabled unsampled log: ok +disabled upper log: ok +disabled zero log: ok +disabled absent log: ok +done diff --git a/tests/unit/common/request_stubs.c b/tests/unit/common/request_stubs.c index 7c5889c6..19c66fd6 100644 --- a/tests/unit/common/request_stubs.c +++ b/tests/unit/common/request_stubs.c @@ -1,12 +1,12 @@ /* - * Stubs for the request-finalize tail of http_request.c + * Stubs for request finalization and response preparation * * The parser targets drive bytes through the parser and stop there: nothing in - * them finalizes a request, so the access-log tail is never reached. These - * exist to satisfy the linker without dragging in the log rings and the - * response object. Reaching one means the test is exercising a path this - * target does not build, and an answer invented here would make it pass - * anyway — so they abort instead. + * them finalizes or dispatches a request, so the access-log tail and response + * preparation are never reached. These exist to satisfy the linker without + * dragging in the log rings and the response object. Reaching one means the + * test is exercising a path this target does not build, and an answer invented + * here would make it pass anyway — so they abort instead. */ #include "php.h" @@ -34,6 +34,12 @@ uint64_t http_response_get_sent_body_size(zend_object *obj) abort(); } +void http_response_set_default_json_flags(zend_object *obj, uint32_t flags) +{ + (void) obj; (void) flags; + abort(); +} + size_t http_response_get_body_len(zend_object *obj) { (void) obj; diff --git a/tests/unit/common/strategy_stubs.c b/tests/unit/common/strategy_stubs.c index c75c88f7..2e4ba091 100644 --- a/tests/unit/common/strategy_stubs.c +++ b/tests/unit/common/strategy_stubs.c @@ -108,12 +108,6 @@ bool http_response_header_allowed_h2h3(const char *name, size_t len, abort(); } -void http_response_set_default_json_flags(zend_object *obj, uint32_t flags) -{ - (void) obj; (void) flags; - abort(); -} - void http_response_set_head(zend_object *obj, bool is_head) { (void) obj; (void) is_head;