fix(http): maintain set redirect code in response

fixes #14346
Elgg · Jul 7, 2023 · c4ad503 · c4ad503
1 parent 9384d39
commit c4ad503
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/engine/classes/Elgg/Http/ResponseFactory.php b/engine/classes/Elgg/Http/ResponseFactory.php
@@ -254,7 +254,7 @@ public function respond(ResponseBuilder $response) {
 
 		$is_xhr = $this->request->isXmlHttpRequest();
 
-		$is_action = str_starts_with($response_type, 'action:');
+		$is_action = $this->isAction();
 
 		if ($is_action && $response->getForwardURL() === null) {
 			// actions must always set a redirect url
@@ -265,10 +265,10 @@ public function respond(ResponseBuilder $response) {
 			$response->setForwardURL((string) $this->request->headers->get('Referer'));
 		}
 
-		if ($response->getForwardURL() !== null && !$is_xhr) {
+		if ($response->getForwardURL() !== null && !$is_xhr && !$response->isRedirection()) {
 			// non-xhr requests should issue a forward if redirect url is set
 			// unless it's an error, in which case we serve an error page
-			if ($this->isAction() || (!$response->isClientError() && !$response->isServerError())) {
+			if ($is_action || (!$response->isClientError() && !$response->isServerError())) {
 				$response->setStatusCode(ELGG_HTTP_FOUND);
 			}
 		}

diff --git a/engine/tests/phpunit/integration/Elgg/Http/ResponseFactoryIntegrationTest.php b/engine/tests/phpunit/integration/Elgg/Http/ResponseFactoryIntegrationTest.php
@@ -159,4 +159,33 @@ public function testRespondWithErrorPassesException() {
 
 		$this->assertTrue($exception_found, 'No exception found in view vars of resource/error');
 	}
+
+	/**
+	 * @dataProvider redirectCodeProvider
+	 */
+	public function testRespondWithRedirectCode(int $status_code, int $expected_code) {
+		$request = $this->prepareHttpRequest('action/foo', 'POST', [], 0, true);
+		_elgg_services()->request = $request;
+
+		$response = new OkResponse('', $status_code, '/forward');
+
+		ob_start();
+		$result = $this->service->respond($response);
+		ob_end_clean();
+
+		$this->assertInstanceOf(Response::class, $result);
+		$this->assertEquals($expected_code, $result->getStatusCode());
+	}
+
+	public function redirectCodeProvider() {
+		return [
+			[ELGG_HTTP_OK, ELGG_HTTP_FOUND],
+			[ELGG_HTTP_CREATED, ELGG_HTTP_CREATED],
+			[ELGG_HTTP_MOVED_PERMANENTLY, ELGG_HTTP_MOVED_PERMANENTLY],
+			[ELGG_HTTP_FOUND, ELGG_HTTP_FOUND],
+			[ELGG_HTTP_SEE_OTHER, ELGG_HTTP_SEE_OTHER],
+			[ELGG_HTTP_TEMPORARY_REDIRECT, ELGG_HTTP_TEMPORARY_REDIRECT],
+			[ELGG_HTTP_PERMANENTLY_REDIRECT, ELGG_HTTP_PERMANENTLY_REDIRECT],
+		];
+	}
 }