From ffd5405db7c4c64a96f45f120a23aafaf16c0818 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Wed, 6 May 2026 16:09:42 +0600 Subject: [PATCH 1/2] Fix: Make redirects, views, JSON, and errors all set original consistently --- src/Phaseolies/Error/WebErrorRenderer.php | 6 +-- src/Phaseolies/Helpers/helpers.php | 13 +++-- src/Phaseolies/Http/Response.php | 2 +- .../Http/Response/RedirectResponse.php | 1 + tests/Error/JsonErrorRendererTest.php | 2 + tests/Error/WebErrorRendererTest.php | 5 +- tests/RedirectResponseTest.php | 18 ++++++- tests/ResponseTest.php | 52 +++++++++++++++++++ tests/Router/RouterTest.php | 7 ++- 9 files changed, 93 insertions(+), 13 deletions(-) diff --git a/src/Phaseolies/Error/WebErrorRenderer.php b/src/Phaseolies/Error/WebErrorRenderer.php index 5e6f4af6..ce058d1f 100644 --- a/src/Phaseolies/Error/WebErrorRenderer.php +++ b/src/Phaseolies/Error/WebErrorRenderer.php @@ -190,13 +190,11 @@ public function renderProduction(Throwable $exception): Response $content = ob_get_clean() ?: ''; return response($content, $statusCode) - ->setOriginal($content) - ->setStatusCode($statusCode); + ->setOriginal($exception); } return response($message, $statusCode) - ->setOriginal($message) - ->setStatusCode($statusCode); + ->setOriginal($exception); } /** diff --git a/src/Phaseolies/Helpers/helpers.php b/src/Phaseolies/Helpers/helpers.php index 169a2449..5c236849 100644 --- a/src/Phaseolies/Helpers/helpers.php +++ b/src/Phaseolies/Helpers/helpers.php @@ -189,7 +189,10 @@ function view($view, array $data = [], array $headers = []): Response $instance = app(Controller::class); $content = $instance->render($view, $data, true); - return response($content, 200, $headers)->setOriginal($content); + return response($content, 200, $headers)->setOriginal([ + 'view' => $view, + 'data' => $data, + ]); } } @@ -205,11 +208,13 @@ function view($view, array $data = [], array $headers = []): Response */ function redirect($to = null, $status = 302, $headers = [], $secure = null) { + $redirect = app(RedirectResponse::class); + if (is_null($to)) { - return app('redirect'); + return $redirect; } - return app('redirect')->to($to, $status, $headers, $secure); + return $redirect->to($to, $status, $headers, $secure); } } @@ -224,7 +229,7 @@ function redirect($to = null, $status = 302, $headers = [], $secure = null) */ function back($status = 302, $headers = [], $fallback = false) { - return app('redirect')->back($status, $headers, $fallback); + return redirect()->back($status, $headers, $fallback); } } diff --git a/src/Phaseolies/Http/Response.php b/src/Phaseolies/Http/Response.php index e8ae7a13..e2c2ce44 100644 --- a/src/Phaseolies/Http/Response.php +++ b/src/Phaseolies/Http/Response.php @@ -192,7 +192,7 @@ public function setException($exception): static * Set the HTTP status code for the response. * * @param int $statusCode The HTTP status code. - * @return int The updated status code. + * @return static */ public function setStatusCode(int $statusCode, ?string $text = null): static { diff --git a/src/Phaseolies/Http/Response/RedirectResponse.php b/src/Phaseolies/Http/Response/RedirectResponse.php index 3bd76787..57a552cf 100644 --- a/src/Phaseolies/Http/Response/RedirectResponse.php +++ b/src/Phaseolies/Http/Response/RedirectResponse.php @@ -40,6 +40,7 @@ public function setTargetUrl(string $url): static throw new \InvalidArgumentException('Cannot redirect to an empty URL.'); } + $this->setOriginal($url); $this->setBody( sprintf(' diff --git a/tests/Error/JsonErrorRendererTest.php b/tests/Error/JsonErrorRendererTest.php index bbb517d0..71df0721 100644 --- a/tests/Error/JsonErrorRendererTest.php +++ b/tests/Error/JsonErrorRendererTest.php @@ -78,6 +78,7 @@ public function testRenderReturnsJsonResponseInstance(): void $payload = json_decode($response->getBody(), true); + $this->assertSame($payload, $response->getOriginal()); $this->assertSame('Boom', $payload['message']); $this->assertIsArray($payload['errors']); $this->assertArrayHasKey('trace', $payload['errors']); @@ -96,6 +97,7 @@ public function testRenderUsesValidationEnvelopeForKnownStatuses(): void $payload = json_decode($response->getBody(), true); + $this->assertSame($payload, $response->getOriginal()); $this->assertSame($errors, $payload['errors']); $this->assertIsString($payload['message']); $this->assertNotSame('', $payload['message']); diff --git a/tests/Error/WebErrorRendererTest.php b/tests/Error/WebErrorRendererTest.php index 4e4b58b1..fc9c93a9 100644 --- a/tests/Error/WebErrorRendererTest.php +++ b/tests/Error/WebErrorRendererTest.php @@ -24,12 +24,14 @@ protected function setUp(): void public function testRenderProductionReturnsResponseForGenericException(): void { $renderer = new WebErrorRenderer(); + $exception = new RuntimeException('Boom'); - $response = $renderer->renderProduction(new RuntimeException('Boom')); + $response = $renderer->renderProduction($exception); $this->assertInstanceOf(Response::class, $response); $this->assertSame(500, $response->getStatusCode()); $this->assertSame('Something went wrong', $response->getBody()); + $this->assertSame($exception, $response->getOriginal()); } public function testRenderProductionPreservesHttpExceptionStatus(): void @@ -41,5 +43,6 @@ public function testRenderProductionPreservesHttpExceptionStatus(): void $this->assertSame(404, $response->getStatusCode()); $this->assertSame('Not Found', $response->getBody()); + $this->assertSame($exception, $response->getOriginal()); } } diff --git a/tests/RedirectResponseTest.php b/tests/RedirectResponseTest.php index fb128084..d1d3b813 100644 --- a/tests/RedirectResponseTest.php +++ b/tests/RedirectResponseTest.php @@ -67,6 +67,11 @@ public function set($key, $value) } }; } + + public function except(...$keys): array + { + return []; + } } class MockMessageBag @@ -125,6 +130,8 @@ protected function setUp(): void MockRouter::$namedRoutes = []; $this->setupGlobalMocks(); + $container->instance('session', $this->session); + $container->instance('request', $this->request); $this->replaceMessageBag(); } @@ -198,6 +205,7 @@ public function testSetTargetUrlWithValidUrl() $result = $this->redirect->setTargetUrl($url); $this->assertSame($this->redirect, $result); + $this->assertSame($url, $this->redirect->getOriginal()); $this->assertEquals($url, $this->redirect->headers->get('Location')); $this->assertEquals('text/html; charset=utf-8', $this->redirect->headers->get('Content-Type')); $this->assertStringContainsString('Redirecting to', $this->redirect->getBody()); @@ -222,6 +230,7 @@ public function testToMethod() $this->assertSame($this->redirect, $result); $this->assertEquals($statusCode, $this->redirect->getStatusCode()); + $this->assertSame($url, $this->redirect->getOriginal()); $this->assertEquals($url, $this->redirect->headers->get('Location')); $this->assertEquals('value', $this->redirect->headers->get('X-Custom')); } @@ -259,6 +268,8 @@ public function testBackMethodWithReferer() $this->assertSame($this->redirect, $result); $this->assertEquals(301, $this->redirect->getStatusCode()); + $this->assertSame($referer, $this->redirect->getOriginal()); + $this->assertEquals($referer, $this->redirect->headers->get('Location')); $this->assertEquals('value', $this->redirect->headers->get('X-Test')); } @@ -268,6 +279,7 @@ public function testBackMethodWithoutReferer() $result = $this->redirect->back(); + $this->assertSame('/', $this->redirect->getOriginal()); $this->assertEquals('/', $this->redirect->headers->get('Location')); } @@ -278,7 +290,9 @@ public function testIntendedMethodWithStoredUrl() $result = $this->redirect->intended('/default'); - $this->assertEquals($intendedUrl, $this->session->get('url.intended')); + $this->assertSame($intendedUrl, $this->redirect->getOriginal()); + $this->assertEquals($intendedUrl, $this->redirect->headers->get('Location')); + $this->assertNull($this->session->get('url.intended')); } public function testIntendedMethodWithoutStoredUrl() @@ -286,6 +300,7 @@ public function testIntendedMethodWithoutStoredUrl() $defaultUrl = '/dashboard'; $result = $this->redirect->intended($defaultUrl, 301); + $this->assertSame($defaultUrl, $this->redirect->getOriginal()); $this->assertEquals($defaultUrl, $this->redirect->headers->get('Location')); $this->assertEquals(301, $this->redirect->getStatusCode()); } @@ -297,6 +312,7 @@ public function testAwayMethod() $result = $this->redirect->away($externalUrl, 301); $this->assertSame($this->redirect, $result); + $this->assertSame($externalUrl, $this->redirect->getOriginal()); $this->assertEquals($externalUrl, $this->redirect->headers->get('Location')); $this->assertEquals(301, $this->redirect->getStatusCode()); } diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index b0ad6766..59e174c7 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -5,9 +5,11 @@ use Phaseolies\Http\Response; use Phaseolies\Http\Request; use Phaseolies\Http\Response\JsonResponse; +use Phaseolies\Http\Response\RedirectResponse; use Phaseolies\Http\Response\Stream\StreamedJsonResponse; use Phaseolies\Http\Response\ResponseHeaderBag; use Phaseolies\Http\Exceptions\HttpException; +use Phaseolies\DI\Container; use PHPUnit\Framework\TestCase; class ResponseTest extends TestCase @@ -170,6 +172,19 @@ public function testJsonResponseUsesBufferedBodyAsSingleSourceOfTruth() $this->assertSame('application/json', $jsonResponse->headers->get('Content-Type')); } + public function testJsonResponsePrepareNullsBodyForHeadRequests() + { + $jsonResponse = $this->response->json(['test' => 'value'], 200); + $request = new Request(); + $request->server->set('SERVER_PROTOCOL', 'HTTP/1.1'); + $request->server->set('REQUEST_METHOD', 'HEAD'); + + $jsonResponse->prepare($request); + + $this->assertNull($jsonResponse->getBody()); + $this->assertSame('application/json', $jsonResponse->headers->get('Content-Type')); + } + public function testStreamedJsonResponseDoesNotCachePretendBody() { $response = new StreamedJsonResponse([['test' => 'value']]); @@ -203,6 +218,43 @@ public function testText() $this->assertEquals('text/plain', $response->headers->get('Content-Type')); } + public function testRedirectHelperSetsOriginalToTargetUrl() + { + $response = redirect('/dashboard'); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('/dashboard', $response->getOriginal()); + $this->assertSame('/dashboard', $response->headers->get('Location')); + } + + public function testBackHelperSetsOriginalToPreviousUrl() + { + $container = new Container(); + Container::setInstance($container); + + $request = new Request(); + $request->headers->set('referer', '/previous'); + $container->instance('request', $request); + + $response = back(); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('/previous', $response->getOriginal()); + $this->assertSame('/previous', $response->headers->get('Location')); + } + + public function testRedirectHelperReturnsFreshInstancesWithoutHeaderLeakage() + { + $first = redirect('/first', 302, ['X-Test' => 'first']); + $second = redirect('/second'); + + $this->assertNotSame($first, $second); + $this->assertSame('/first', $first->getOriginal()); + $this->assertSame('/second', $second->getOriginal()); + $this->assertSame('first', $first->headers->get('X-Test')); + $this->assertNull($second->headers->get('X-Test')); + } + public function testIsNotCacheable() { $this->response->setStatusCode(500); diff --git a/tests/Router/RouterTest.php b/tests/Router/RouterTest.php index c6ae8f91..f596fb66 100644 --- a/tests/Router/RouterTest.php +++ b/tests/Router/RouterTest.php @@ -237,7 +237,7 @@ public function testAnyMethodRegistersRoute(): void $request = new Request(); $container = Container::getInstance(); - $container->bind('request', fn() => $request); + $container->instance('request', $request); $result = $this->router->any('/test', $callback); @@ -458,7 +458,10 @@ public function testViewHelperReturnsFreshResponseInstance(): void $this->assertNotSame($sharedResponse, $response); $this->assertSame('

Doppar

', $response->getBody()); - $this->assertSame('

Doppar

', $response->getOriginal()); + $this->assertSame([ + 'view' => 'demo', + 'data' => ['name' => 'Doppar'], + ], $response->getOriginal()); $this->assertSame('ok', $response->headers->get('X-Test')); $this->assertNull($response->headers->get('X-Leaked')); $this->assertSame(200, $response->getStatusCode()); From 2f8eedca77544b969300130f28e77139a62b18a0 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Wed, 6 May 2026 16:19:58 +0600 Subject: [PATCH 2/2] added response lifecycle unit test: --- src/Phaseolies/Helpers/helpers.php | 2 +- tests/ResponseLifecycleTest.php | 286 +++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 tests/ResponseLifecycleTest.php diff --git a/src/Phaseolies/Helpers/helpers.php b/src/Phaseolies/Helpers/helpers.php index 5c236849..397c6f2e 100644 --- a/src/Phaseolies/Helpers/helpers.php +++ b/src/Phaseolies/Helpers/helpers.php @@ -208,7 +208,7 @@ function view($view, array $data = [], array $headers = []): Response */ function redirect($to = null, $status = 302, $headers = [], $secure = null) { - $redirect = app(RedirectResponse::class); + $redirect = new RedirectResponse(); if (is_null($to)) { return $redirect; diff --git a/tests/ResponseLifecycleTest.php b/tests/ResponseLifecycleTest.php new file mode 100644 index 00000000..d4515095 --- /dev/null +++ b/tests/ResponseLifecycleTest.php @@ -0,0 +1,286 @@ +testMethod = strtoupper($method); + $this->testPath = $path; + $this->testHost = $host; + } + + public function getMethod(): string + { + return $this->testMethod; + } + + public function getPath(): string + { + return $this->testPath; + } + + public function getHost(): string + { + return $this->testHost; + } + + public function getRouteParams(): array + { + return $this->testRouteParams; + } + + public function setRouteParams(array $params): self + { + $this->testRouteParams = $params; + + return $this; + } +} + +class LifecycleTestableRouter extends Router +{ + public function handle(Request $request, \Closure $handler): Response + { + return $handler($request); + } +} + +class ResponseLifecycleTest extends TestCase +{ + private MockContainer $container; + private string $translationPath; + private array $originalServer; + private array $originalGet; + private array $originalPost; + private array $originalSession; + + protected function setUp(): void + { + parent::setUp(); + + $this->originalServer = $_SERVER ?? []; + $this->originalGet = $_GET ?? []; + $this->originalPost = $_POST ?? []; + $this->originalSession = $_SESSION ?? []; + + $_SERVER = []; + $_GET = []; + $_POST = []; + $_SESSION = []; + + $this->container = new MockContainer(); + Container::setInstance($this->container); + + $this->translationPath = sys_get_temp_dir() . '/phaseolies_response_lifecycle_lang_' . uniqid(); + mkdir($this->translationPath . '/en', 0777, true); + file_put_contents($this->translationPath . '/en/validation.php', <<<'PHP' + 'Validation failed.', + 'rate_limit' => ['message' => 'Too many requests.'], + 'unauthorized' => ['message' => 'Unauthorized.'], + 'required' => 'The :attribute field is required.', +]; +PHP); + + $this->container->bind('translator', fn() => new Translator(new FileLoader($this->translationPath), 'en')); + $this->container->bind('session', Session::class); + $this->container->instance('session', new Session()); + } + + protected function tearDown(): void + { + Mockery::close(); + $this->deleteDir($this->translationPath); + + $_SERVER = $this->originalServer; + $_GET = $this->originalGet; + $_POST = $this->originalPost; + $_SESSION = $this->originalSession; + + parent::tearDown(); + } + + public function testControllerReturnAndResponseHelperProduceSameJsonLifecycleOutput(): void + { + $payload = ['framework' => 'Doppar', 'version' => 3]; + $app = $this->createStub(Application::class); + $router = new LifecycleTestableRouter($app); + + $router->get('/payload', fn() => $payload); + + $request = new LifecycleRouteRequestStub('GET', '/payload', 'localhost'); + + $routeResponse = $router->resolve($app, $request); + $helperResponse = response($payload); + + $this->assertSame(200, $routeResponse->getStatusCode()); + $this->assertSame(200, $helperResponse->getStatusCode()); + $this->assertSame('application/json', $routeResponse->headers->get('Content-Type')); + $this->assertSame('application/json', $helperResponse->headers->get('Content-Type')); + $this->assertSame($payload, $routeResponse->getOriginal()); + $this->assertSame($payload, $helperResponse->getOriginal()); + $this->assertSame($helperResponse->getBody(), $routeResponse->getBody()); + } + + public function testHeadPreparationStripsBodyForControllerAndHelperJsonResponses(): void + { + $payload = ['framework' => 'Doppar']; + $app = $this->createStub(Application::class); + $router = new LifecycleTestableRouter($app); + + $router->get('/payload', fn() => $payload); + + $routeRequest = new LifecycleRouteRequestStub('GET', '/payload', 'localhost'); + $routeResponse = $router->resolve($app, $routeRequest); + $helperResponse = response($payload); + + $_SERVER['REQUEST_METHOD'] = 'HEAD'; + $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1'; + $headRequest = new Request(); + + $routeResponse->prepare($headRequest); + $helperResponse->prepare($headRequest); + + $this->assertNull($routeResponse->getBody()); + $this->assertNull($helperResponse->getBody()); + $this->assertSame('application/json', $routeResponse->headers->get('Content-Type')); + $this->assertSame('application/json', $helperResponse->headers->get('Content-Type')); + } + + public function testAbortJsonBranchReturnsPreparedJsonResponse(): void + { + $request = Mockery::mock(Request::class); + $request->shouldReceive('isAjax')->andReturn(true); + $request->shouldReceive('isApiRequest')->andReturn(false); + $this->container->instance('request', $request); + + $abortion = new RequestAbortion(); + + try { + $abortion->abort(403, 'Forbidden'); + $this->fail('Expected HttpResponseException was not thrown.'); + } catch (HttpResponseException $exception) { + $response = $exception->getResponse(); + $payload = json_decode($response?->getBody() ?? '', true); + + $this->assertNotNull($response); + $this->assertSame(403, $response->getStatusCode()); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + $this->assertSame($payload, $response->getOriginal()); + $this->assertSame('Forbidden', $payload['message']); + $this->assertIsArray($payload['errors']); + } + } + + public function testValidationRedirectBranchReturnsPreparedRedirectResponse(): void + { + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SERVER['REQUEST_URI'] = '/submit'; + $_SERVER['HTTP_REFERER'] = '/form'; + $_POST = []; + + $request = new Request(); + $this->container->instance('request', $request); + + try { + $request->sanitize(['email' => 'required']); + $this->fail('Expected HttpResponseException was not thrown.'); + } catch (HttpResponseException $exception) { + $response = $exception->getResponse(); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame(302, $response->getStatusCode()); + $this->assertSame('/form', $response->headers->get('Location')); + $this->assertSame('/form', $response->getOriginal()); + $this->assertArrayHasKey('email', $this->container->get('session')->get('errors')); + $this->assertSame([], $this->container->get('session')->get('input')); + } + } + + public function testViewJsonAndRedirectHelpersDoNotReuseStaleSharedInstances(): void + { + $sharedResponse = new Response('stale body', 202, ['X-Leaked' => 'yes']); + $sharedRedirect = (new RedirectResponse())->to('/old', 302, ['X-Redirect-Leaked' => 'yes']); + $controller = $this->createMock(Controller::class); + + $controller->expects($this->once()) + ->method('render') + ->with('demo', ['name' => 'Doppar'], true) + ->willReturn('

Doppar

'); + + $this->container->instance('response', $sharedResponse); + $this->container->instance('redirect', $sharedRedirect); + $this->container->instance(Controller::class, $controller); + + $viewResponse = view('demo', ['name' => 'Doppar']); + $jsonResponse = response(['fresh' => true]); + $redirectResponse = redirect('/fresh'); + + $this->assertNotSame($sharedResponse, $viewResponse); + $this->assertNotSame($sharedResponse, $jsonResponse); + $this->assertNotSame($sharedRedirect, $redirectResponse); + + $this->assertNull($viewResponse->headers->get('X-Leaked')); + $this->assertNull($jsonResponse->headers->get('X-Leaked')); + $this->assertNull($redirectResponse->headers->get('X-Redirect-Leaked')); + + $this->assertSame([ + 'view' => 'demo', + 'data' => ['name' => 'Doppar'], + ], $viewResponse->getOriginal()); + $this->assertSame(['fresh' => true], $jsonResponse->getOriginal()); + $this->assertSame('/fresh', $redirectResponse->getOriginal()); + $this->assertSame('/fresh', $redirectResponse->headers->get('Location')); + } + + private function deleteDir(string $dir): void + { + if (!is_dir($dir)) { + return; + } + + $files = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($files as $file) { + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + + @rmdir($dir); + } +}