Skip to content

Commit 6414090

Browse files
paulbalandanpaulbalandan
committed
Merge branch 'develop' into 4.6
2 parents 80719d4 + 97a6d66 commit 6414090

File tree

5 files changed

+253
-24
lines changed

5 files changed

+253
-24
lines changed

‎system/HTTP/Header.php‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace CodeIgniter\HTTP;
1515

16+
use InvalidArgumentException;
1617
use Stringable;
1718

1819
/**
@@ -54,7 +55,7 @@ class Header implements Stringable
5455
*/
5556
public function __construct(string $name, $value = null)
5657
{
57-
$this->name = $name;
58+
$this->setName($name);
5859
$this->setValue($value);
5960
}
6061

@@ -81,9 +82,12 @@ public function getValue()
8182
* Sets the name of the header, overwriting any previous value.
8283
*
8384
* @return $this
85+
*
86+
* @throws InvalidArgumentException
8487
*/
8588
public function setName(string $name)
8689
{
90+
$this->validateName($name);
8791
$this->name = $name;
8892

8993
return $this;
@@ -95,10 +99,16 @@ public function setName(string $name)
9599
* @param array<int|string, array<string, string>|string>|string|null $value
96100
*
97101
* @return $this
102+
*
103+
* @throws InvalidArgumentException
98104
*/
99105
public function setValue($value = null)
100106
{
101-
$this->value = is_array($value) ? $value : (string) $value;
107+
$value = is_array($value) ? $value : (string) $value;
108+
109+
$this->validateValue($value);
110+
111+
$this->value = $value;
102112

103113
return $this;
104114
}
@@ -110,13 +120,17 @@ public function setValue($value = null)
110120
* @param array<string, string>|string|null $value
111121
*
112122
* @return $this
123+
*
124+
* @throws InvalidArgumentException
113125
*/
114126
public function appendValue($value = null)
115127
{
116128
if ($value === null) {
117129
return $this;
118130
}
119131

132+
$this->validateValue($value);
133+
120134
if (! is_array($this->value)) {
121135
$this->value = [$this->value];
122136
}
@@ -135,13 +149,17 @@ public function appendValue($value = null)
135149
* @param array<string, string>|string|null $value
136150
*
137151
* @return $this
152+
*
153+
* @throws InvalidArgumentException
138154
*/
139155
public function prependValue($value = null)
140156
{
141157
if ($value === null) {
142158
return $this;
143159
}
144160

161+
$this->validateValue($value);
162+
145163
if (! is_array($this->value)) {
146164
$this->value = [$this->value];
147165
}
@@ -193,4 +211,54 @@ public function __toString(): string
193211
{
194212
return $this->name . ': ' . $this->getValueLine();
195213
}
214+
215+
/**
216+
* Validate header name.
217+
*
218+
* Regex is based on code from a guzzlehttp/psr7 library.
219+
*
220+
* @see https://datatracker.ietf.org/doc/html/rfc7230#section-3.2
221+
*
222+
* @throws InvalidArgumentException
223+
*/
224+
private function validateName(string $name): void
225+
{
226+
if (preg_match('/^[a-zA-Z0-9\'`#$%&*+.^_|~!-]+$/D', $name) !== 1) {
227+
throw new InvalidArgumentException('The header name is not valid as per RFC 7230.');
228+
}
229+
}
230+
231+
/**
232+
* Validate header value.
233+
*
234+
* Regex is based on code from a guzzlehttp/psr7 library.
235+
*
236+
* @see https://datatracker.ietf.org/doc/html/rfc7230#section-3.2
237+
*
238+
* @param array<int|string, array<string, string>|string>|int|string $value
239+
*
240+
* @throws InvalidArgumentException
241+
*/
242+
private function validateValue(array|int|string $value): void
243+
{
244+
if (is_int($value)) {
245+
return;
246+
}
247+
248+
if (is_array($value)) {
249+
foreach ($value as $key => $val) {
250+
$this->validateValue($key);
251+
$this->validateValue($val);
252+
}
253+
254+
return;
255+
}
256+
257+
// The regular expression excludes obs-fold per RFC 7230#3.2.4, as sending folded lines
258+
// is deprecated and rare. This obscure HTTP/1.1 feature is unlikely to impact legitimate
259+
// use cases. Libraries like Guzzle and AMPHP follow the same principle.
260+
if (preg_match('/^[\x20\x09\x21-\x7E\x80-\xFF]*$/D', $value) !== 1) {
261+
throw new InvalidArgumentException('The header value is not valid as per RFC 7230.');
262+
}
263+
}
196264
}

‎system/Security/Security.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,26 +307,29 @@ private function getPostedToken(RequestInterface $request): ?string
307307
// Does the token exist in POST, HEADER or optionally php:://input - json data or PUT, DELETE, PATCH - raw data.
308308

309309
if ($tokenValue = $request->getPost($this->config->tokenName)) {
310-
return $tokenValue;
310+
return is_string($tokenValue) ? $tokenValue : null;
311311
}
312312

313-
if ($request->hasHeader($this->config->headerName)
314-
&& $request->header($this->config->headerName)->getValue() !== ''
315-
&& $request->header($this->config->headerName)->getValue() !== []) {
316-
return $request->header($this->config->headerName)->getValue();
313+
if ($request->hasHeader($this->config->headerName)) {
314+
$tokenValue = $request->header($this->config->headerName)->getValue();
315+
316+
return (is_string($tokenValue) && $tokenValue !== '') ? $tokenValue : null;
317317
}
318318

319319
$body = (string) $request->getBody();
320320

321321
if ($body !== '') {
322322
$json = json_decode($body);
323323
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
324-
return $json->{$this->config->tokenName} ?? null;
324+
$tokenValue = $json->{$this->config->tokenName} ?? null;
325+
326+
return is_string($tokenValue) ? $tokenValue : null;
325327
}
326328

327329
parse_str($body, $parsed);
330+
$tokenValue = $parsed[$this->config->tokenName] ?? null;
328331

329-
return $parsed[$this->config->tokenName] ?? null;
332+
return is_string($tokenValue) ? $tokenValue : null;
330333
}
331334

332335
return null;

‎tests/system/HTTP/HeaderTest.php‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
use CodeIgniter\Test\CIUnitTestCase;
1717
use Error;
18+
use InvalidArgumentException;
19+
use PHPUnit\Framework\Attributes\DataProvider;
1820
use PHPUnit\Framework\Attributes\Group;
1921
use stdClass;
2022

@@ -234,4 +236,83 @@ public function testHeaderToStringShowsEntireHeader(): void
234236

235237
$this->assertSame($expected, (string) $header);
236238
}
239+
240+
/**
241+
* @param string $name
242+
*/
243+
#[DataProvider('invalidNamesProvider')]
244+
public function testInvalidHeaderNames($name): void
245+
{
246+
$this->expectException(InvalidArgumentException::class);
247+
248+
new Header($name, 'text/html');
249+
}
250+
251+
/**
252+
* @return list<list<string>>
253+
*/
254+
public static function invalidNamesProvider(): array
255+
{
256+
return [
257+
["Content-Type\r\n\r\n"],
258+
["Content-Type\r\n"],
259+
["Content-Type\n"],
260+
["\tContent-Type\t"],
261+
["\n\nContent-Type\n\n"],
262+
["\r\nContent-Type"],
263+
["\nContent-Type"],
264+
["Content\r\n-Type"],
265+
["\n"],
266+
["\r\n"],
267+
["\t"],
268+
[' Content-Type '],
269+
['Content - Type'],
270+
["Content\x00Type"],
271+
[':Content-Type'],
272+
['Content-Type:'],
273+
[''],
274+
];
275+
}
276+
277+
/**
278+
* @param array<int|string, array<string, string>|string>|string|null $value
279+
*/
280+
#[DataProvider('invalidValuesProvider')]
281+
public function testInvalidHeaderValues($value): void
282+
{
283+
$this->expectException(InvalidArgumentException::class);
284+
285+
new Header('X-Test-Header', $value);
286+
}
287+
288+
/**
289+
* @return list<list<array<(int|string), string>|string>>
290+
*/
291+
public static function invalidValuesProvider(): array
292+
{
293+
return [
294+
["Header\n Value"],
295+
["Header\r\n Value"],
296+
["Header\r Value"],
297+
["Header Value\n"],
298+
["\nHeader Value"],
299+
["Header Value\r\n"],
300+
["\n\rHeader Value"],
301+
["\n\nHeader Value\n\n"],
302+
[
303+
["Header\n Value"],
304+
["Header\r\n Value"],
305+
],
306+
[
307+
[
308+
"Header\n" => 'Value',
309+
],
310+
],
311+
[
312+
[
313+
'Header' => "Value\r\n",
314+
],
315+
],
316+
];
317+
}
237318
}

‎tests/system/Security/SecurityTest.php‎

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use CodeIgniter\Test\Mock\MockSecurity;
2525
use Config\Security as SecurityConfig;
2626
use PHPUnit\Framework\Attributes\BackupGlobals;
27+
use PHPUnit\Framework\Attributes\DataProvider;
2728
use PHPUnit\Framework\Attributes\Group;
2829

2930
/**
@@ -42,13 +43,23 @@ protected function setUp(): void
4243
$this->resetServices();
4344
}
4445

45-
private function createMockSecurity(?SecurityConfig $config = null): MockSecurity
46+
private static function createMockSecurity(SecurityConfig $config = new SecurityConfig()): MockSecurity
4647
{
47-
$config ??= new SecurityConfig();
48-
4948
return new MockSecurity($config);
5049
}
5150

51+
private static function createIncomingRequest(): IncomingRequest
52+
{
53+
$config = new MockAppConfig();
54+
55+
return new IncomingRequest(
56+
$config,
57+
new SiteURI($config),
58+
null,
59+
new UserAgent(),
60+
);
61+
}
62+
5263
public function testBasicConfigIsSaved(): void
5364
{
5465
$security = $this->createMockSecurity();
@@ -108,18 +119,6 @@ public function testCSRFVerifyPostThrowsExceptionOnNoMatch(): void
108119
$security->verify($request);
109120
}
110121

111-
private function createIncomingRequest(): IncomingRequest
112-
{
113-
$config = new MockAppConfig();
114-
115-
return new IncomingRequest(
116-
$config,
117-
new SiteURI($config),
118-
null,
119-
new UserAgent(),
120-
);
121-
}
122-
123122
public function testCSRFVerifyPostReturnsSelfOnMatch(): void
124123
{
125124
$_SERVER['REQUEST_METHOD'] = 'POST';
@@ -315,4 +314,73 @@ public function testGetters(): void
315314
$this->assertIsString($security->getCookieName());
316315
$this->assertIsBool($security->shouldRedirect());
317316
}
317+
318+
public function testGetPostedTokenReturnsTokenFromPost(): void
319+
{
320+
$_POST['csrf_test_name'] = '8b9218a55906f9dcc1dc263dce7f005a';
321+
$request = $this->createIncomingRequest();
322+
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
323+
324+
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
325+
}
326+
327+
public function testGetPostedTokenReturnsTokenFromHeader(): void
328+
{
329+
$_POST = [];
330+
$request = $this->createIncomingRequest()->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a');
331+
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
332+
333+
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
334+
}
335+
336+
public function testGetPostedTokenReturnsTokenFromJsonBody(): void
337+
{
338+
$_POST = [];
339+
$jsonBody = json_encode(['csrf_test_name' => '8b9218a55906f9dcc1dc263dce7f005a']);
340+
$request = $this->createIncomingRequest()->setBody($jsonBody);
341+
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
342+
343+
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
344+
}
345+
346+
public function testGetPostedTokenReturnsTokenFromFormBody(): void
347+
{
348+
$_POST = [];
349+
$formBody = 'csrf_test_name=8b9218a55906f9dcc1dc263dce7f005a';
350+
$request = $this->createIncomingRequest()->setBody($formBody);
351+
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
352+
353+
$this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $method($request));
354+
}
355+
356+
#[DataProvider('provideGetPostedTokenReturnsNullForInvalidInputs')]
357+
public function testGetPostedTokenReturnsNullForInvalidInputs(string $case, IncomingRequest $request): void
358+
{
359+
$method = $this->getPrivateMethodInvoker($this->createMockSecurity(), 'getPostedToken');
360+
361+
$this->assertNull(
362+
$method($request),
363+
sprintf('Failed asserting that %s returns null on invalid input.', $case),
364+
);
365+
}
366+
367+
/**
368+
* @return iterable<string, array{string, IncomingRequest}>
369+
*/
370+
public static function provideGetPostedTokenReturnsNullForInvalidInputs(): iterable
371+
{
372+
$testCases = [
373+
'empty_post' => self::createIncomingRequest(),
374+
'invalid_post_data' => self::createIncomingRequest()->setGlobal('post', ['csrf_test_name' => ['invalid' => 'data']]),
375+
'empty_header' => self::createIncomingRequest()->setHeader('X-CSRF-TOKEN', ''),
376+
'invalid_json_data' => self::createIncomingRequest()->setBody(json_encode(['csrf_test_name' => ['invalid' => 'data']])),
377+
'invalid_json' => self::createIncomingRequest()->setBody('{invalid json}'),
378+
'missing_token_in_body' => self::createIncomingRequest()->setBody('other=value&another=test'),
379+
'invalid_form_data' => self::createIncomingRequest()->setBody('csrf_test_name[]=invalid'),
380+
];
381+
382+
foreach ($testCases as $case => $request) {
383+
yield $case => [$case, $request];
384+
}
385+
}
318386
}

0 commit comments

Comments
 (0)