Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use addHeader() #8240

Merged
merged 7 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
33 changes: 26 additions & 7 deletions app/Views/errors/html/error_exception.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
use CodeIgniter\HTTP\Header;
use Config\Services;
use CodeIgniter\CodeIgniter;

Expand Down Expand Up @@ -289,10 +290,20 @@
</tr>
</thead>
<tbody>
<?php foreach ($headers as $header) : ?>
<?php foreach ($headers as $name => $value) : ?>
<tr>
<td><?= esc($header->getName(), 'html') ?></td>
<td><?= esc($header->getValueLine(), 'html') ?></td>
<td><?= esc($name, 'html') ?></td>
<td>
<?php
if ($value instanceof Header) {
echo esc($value->getValueLine(), 'html');
} else {
foreach ($value as $i => $header) {
echo ' ('. $i+1 . ') ' . esc($header->getValueLine(), 'html');
}
}
?>
</td>
</tr>
<?php endforeach; ?>
</tbody>
Expand All @@ -316,8 +327,6 @@

<?php $headers = $response->headers(); ?>
<?php if (! empty($headers)) : ?>
<?php natsort($headers) ?>

<h3>Headers</h3>

<table>
Expand All @@ -328,10 +337,20 @@
</tr>
</thead>
<tbody>
<?php foreach (array_keys($headers) as $name) : ?>
<?php foreach ($headers as $name => $value) : ?>
<tr>
<td><?= esc($name, 'html') ?></td>
<td><?= esc($response->getHeaderLine($name), 'html') ?></td>
<td>
<?php
if ($value instanceof Header) {
echo esc($response->getHeaderLine($name), 'html');
} else {
foreach ($value as $i => $header) {
echo ' ('. $i+1 . ') ' . esc($header->getValueLine(), 'html');
}
}
?>
</td>
</tr>
<?php endforeach; ?>
</tbody>
Expand Down
1 change: 1 addition & 0 deletions deptrac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ parameters:
# Individual class exemptions
CodeIgniter\Cache\ResponseCache:
- CodeIgniter\HTTP\CLIRequest
- CodeIgniter\HTTP\Header
- CodeIgniter\HTTP\IncomingRequest
- CodeIgniter\HTTP\ResponseInterface
CodeIgniter\Entity\Cast\URICast:
Expand Down
11 changes: 9 additions & 2 deletions system/Cache/ResponseCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\Cache;

use CodeIgniter\HTTP\CLIRequest;
use CodeIgniter\HTTP\Header;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\ResponseInterface;
use Config\Cache as CacheConfig;
Expand Down Expand Up @@ -99,8 +100,14 @@ public function make($request, ResponseInterface $response): bool

$headers = [];

foreach ($response->headers() as $header) {
$headers[$header->getName()] = $header->getValueLine();
foreach ($response->headers() as $name => $value) {
if ($value instanceof Header) {
$headers[$name] = $value->getValueLine();
} else {
foreach ($value as $header) {
$headers[$name][] = $header->getValueLine();
}
Comment on lines +107 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ($value as $header) {
$headers[$name][] = $header->getValueLine();
}
if(is_array($value)) {
foreach ($value as $header) {
$headers[$name][] = $header->getValueLine();
}
}

}
}

return $this->cache->save(
Expand Down
27 changes: 23 additions & 4 deletions system/Debug/Toolbar.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use CodeIgniter\Format\JSONFormatter;
use CodeIgniter\Format\XMLFormatter;
use CodeIgniter\HTTP\DownloadResponse;
use CodeIgniter\HTTP\Header;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\RequestInterface;
use CodeIgniter\HTTP\ResponseInterface;
Expand Down Expand Up @@ -140,8 +141,17 @@ public function run(float $startTime, float $totalTime, RequestInterface $reques
$data['vars']['post'][esc($name)] = is_array($value) ? '<pre>' . esc(print_r($value, true)) . '</pre>' : esc($value);
}

foreach ($request->headers() as $header) {
$data['vars']['headers'][esc($header->getName())] = esc($header->getValueLine());
foreach ($request->headers() as $name => $value) {
if ($value instanceof Header) {
$data['vars']['headers'][esc($name)] = esc($value->getValueLine());
} else {
foreach ($value as $i => $header) {
$index = $i + 1;
$data['vars']['headers'][esc($name)] ??= '';
$data['vars']['headers'][esc($name)] .= ' (' . $index . ') '
. esc($header->getValueLine());
}
Comment on lines +148 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ($value as $i => $header) {
$index = $i + 1;
$data['vars']['headers'][esc($name)] ??= '';
$data['vars']['headers'][esc($name)] .= ' (' . $index . ') '
. esc($header->getValueLine());
}
if(is_array($value)) {
foreach ($value as $i => $header) {
$index = $i + 1;
$data['vars']['headers'][esc($name)] ??= '';
$data['vars']['headers'][esc($name)] .= ' (' . $index . ') '
. esc($header->getValueLine());
}
}

}
}

foreach ($request->getCookie() as $name => $value) {
Expand All @@ -157,8 +167,17 @@ public function run(float $startTime, float $totalTime, RequestInterface $reques
'headers' => [],
];

foreach ($response->headers() as $header) {
$data['vars']['response']['headers'][esc($header->getName())] = esc($header->getValueLine());
foreach ($response->headers() as $name => $value) {
if ($value instanceof Header) {
$data['vars']['response']['headers'][esc($name)] = esc($value->getValueLine());
} else {
foreach ($value as $i => $header) {
$index = $i + 1;
$data['vars']['response']['headers'][esc($name)] ??= '';
$data['vars']['response']['headers'][esc($name)] .= ' (' . $index . ') '
. esc($header->getValueLine());
}
}
}

$data['config'] = Config::display();
Expand Down
10 changes: 7 additions & 3 deletions system/HTTP/CURLRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,14 @@ protected function setResponseHeaders(array $headers = [])
{
foreach ($headers as $header) {
if (($pos = strpos($header, ':')) !== false) {
$title = substr($header, 0, $pos);
$value = substr($header, $pos + 1);
$title = trim(substr($header, 0, $pos));
$value = trim(substr($header, $pos + 1));

$this->response->setHeader($title, $value);
if ($this->response instanceof Response) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ($this->response instanceof Response) {
if ($this->response->hasHeader($title)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

We cannot remove if ($this->response instanceof Response) because the type of $this->response is ResponseInterface, and it does not have the addHeader() method.

$this->response->addHeader($title, $value);
} else {
$this->response->setHeader($title, $value);
}
} elseif (strpos($header, 'HTTP') === 0) {
preg_match('#^HTTP\/([12](?:\.[01])?) (\d+) (.+)#', $header, $matches);

Expand Down
10 changes: 8 additions & 2 deletions system/HTTP/RedirectResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,14 @@ public function withCookies()
*/
public function withHeaders()
{
foreach (Services::response()->headers() as $name => $header) {
$this->setHeader($name, $header->getValue());
foreach (Services::response()->headers() as $name => $value) {
if ($value instanceof Header) {
$this->setHeader($name, $value->getValue());
} else {
foreach ($value as $header) {
$this->addHeader($name, $header->getValue());
}
Comment on lines +168 to +170
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ($value as $header) {
$this->addHeader($name, $header->getValue());
}
if(is_array($value)) {
foreach ($value as $header) {
$this->addHeader($name, $header->getValue());
}
}

}
}

return $this;
Expand Down
18 changes: 16 additions & 2 deletions system/HTTP/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,22 @@ public function sendHeaders()
header(sprintf('HTTP/%s %s %s', $this->getProtocolVersion(), $this->getStatusCode(), $this->getReasonPhrase()), true, $this->getStatusCode());

// Send all of our headers
foreach (array_keys($this->headers()) as $name) {
header($name . ': ' . $this->getHeaderLine($name), false, $this->getStatusCode());
foreach ($this->headers() as $name => $value) {
if ($value instanceof Header) {
header(
$name . ': ' . $value->getValueLine(),
false,
$this->getStatusCode()
);
} else {
foreach ($value as $header) {
header(
$name . ': ' . $header->getValueLine(),
false,
$this->getStatusCode()
);
}
}
Comment on lines +401 to +416
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
foreach ($this->headers() as $name => $value) {
if ($value instanceof Header) {
header(
$name . ': ' . $value->getValueLine(),
false,
$this->getStatusCode()
);
} else {
foreach ($value as $header) {
header(
$name . ': ' . $header->getValueLine(),
false,
$this->getStatusCode()
);
}
}
foreach (array_keys($this->headers()) as $name)
{
$headerValue = $this->getHeader($name)->getValue();
if(is_array($headerValue)) {
foreach ($headerValue as $h_value) {
header($name . ': ' . $h_value, false, $this->getStatusCode());
}
} else {
header($name . ': ' . $this->getHeaderLine($name), false, $this->getStatusCode());
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why my code does not work for you.
It seems your code does the same thing.

What was wrong? Any error message?

}

return $this;
Expand Down
32 changes: 32 additions & 0 deletions tests/system/HTTP/CURLRequestDoNotShareOptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,38 @@ public function testResponseHeadersWithMultipleRequests(): void
$this->assertSame(200, $response->getStatusCode());
}

public function testResponseHeadersWithMultipleSetCookies(): void
{
$request = $this->getRequest([
'base_uri' => 'https://github.com/',
]);

$output = "HTTP/2 200
server: GitHub.com
date: Sat, 11 Nov 2023 02:26:55 GMT
content-type: text/html; charset=utf-8
set-cookie: _gh_sess=PlRlha1YumlLhLuo5MuNbIWJRO9RRuR%2FHfYsWRh5B0mkalFIZstlAbTmSstl8q%2FAC57IsWMVuFHWQc6L4qDHQJrwhuYVO5ZaigPCUjAStnhh%2FieZQVqIf92Al7vusuzx2o8XH%2Fv6nd9qzMTAWc2%2FkRsl8jxPQYGNaWeuUBY2w3%2FDORSikN4c0vHOyedhU7Xcv3Ryz5xD3DNxK9R8xKNZ6OSXLJ6bjX8iIT6LxvroVIf2HjvowW9cQsq0kN08mS6KtTnH0mD3ANWqsVVWeMzFNA%3D%3D--Jx830Q9Nmkfz9OGA--kEcPtNphvjNMopYqFDxUbw%3D%3D; Path=/; HttpOnly; Secure; SameSite=Lax
set-cookie: _octo=GH1.1.599292127.1699669625; Path=/; Domain=github.com; Expires=Mon, 11 Nov 2024 02:27:05 GMT; Secure; SameSite=Lax
set-cookie: logged_in=no; Path=/; Domain=github.com; Expires=Mon, 11 Nov 2024 02:27:05 GMT; HttpOnly; Secure; SameSite=Lax
accept-ranges: bytes\x0d\x0a\x0d\x0a";
$request->setOutput($output);

$response = $request->get('/');

$setCookieHeaders = $response->header('set-cookie');

$this->assertCount(3, $setCookieHeaders);
$this->assertSame(
'logged_in=no; Path=/; Domain=github.com; Expires=Mon, 11 Nov 2024 02:27:05 GMT; HttpOnly; Secure; SameSite=Lax',
$setCookieHeaders[2]->getValue()
);

$this->assertSame(
'_octo=GH1.1.599292127.1699669625; Path=/; Domain=github.com; Expires=Mon, 11 Nov 2024 02:27:05 GMT; Secure; SameSite=Lax',
$setCookieHeaders[1]->getValueLine()
);
}

public function testSplitResponse(): void
{
$request = $this->getRequest([
Expand Down
Loading