0.34.x

[loks0n]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d29c187e-8381-420b-ab45-9fec0cd3a823

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 0.34.x

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR refactors utopia-php/http from 0.33.x to 0.34.x by splitting the previously monolithic Request/Response classes into adapter-specific implementations (FPM, Swoole, SwooleCoroutine), introducing per-request DI container scoping via Swoole coroutine context, and adding OpenTelemetry-aligned HTTP server metrics (request duration, active requests, request/response body size).

Key changes:

• Request and Response are now abstract base classes; concrete adapters live under src/Http/Adapter/{FPM,Swoole,SwooleCoroutine}/.
• Per-request DI containers are created on each Swoole request and stored in coroutine context (REQUEST_CONTAINER_CONTEXT_KEY), preventing cross-request resource bleed.
• Http::run() wraps runInternal() with telemetry instrumentation using four OTel instruments aligned to the HTTP semantic conventions.
• Http::onStart() and Http::onRequest() hook types are added.

Issues found:

• src/Http/Adapter/Swoole/Request.php line 209: getReferer() hardcodes '' as the fallback instead of forwarding the caller's $default, making all non-default callers silently receive an empty string. SwooleCoroutine\\Request inherits this bug.
• src/Http/Http.php lines 822-848: The activeRequests counter increment in run() is not guarded by try/finally. If runInternal() throws (e.g. when an error handler itself re-throws), the matching decrement is skipped, permanently over-counting the gauge.

Confidence Score: 3/5

Two P1 defects should be fixed before merging: a broken getReferer default in the Swoole adapter and an unguarded telemetry counter that can permanently over-count active requests.

The PR introduces two newly-found P1 issues on top of the ones noted in prior review threads. The getReferer bug silently drops caller-supplied defaults for all Swoole requests, and the missing try/finally in Http::run() means the active_requests gauge leaks whenever an error handler re-throws. Both are straightforward fixes, but they are present defects on changed paths.

src/Http/Http.php (telemetry try/finally) and src/Http/Adapter/Swoole/Request.php (getReferer default forwarding) need attention before merge.

Vulnerabilities

No security concerns identified. Trusted-IP-header validation (allowlist + FILTER_VALIDATE_IP) is correctly preserved in both the FPM and Swoole adapters. Per-request DI container isolation prevents cross-request data leakage in Swoole. No secrets are introduced or exposed.

Important Files Changed

Filename	Overview
src/Http/Http.php	Core HTTP dispatcher with new telemetry instrumentation; activeRequests counter can permanently leak if runInternal throws, and getSize() always reads from php://input which is empty under Swoole.
src/Http/Adapter/Swoole/Request.php	New Swoole request adapter; getReferer() hardcodes an empty-string fallback instead of forwarding the caller's $default argument.
src/Http/Adapter/Swoole/Server.php	New process-mode Swoole server adapter with per-request DI container scoping via coroutine context; missing try/finally cleanup for context key (noted in previous thread).
src/Http/Adapter/SwooleCoroutine/Server.php	Coroutine Swoole server correctly uses try/finally for context cleanup; $port passed without (int) cast (noted in previous thread).
src/Http/Adapter/FPM/Server.php	New FPM adapter server; simple and correct — delegates request/response creation and resource registration to the shared container appropriately for single-request FPM lifecycle.
src/Http/Adapter/FPM/Request.php	Complete FPM request adapter extracted from base class; correctly implements all abstract methods including getReferer with proper $default forwarding.
src/Http/Adapter/Swoole/Response.php	New Swoole response adapter; sendStatus() incorrectly casts $statusCode to string before passing to Swoole\Http\Response::status() (noted in previous thread).
src/Http/Request.php	Request base class refactored to abstract; getSize() reads php://input for body measurement which is empty under Swoole adapters (noted in previous thread).

_{Reviews (3): Last reviewed commit: "Use Swoole parsed cookies again (#233)" | Re-trigger Greptile}

[greptile-apps]

Unnecessary (string) cast in sendStatus

Swoole\Http\Response::status() has the signature status(int $http_status_code, string $reason = ''): bool. $statusCode is already typed as int by the parent abstract method — casting it to string before passing it in is at best a no-op (PHP coerces it back) and could cause a TypeError if Swoole ever enforces strict typing internally. The cast should be removed.

Suggested change

	$this->swoole->status((string) $statusCode);
	$this->swoole->status($statusCode);

[greptile-apps]

Missing try/finally cleanup for coroutine context

The SwooleCoroutine\Server variant wraps the callback invocation in a try/finally to unset(Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY]) after each request, ensuring stale per-request containers don't linger if an exception escapes. The process-mode Swoole\Server does not do this:

Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
\call_user_func($callback, new Request($request), new Response($response));
// No cleanup

Although Swoole destroys each request coroutine's context automatically when the coroutine exits, adding the matching try/finally here keeps the two adapters consistent and guards against any future change that reuses coroutines across requests:

Suggested change

	$this->server->on('request', function (SwooleRequest $request, SwooleResponse $response) use ($callback) {
	$requestContainer = new Container($this->container);
	$requestContainer->set('swooleRequest', fn () => $request);
	$requestContainer->set('swooleResponse', fn () => $response);
	Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
	\call_user_func($callback, new Request($request), new Response($response));
	Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
	try {
	\call_user_func($callback, new Request($request), new Response($response));
	} finally {
	unset(Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY]);
	}

[greptile-apps]

getSize() reads php://input — incorrect for Swoole adapters

The base getSize() calls file_get_contents('php://input') to measure the request body size. This works under PHP-FPM, but in Swoole the request body is only accessible via $swoole->rawContent() — PHP's php://input stream is empty in that context.

Neither Swoole/Request.php nor SwooleCoroutine/Request.php override getSize(). Because Http::run() now calls $this->requestBodySize->record($request->getSize(), ...) for the new telemetry, all Swoole requests will report zero bytes for the body in the http.server.request.body.size metric.

Fix: override getSize() in Swoole/Request.php to use rawContent() for the body component:

public function getSize(): int
{
    $headers = $this->generateHeaders();
    $headerStrings = [];
    foreach ($headers as $key => $value) {
        $headerStrings[] = is_array($value)
            ? $key . ': ' . implode(', ', $value)
            : $key . ': ' . $value;
    }
    return mb_strlen(implode("\n", $headerStrings), '8bit')
         + mb_strlen($this->swoole->rawContent(), '8bit');
}

SwooleCoroutine/Request.php extends Swoole/Request.php, so it will inherit the fix.

[greptile-apps]

activeRequests counter leaks when runInternal throws

run() increments the active-requests counter before calling runInternal(), but the matching decrement sits after the call with no try/finally guard. runInternal can throw — for example, when an error handler itself raises an exception the code explicitly re-throws:

throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);

If that path is taken the add(-1) call on line 842 is skipped, and the http.server.active_requests gauge is permanently over-counted by one for every such request. Over time this makes the metric untrustworthy.

The fix is to wrap the post-call telemetry in a try/finally:

$start = microtime(true);
try {
    $result = $this->runInternal($request, $response);
} finally {
    $requestDuration = microtime(true) - $start;
    $attributes = [
        'url.scheme' => $request->getProtocol(),
        'http.request.method' => $request->getMethod(),
        'http.route' => $this->route?->getPath(),
        'http.response.status_code' => $response->getStatusCode(),
    ];
    $this->requestDuration->record($requestDuration, $attributes);
    $this->requestBodySize->record($request->getSize(), $attributes);
    $this->responseBodySize->record($response->getSize(), $attributes);
    $this->activeRequests->add(-1, [
        'http.request.method' => $request->getMethod(),
        'url.scheme' => $request->getProtocol(),
    ]);
}
return $result;