Skip to content

Commit c430f34

Browse files
michalsnmichalsn
authored
fix: set headers for CORS (#9437)
* fix: set everything in the before filter for CORS * fix append vary header * cors: apply response headers in the after filter only if they are not already set * cs fix
1 parent efcabb3 commit c430f34

File tree

5 files changed

+128
-13
lines changed

5 files changed

+128
-13
lines changed

‎system/Filters/Cors.php‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,32 @@ public function before(RequestInterface $request, $arguments = null)
5858

5959
$this->createCorsService($arguments);
6060

61-
if (! $this->cors->isPreflightRequest($request)) {
62-
return null;
63-
}
64-
6561
/** @var ResponseInterface $response */
6662
$response = service('response');
6763

68-
$response = $this->cors->handlePreflightRequest($request, $response);
64+
if ($this->cors->isPreflightRequest($request)) {
65+
$response = $this->cors->handlePreflightRequest($request, $response);
6966

70-
// Always adds `Vary: Access-Control-Request-Method` header for cacheability.
71-
// If there is an intermediate cache server such as a CDN, if a plain
72-
// OPTIONS request is sent, it may be cached. But valid preflight requests
73-
// have this header, so it will be cached separately.
74-
$response->appendHeader('Vary', 'Access-Control-Request-Method');
67+
// Always adds `Vary: Access-Control-Request-Method` header for cacheability.
68+
// If there is an intermediate cache server such as a CDN, if a plain
69+
// OPTIONS request is sent, it may be cached. But valid preflight requests
70+
// have this header, so it will be cached separately.
71+
$response->appendHeader('Vary', 'Access-Control-Request-Method');
72+
73+
return $response;
74+
}
7575

76-
return $response;
76+
if ($request->is('OPTIONS')) {
77+
// Always adds `Vary: Access-Control-Request-Method` header for cacheability.
78+
// If there is an intermediate cache server such as a CDN, if a plain
79+
// OPTIONS request is sent, it may be cached. But valid preflight requests
80+
// have this header, so it will be cached separately.
81+
$response->appendHeader('Vary', 'Access-Control-Request-Method');
82+
}
83+
84+
$this->cors->addResponseHeaders($request, $response);
85+
86+
return null;
7787
}
7888

7989
/**
@@ -87,8 +97,6 @@ private function createCorsService(?array $arguments): void
8797

8898
/**
8999
* @param list<string>|null $arguments
90-
*
91-
* @return ResponseInterface|null
92100
*/
93101
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
94102
{
@@ -98,6 +106,10 @@ public function after(RequestInterface $request, ResponseInterface $response, $a
98106

99107
$this->createCorsService($arguments);
100108

109+
if ($this->cors->hasResponseHeaders($request, $response)) {
110+
return null;
111+
}
112+
101113
// Always adds `Vary: Access-Control-Request-Method` header for cacheability.
102114
// If there is an intermediate cache server such as a CDN, if a plain
103115
// OPTIONS request is sent, it may be cached. But valid preflight requests

‎system/HTTP/Cors.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,4 +227,24 @@ private function setExposeHeaders(ResponseInterface $response): void
227227
);
228228
}
229229
}
230+
231+
/**
232+
* Check if response headers were set
233+
*/
234+
public function hasResponseHeaders(RequestInterface $request, ResponseInterface $response): bool
235+
{
236+
if (! $response->hasHeader('Access-Control-Allow-Origin')) {
237+
return false;
238+
}
239+
240+
if ($this->config['supportsCredentials']
241+
&& ! $response->hasHeader('Access-Control-Allow-Credentials')) {
242+
return false;
243+
}
244+
245+
return ! ($this->config['exposedHeaders'] !== [] && (! $response->hasHeader('Access-Control-Expose-Headers') || ! str_contains(
246+
$response->getHeaderLine('Access-Control-Expose-Headers'),
247+
implode(', ', $this->config['exposedHeaders']),
248+
)));
249+
}
230250
}

‎tests/system/Filters/CorsTest.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use CodeIgniter\Exceptions\ConfigException;
1717
use CodeIgniter\HTTP\CLIRequest;
1818
use CodeIgniter\HTTP\IncomingRequest;
19+
use CodeIgniter\HTTP\RedirectResponse;
1920
use CodeIgniter\HTTP\RequestInterface;
2021
use CodeIgniter\HTTP\ResponseInterface;
2122
use CodeIgniter\HTTP\SiteURI;
@@ -154,6 +155,25 @@ private function handle(RequestInterface $request): ResponseInterface
154155
return $response;
155156
}
156157

158+
private function handleRedirect(RequestInterface $request): ResponseInterface
159+
{
160+
$response = $this->cors->before($request);
161+
if ($response instanceof ResponseInterface) {
162+
$this->response = $response;
163+
164+
return $response;
165+
}
166+
167+
$response = service('redirectresponse');
168+
169+
$response = $this->cors->after($request, $response);
170+
$response ??= service('redirectresponse');
171+
172+
$this->response = $response;
173+
174+
return $response;
175+
}
176+
157177
public function testItDoesModifyOnARequestWithSameOrigin(): void
158178
{
159179
$this->cors = $this->createCors(['allowedOrigins' => ['*']]);
@@ -461,4 +481,29 @@ public function testItAddsVaryAccessControlRequestMethodHeaderEvenIfItIsNormalOp
461481
// Always adds `Vary: Access-Control-Request-Method` header.
462482
$this->assertHeader('Vary', 'Access-Control-Request-Method');
463483
}
484+
485+
public function testItReturnsAllowOriginHeaderOnValidActualRequestWithRedirect(): void
486+
{
487+
$this->cors = $this->createCors();
488+
$request = $this->createValidActualRequest();
489+
490+
$response = $this->handleRedirect($request);
491+
492+
$this->assertInstanceOf(RedirectResponse::class, $response);
493+
$this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
494+
$this->assertHeader('Access-Control-Allow-Origin', 'http://localhost');
495+
}
496+
497+
public function testItReturnsAllowOriginHeaderOnAllowAllOriginRequestWithRedirect(): void
498+
{
499+
$this->cors = $this->createCors(['allowedOrigins' => ['*']]);
500+
$request = $this->createRequest();
501+
$request->setHeader('Origin', 'http://localhost');
502+
503+
$response = $this->handleRedirect($request);
504+
505+
$this->assertInstanceOf(RedirectResponse::class, $response);
506+
$this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
507+
$this->assertHeader('Access-Control-Allow-Origin', '*');
508+
}
464509
}

‎tests/system/HTTP/CorsTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,4 +521,41 @@ public function testAddResponseHeadersMultipleAllowedOriginsNotAllowed(): void
521521
$response->hasHeader('Access-Control-Allow-Methods'),
522522
);
523523
}
524+
525+
public function testHasResponseHeadersFalse(): void
526+
{
527+
$config = $this->getDefaultConfig();
528+
$config['allowedOrigins'] = ['http://localhost:8080'];
529+
$config['allowedMethods'] = ['GET', 'POST', 'PUT'];
530+
$cors = $this->createCors($config);
531+
532+
$request = $this->createRequest()
533+
->withMethod('GET')
534+
->setHeader('Origin', 'http://localhost:8080');
535+
536+
$response = service('redirectresponse', null, false);
537+
538+
$this->assertFalse(
539+
$cors->hasResponseHeaders($request, $response),
540+
);
541+
}
542+
543+
public function testHasResponseHeadersTrue(): void
544+
{
545+
$config = $this->getDefaultConfig();
546+
$config['allowedOrigins'] = ['http://localhost:8080'];
547+
$config['allowedMethods'] = ['GET', 'POST'];
548+
$cors = $this->createCors($config);
549+
550+
$request = $this->createRequest()
551+
->withMethod('GET')
552+
->setHeader('Origin', 'http://localhost:8080');
553+
554+
$response = service('redirectresponse', null, false);
555+
$response = $cors->addResponseHeaders($request, $response);
556+
557+
$this->assertTrue(
558+
$cors->hasResponseHeaders($request, $response),
559+
);
560+
}
524561
}

‎user_guide_src/source/changelogs/v4.6.1.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Bugs Fixed
3131
**********
3232

3333
- **CURLRequest:** Fixed an issue where multiple header sections appeared in the CURL response body during multiple redirects from the target server.
34+
- **Cors:** Fixed a bug in the Cors filter that caused the appropriate headers to not be added when another filter returned a response object in the ``before`` filter.
3435

3536
See the repo's
3637
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_

0 commit comments

Comments
 (0)