Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/GlobalState.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

declare(strict_types=1);

namespace Spiral\RoadRunner\Http;

use function time;
use function microtime;
use function strtoupper;
use function str_replace;
use function implode;

final class GlobalState
{
/**
* Sets ip-address, request-time and other values.
*
* @return non-empty-array<array-key|string, mixed|string>
*/
public static function populateServer(Request $request): array
{
/** @var non-empty-array<array-key|string, mixed|string>|null $originalServer */
static $originalServer = null;

if ($originalServer == null) {
$originalServer = $_SERVER;
}

$newServer = $originalServer;

$newServer['REQUEST_URI'] = $request->uri;
$newServer['REQUEST_TIME'] = time();
$newServer['REQUEST_TIME_FLOAT'] = microtime(true);
$newServer['REMOTE_ADDR'] = $request->getRemoteAddr();
$newServer['REQUEST_METHOD'] = $request->method;
$newServer['HTTP_USER_AGENT'] = '';

foreach ($request->headers as $key => $value) {
$key = strtoupper(str_replace('-', '_', $key));

if ($key == 'CONTENT_TYPE' || $key == 'CONTENT_LENGTH') {
$newServer[$key] = implode(', ', $value);

continue;
}

$newServer['HTTP_' . $key] = implode(', ', $value);
}

return $newServer;
}
}
34 changes: 13 additions & 21 deletions src/PSR7Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class PSR7Worker implements PSR7WorkerInterface

private readonly HttpWorker $httpWorker;

private readonly array $originalServer;

/**
* @var string[] Valid values for HTTP protocol version
Expand All @@ -46,7 +45,6 @@ public function __construct(
private readonly UploadedFileFactoryInterface $uploadsFactory,
) {
$this->httpWorker = new HttpWorker($worker);
$this->originalServer = $_SERVER;
}

public function getWorker(): WorkerInterface
Expand All @@ -60,6 +58,8 @@ public function getHttpWorker(): HttpWorker
}

/**
* @psalm-suppress DeprecatedMethod
*
* @throws \JsonException
*/
public function waitRequest(): ?ServerRequestInterface
Expand Down Expand Up @@ -121,36 +121,28 @@ private function streamToGenerator(StreamInterface $stream): Generator
* Returns altered copy of _SERVER variable. Sets ip-address,
* request-time and other values.
*
* @deprecated
*
* @return non-empty-array<array-key|string, mixed|string>
*/
protected function configureServer(Request $request): array
Comment thread
root-aza marked this conversation as resolved.
{
$server = $this->originalServer;

$server['REQUEST_URI'] = $request->uri;
$server['REQUEST_TIME'] = $this->timeInt();
$server['REQUEST_TIME_FLOAT'] = $this->timeFloat();
$server['REMOTE_ADDR'] = $request->getRemoteAddr();
$server['REQUEST_METHOD'] = $request->method;

$server['HTTP_USER_AGENT'] = '';
foreach ($request->headers as $key => $value) {
$key = \strtoupper(\str_replace('-', '_', $key));
if (\in_array($key, ['CONTENT_TYPE', 'CONTENT_LENGTH'])) {
$server[$key] = \implode(', ', $value);
} else {
$server['HTTP_' . $key] = \implode(', ', $value);
}
}

return $server;
return GlobalState::populateServer($request);
}


/**
* @deprecated
*/
protected function timeInt(): int
{
return \time();
}


/**
* @deprecated
*/
protected function timeFloat(): float
{
return \microtime(true);
Expand Down
56 changes: 48 additions & 8 deletions tests/Unit/PSR7WorkerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,70 @@

declare(strict_types=1);

namespace Spiral\RoadRunner\Tests\Http\Unit;

use Nyholm\Psr7\Factory\Psr17Factory;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\RunClassInSeparateProcess;
use PHPUnit\Framework\TestCase;
use Spiral\RoadRunner\Http\HttpWorker;
use Spiral\Goridge\Frame;
use Spiral\RoadRunner\Http\GlobalState;
use Spiral\RoadRunner\Http\PSR7Worker;
use Spiral\RoadRunner\Tests\Http\Unit\Stub\TestRelay;
use Spiral\RoadRunner\Worker;


#[CoversClass(PSR7Worker::class)]
#[CoversClass(GlobalState::class)]
#[RunClassInSeparateProcess]
final class PSR7WorkerTest extends TestCase
{
public function testHttpWorkerIsAvailable(): void
/***
* @param array $headers
*/
public function testStateServerLeak(): void
{
$psrFactory = new Psr17Factory();

$psrWorker = new PSR7Worker(
Worker::create(),
$relay = new TestRelay();
$psrWorker = new PSR7Worker(
new Worker($relay),
$psrFactory,
$psrFactory,
$psrFactory,
);

self::assertInstanceOf(HttpWorker::class, $psrWorker->getHttpWorker());
//dataProvider is always random and we need to keep the order
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Clarify the comment about DataProviders order

PHPUnit preserves the order of items provided by a data provider (unless a randomizer plugin is in use). If order is important, it’s fine to keep inline fixtures; otherwise consider a data provider for better test reporting.

Would you like to switch to a data provider? If you want to confirm ordering guarantees, run this web query:


🌐 Web query:

Does PHPUnit guarantee the iteration order of items returned by a data provider?

💡 Result:

PHPUnit does not explicitly guarantee the iteration order of items returned by a data provider. However, in practice, the data provider method returns an array or an iterable, and PHPUnit runs the test with each dataset in the order they are returned from the data provider.

Since the data provider returns a PHP array or iterable, and PHP arrays preserve insertion order (since PHP 5.4), PHPUnit will run the tests in the order the items are provided by the data provider.

So while not explicitly documented as a guarantee by PHPUnit, in typical usage and by PHP's own behavior regarding arrays, the iteration order of data provider items can be considered stable and predictable.

If you rely on a specific order, it's a good practice to:

  • Return the datasets in the desired order from the data provider.
  • Use numerically indexed arrays or ordered structures like generators that yield data in a specific sequence.

If you want to be absolutely certain about order, it is best to check against the PHPUnit documentation or source code to ensure no changes in future versions.

Would you like me to check for any recent documentation or discussions on this topic?


Update misleading comment about DataProvider ordering

The inline comment in tests/Unit/PSR7WorkerTest.php (line 33) is incorrect: PHPUnit runs data‐provider datasets in the order they’re returned (PHP arrays preserve insertion order). To improve clarity and test reporting, either adjust the comment or extract these cases into a named data provider.

Locations to update:

  • tests/Unit/PSR7WorkerTest.php (line 33)

Suggested diff:

-        //dataProvider is always random and we need to keep the order
+        // PHPUnit executes dataProvider items in the order they’re returned
+        // (PHP arrays preserve insertion order). Consider using a named data provider
+        // for clearer test reporting.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//dataProvider is always random and we need to keep the order
// PHPUnit executes dataProvider items in the order they’re returned
// (PHP arrays preserve insertion order). Consider using a named data provider
// for clearer test reporting.
🤖 Prompt for AI Agents
In tests/Unit/PSR7WorkerTest.php at line 33, the comment incorrectly states that
the dataProvider is always random, but PHPUnit actually runs data-provider
datasets in the order they are returned. Update the comment to accurately
reflect that the order is preserved or refactor the test to use a named data
provider method for clarity and better test reporting.

$fixtures = [
[['Content-Type' => ['application/html'], 'Connection' => ['keep-alive']], ['REQUEST_URI' => 'http://localhost', 'REMOTE_ADDR' => '127.0.0.1', 'REQUEST_METHOD' => 'GET', 'HTTP_USER_AGENT' => '', 'CONTENT_TYPE' => 'application/html', 'HTTP_CONNECTION' => 'keep-alive',]],
[['Content-Type' => ['application/json']], ['REQUEST_URI' => 'http://localhost', 'REMOTE_ADDR' => '127.0.0.1', 'REQUEST_METHOD' => 'GET', 'HTTP_USER_AGENT' => '', 'CONTENT_TYPE' => 'application/json']],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify REQUEST_URI semantics

The test expects REQUEST_URI to be the full URI (e.g., "http://localhost"). In classic PHP SAPI, REQUEST_URI usually contains the path + query, not scheme/host. If this divergence is intentional with GlobalState::populateServer(), document it to prevent future confusion.

To confirm usages and expectations across the codebase:


🏁 Script executed:

#!/bin/bash
# Find usages of REQUEST_URI to assess assumptions about its shape
rg -n "REQUEST_URI" -A2 -B2 src tests | sed -n '1,200p'

Length of output: 1166


🏁 Script executed:

#!/bin/bash
# Display the beginning of GlobalState.php to inspect the method signature and existing docblocks
sed -n '1,200p' src/GlobalState.php

Length of output: 1347


🏁 Script executed:

#!/bin/bash
# Locate the Request class definition to inspect the `uri` property
rg -n "class Request" -A10 -B5 src

Length of output: 741


🏁 Script executed:

#!/bin/bash
# Show property definitions and constructor in Request.php
sed -n '1,200p' src/Request.php

Length of output: 1969


Clarify REQUEST_URI semantics

GlobalState::populateServer currently assigns REQUEST_URI from Request::$uri, which holds the full URI (scheme + host). This deviates from PHP SAPI’s usual behavior (path + query only). If this is intentional, please update the docblock on populateServer() to document that REQUEST_URI will include the full URI from the Request object.

• File needing update: src/GlobalState.php (populateServer docblock)
• Suggested diff:

--- a/src/GlobalState.php
+++ b/src/GlobalState.php
@@ /**
-     * Sets ip-address, request-time and other values.
+     * Populates $_SERVER values (ip address, request time, HTTP headers, etc.).
+     * REQUEST_URI is set to the full URI (including scheme and host) from the Request object,
+     * rather than the path+query typical of PHP SAPI.
🤖 Prompt for AI Agents
In tests/Unit/PSR7WorkerTest.php around lines 35 to 36, the test data uses
REQUEST_URI with a full URI including scheme and host, which differs from
typical PHP SAPI behavior where REQUEST_URI contains only path and query. To fix
this, update the docblock of the populateServer() method in src/GlobalState.php
to clearly document that REQUEST_URI will include the full URI from the Request
object, reflecting this intentional design choice.

];

foreach ($fixtures as [$headers, $expectedServer]) {
$body = [
'headers' => $headers,
'rawQuery' => '',
'remoteAddr' => '127.0.0.1',
'protocol' => 'HTTP/1.1',
'method' => 'GET',
'uri' => 'http://localhost',
'parsed' => false,
];

$head = (string)\json_encode($body, \JSON_THROW_ON_ERROR);
$frame = new Frame($head .'test', [\strlen($head)]);

$relay->addFrames($frame);

$_SERVER = [];

$psrWorker->waitRequest();

unset($_SERVER['REQUEST_TIME']);
unset($_SERVER['REQUEST_TIME_FLOAT']);

self::assertEquals($expectedServer, $_SERVER);
}
}


protected function tearDown(): void
{
// Clean all extra output buffers
Expand Down
Loading