fix(relay): properly relay request paramters (#28)

also:
- improve the relay test.
- remove unnecessary exemption of the content-type header from the headers list.
This commit is contained in:
Mazen Touati
2026-01-03 16:48:30 +01:00
committed by GitHub
parent 7b4c37d10d
commit 64ef46a8a4
4 changed files with 286 additions and 19 deletions

View File

@@ -66,22 +66,25 @@ class RequestRelayAction
{
$contentType = $requestRelayData->headers['content-type'] ?? self::DEFAULT_CONTENT_TYPE;
$headers = Arr::except(
$requestRelayData->headers,
[
// The Laravel HTTP client automatically sets a Content-Type header when sending requests.
// To prevent duplicate Content-Type headers, we explicitly remove any user-supplied value.
'content-type',
],
$queryParameters = $requestRelayData->queryParameters;
$requestBody = $requestRelayData->body;
if (in_array($requestRelayData->method, ['get', 'head'])) {
$queryParameters = array_merge(
$queryParameters,
$requestBody,
);
$requestBody = [];
}
// SSL verification is disabled to support development environments with self-signed certificates.
return Http::withoutVerifying()
->withHeaders($headers)
->withHeaders($requestRelayData->headers)
->withQueryParameters($queryParameters)
->when(
in_array($requestRelayData->method, ['get', 'head']),
callback: fn (PendingRequest $pendingRequest) => $pendingRequest->withQueryParameters($requestRelayData->body),
default: fn (PendingRequest $pendingRequest) => $pendingRequest->withBody(
$requestBody !== [],
fn (PendingRequest $pendingRequest) => $pendingRequest->withBody(
json_encode($requestRelayData->body) ?: throw new RuntimeException('Cannot parse body.'),
contentType: $contentType,
),

View File

@@ -2,6 +2,7 @@
namespace Sunchayn\Nimbus\Modules\Relay\DataTransferObjects;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Sunchayn\Nimbus\Http\Api\Relay\NimbusRelayRequest;
use Sunchayn\Nimbus\Modules\Relay\Authorization\AuthorizationCredentials;
@@ -13,6 +14,7 @@ readonly class RequestRelayData
/**
* @param array<string, string> $headers
* @param array<string, mixed> $body
* @param array<string, string|null> $queryParameters
*/
public function __construct(
public string $method,
@@ -21,6 +23,7 @@ readonly class RequestRelayData
public array $headers,
public array $body,
public ParameterBag $cookies,
public array $queryParameters = [],
) {}
public static function fromRelayApiRequest(NimbusRelayRequest $nimbusRelayRequest): self
@@ -50,9 +53,14 @@ readonly class RequestRelayData
fn () => $headers->put('User-Agent', (string) $nimbusRelayRequest->userAgent()),
);
[
'endpoint' => $endpoint,
'queryParameters' => $queryParameters,
] = self::extractAndRemoveQueryParametersFromEndpoint($data['endpoint']);
return new self(
method: strtolower($data['method']),
endpoint: $data['endpoint'],
endpoint: $endpoint,
authorization: array_key_exists('authorization', $data)
? new AuthorizationCredentials(
type: AuthorizationTypeEnum::from($data['authorization']['type']),
@@ -62,6 +70,47 @@ readonly class RequestRelayData
headers: $headers->mapWithKeys(fn (mixed $value, string $key): array => [strtolower($key) => $value])->all(),
body: $nimbusRelayRequest->getBody(),
cookies: $nimbusRelayRequest->cookies,
queryParameters: $queryParameters,
);
}
/**
* @return array{endpoint: string, queryParameters: array<string, string|null>}
*/
private static function extractAndRemoveQueryParametersFromEndpoint(string $endpoint): array
{
$urlParts = parse_url($endpoint);
if (! $urlParts) {
return [
'endpoint' => $endpoint,
'queryParameters' => [],
];
}
$cleanEndpoint = (array_key_exists('scheme', $urlParts) ? $urlParts['scheme'].'://' : '')
.(array_key_exists('host', $urlParts) ? $urlParts['host'] : '')
.(array_key_exists('port', $urlParts) ? ':'.$urlParts['port'] : '')
.(array_key_exists('path', $urlParts) ? $urlParts['path'] : '');
$queryParameters = array_key_exists('query', $urlParts)
? Arr::mapWithKeys(
explode('&', $urlParts['query']),
static function (string $query): array {
$parts = explode('=', $query);
if (count($parts) !== 2) {
return [];
}
return [$parts[0] => $parts[1]];
},
)
: [];
return [
'endpoint' => $cleanEndpoint,
'queryParameters' => $queryParameters,
];
}
}

View File

@@ -28,7 +28,7 @@ use Symfony\Component\HttpFoundation\ParameterBag;
#[CoversClass(RelayedRequestResponseData::class)]
class RequestRelayActionFunctionalTest extends TestCase
{
private const ENDPOINT = 'https:://localhost/api/test-endpoint';
private const ENDPOINT = 'https://localhost/api/test-endpoint';
#[TestWith([200, 'OK'])]
#[TestWith([404, 'Not Found'])]
@@ -50,7 +50,7 @@ class RequestRelayActionFunctionalTest extends TestCase
endpoint: self::ENDPOINT,
authorization: $authorizationCredentials = $this->getRandomAuthorizationCredentials(),
headers: [
'Content-Type' => 'application/json',
'Content-Type' => fake()->mimeType(),
'X-Custom-Header' => $customHeaderValue = uniqid(),
],
body: ['test' => 'data'],
@@ -161,6 +161,149 @@ class RequestRelayActionFunctionalTest extends TestCase
);
}
#[TestWith(['get'])]
#[TestWith(['head'])]
public function test_it_merges_body_into_query_parameters_for_get_and_head_requests(string $method): void
{
// Arrange
$bodyData = ['filter' => 'active', 'sort' => 'desc'];
$queryParameters = ['page' => '1', 'limit' => '10'];
$requestData = new RequestRelayData(
method: $method,
endpoint: self::ENDPOINT,
authorization: AuthorizationCredentials::none(),
headers: ['X-Custom-Header' => 'test'],
body: $bodyData,
cookies: new ParameterBag,
queryParameters: $queryParameters,
);
// Anticipate
Http::fake(function (Request $request) use ($bodyData, $queryParameters) {
// Assert that body data is merged into query parameters
$expectedQueryParams = array_merge($queryParameters, $bodyData);
foreach ($expectedQueryParams as $key => $value) {
if (! str_contains($request->url(), "{$key}={$value}")) {
return Http::response(['error' => 'Missing query parameter'], 400);
}
}
// Assert that the request body is empty
if (! empty($request->body())) {
return Http::response(['error' => 'Body should be empty'], 400);
}
return Http::response([
'success' => true,
]);
});
$this->mockAuthorizationHandler();
$requestRelayAction = resolve(RequestRelayAction::class);
// Act
$response = $requestRelayAction->execute($requestData);
// Assert
$this->assertEquals(200, $response->statusCode);
$this->assertEquals(
[
'success' => true,
],
$response->body->body,
);
}
public function test_it_sends_json_body_by_default(): void
{
// Arrange
$bodyData = ['user' => 'john', 'action' => 'login'];
$requestData = new RequestRelayData(
method: 'post',
endpoint: self::ENDPOINT,
authorization: AuthorizationCredentials::none(),
headers: [], // <- Content-Type is missing => Json by default.
body: $bodyData,
cookies: new ParameterBag,
);
// Anticipate
Http::fake(function (Request $request) use ($bodyData) {
$requestBody = json_decode($request->body(), true);
return Http::response([
'receivedBody' => $requestBody,
'bodyMatches' => $requestBody === $bodyData,
], 200);
});
$this->mockAuthorizationHandler();
$requestRelayAction = resolve(RequestRelayAction::class);
// Act
$response = $requestRelayAction->execute($requestData);
// Assert
$this->assertTrue($response->body->body['bodyMatches'], 'POST body should be sent as JSON');
$this->assertEquals($bodyData, $response->body->body['receivedBody']);
}
public function test_it_url_decodes_cookie_values(): void
{
// Arrange
$requestData = new RequestRelayData(
method: 'get',
endpoint: self::ENDPOINT,
authorization: AuthorizationCredentials::none(),
headers: [],
body: [],
cookies: new ParameterBag,
);
$encodedValue = urlencode('test value with spaces');
$stubHeaders = [
'Set-Cookie' => [
"testCookie={$encodedValue}; Path=/; HttpOnly",
],
];
// Anticipate
Http::fake(fn () => Http::response(['success' => true], 200, $stubHeaders));
$this->mockAuthorizationHandler();
$requestRelayAction = resolve(RequestRelayAction::class);
// Act
$response = $requestRelayAction->execute($requestData);
// Assert
$this->assertCount(1, $response->cookies);
$this->assertEquals('test value with spaces', $response->cookies[0]->toArray()['value']['raw']);
}
/*
* Helpers.
*/
@@ -185,6 +328,22 @@ class RequestRelayActionFunctionalTest extends TestCase
);
}
private function mockAuthorizationHandler(): void
{
$dummyAuthorizationHandler = $this->mock(
AuthorizationHandler::class,
fn (MockInterface $mock) => $mock->shouldReceive('authorize')
->andReturnArg(0),
);
$this->mock(
AuthorizationHandlerFactory::class,
fn (MockInterface $mock) => $mock
->shouldReceive('create')
->andReturn($dummyAuthorizationHandler),
);
}
/*
* Asserts.
*/

View File

@@ -2,8 +2,10 @@
namespace Sunchayn\Nimbus\Tests\App\Modules\Relay\DataTransferObjects;
use Generator;
use Mockery;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Sunchayn\Nimbus\Http\Api\Relay\NimbusRelayRequest;
use Sunchayn\Nimbus\Modules\Relay\Authorization\AuthorizationTypeEnum;
@@ -13,8 +15,12 @@ use Symfony\Component\HttpFoundation\InputBag;
#[CoversClass(RequestRelayData::class)]
class RequestRelayDataUnitTest extends TestCase
{
public function test_it_creates_instance_from_api_request(): void
{
#[DataProvider('endpointsDataProvider')]
public function test_it_creates_instance_from_api_request(
string $endpoint,
string $expectedEndpoint,
array $expectedParameters,
): void {
// Arrange
$mockRequest = Mockery::mock(NimbusRelayRequest::class);
@@ -36,7 +42,7 @@ class RequestRelayDataUnitTest extends TestCase
->andReturn(
[
'method' => $method = 'POST',
'endpoint' => $endpoint = '/api/test',
'endpoint' => $endpoint,
'authorization' => [
'type' => $stubAuthorizationType->value,
'value' => $authorizationValue = 'foobar',
@@ -59,7 +65,7 @@ class RequestRelayDataUnitTest extends TestCase
$this->assertEquals(strtolower($method), $result->method);
$this->assertEquals($endpoint, $result->endpoint);
$this->assertEquals($expectedEndpoint, $result->endpoint);
$this->assertEquals($stubAuthorizationType, $result->authorization->type);
@@ -77,5 +83,55 @@ class RequestRelayDataUnitTest extends TestCase
$this->assertEquals($body, $result->body);
$this->assertSame($mockCookies, $result->cookies);
$this->assertEquals(
$expectedParameters,
$result->queryParameters,
);
}
public static function endpointsDataProvider(): Generator
{
yield 'simple path without params' => [
'endpoint' => '/api/test',
'expectedEndpoint' => '/api/test',
'expectedParameters' => [],
];
yield 'simple path with single param' => [
'endpoint' => '/api/test?parameter-1=value',
'expectedEndpoint' => '/api/test',
'expectedParameters' => ['parameter-1' => 'value'],
];
yield 'absolute URL without params' => [
'endpoint' => 'https://127.0.0.1/api/test',
'expectedEndpoint' => 'https://127.0.0.1/api/test',
'expectedParameters' => [],
];
yield 'absolute URL with multiple params including broken' => [
'endpoint' => 'https://127.0.0.1/api/test?key=1&key-2=&broken',
'expectedEndpoint' => 'https://127.0.0.1/api/test',
'expectedParameters' => ['key' => '1', 'key-2' => ''],
];
yield 'absolute URL with multiple valid params' => [
'endpoint' => 'https://127.0.0.1/api/test?key=value&key-2=value-2',
'expectedEndpoint' => 'https://127.0.0.1/api/test',
'expectedParameters' => ['key' => 'value', 'key-2' => 'value-2'],
];
yield 'absolute URL with port and param' => [
'endpoint' => 'https://127.0.0.1:8000/api/test?key=value',
'expectedEndpoint' => 'https://127.0.0.1:8000/api/test',
'expectedParameters' => ['key' => 'value'],
];
yield 'invalid URL with port and param' => [
'endpoint' => 'http://:80?key=value',
'expectedEndpoint' => 'http://:80?key=value', // parse_url failed, return as is.
'expectedParameters' => [],
];
}
}