Skip to content

Commit 7484ada

Browse files
committed
perf: inject Superglobals into IncomingRequest constructor instead of service() calls
- Add optional ?Superglobals parameter to constructor (defaults to service('superglobals') when null, fully backward compatible) - Replace service('superglobals') calls in isSecure(), getPostGet(), getGetPost() with ->superglobals property - Add tests verifying injected instance takes precedence over the service
1 parent b08160c commit 7484ada

2 files changed

Lines changed: 78 additions & 6 deletions

File tree

system/HTTP/IncomingRequest.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use CodeIgniter\HTTP\Exceptions\HTTPException;
1818
use CodeIgniter\HTTP\Files\FileCollection;
1919
use CodeIgniter\HTTP\Files\UploadedFile;
20+
use CodeIgniter\Superglobals;
2021
use Config\App;
2122
use Config\Services;
2223
use Locale;
@@ -123,18 +124,27 @@ class IncomingRequest extends Request
123124
*/
124125
protected $userAgent;
125126

127+
/**
128+
* Superglobals instance.
129+
*
130+
* @var Superglobals|null
131+
*/
132+
protected $superglobals;
133+
126134
/**
127135
* Constructor
128136
*
129137
* @param App $config
130138
* @param string|null $body
131139
*/
132-
public function __construct($config, ?URI $uri = null, $body = 'php://input', ?UserAgent $userAgent = null)
140+
public function __construct($config, ?URI $uri = null, $body = 'php://input', ?UserAgent $userAgent = null, ?Superglobals $superglobals = null)
133141
{
134142
if (! $uri instanceof URI || ! $userAgent instanceof UserAgent) {
135143
throw new InvalidArgumentException('You must supply the parameters: uri, userAgent.');
136144
}
137145

146+
$this->superglobals = $superglobals ?? service('superglobals');
147+
138148
$this->populateHeaders();
139149

140150
if (
@@ -266,7 +276,7 @@ public function isAJAX(): bool
266276
*/
267277
public function isSecure(): bool
268278
{
269-
$https = service('superglobals')->server('HTTPS');
279+
$https = $this->superglobals->server('HTTPS');
270280

271281
if ($https !== null && strtolower($https) !== 'off') {
272282
return true;
@@ -601,9 +611,11 @@ public function getPostGet($index = null, $filter = null, $flags = null)
601611
// Use $_POST directly here, since filter_has_var only
602612
// checks the initial POST data, not anything that might
603613
// have been added since.
604-
return service('superglobals')->post($index) !== null
614+
$superglobals = $this->superglobals;
615+
616+
return $superglobals->post($index) !== null
605617
? $this->getPost($index, $filter, $flags)
606-
: (service('superglobals')->get($index) !== null ? $this->getGet($index, $filter, $flags) : $this->getPost($index, $filter, $flags));
618+
: ($superglobals->get($index) !== null ? $this->getGet($index, $filter, $flags) : $this->getPost($index, $filter, $flags));
607619
}
608620

609621
/**
@@ -624,9 +636,11 @@ public function getGetPost($index = null, $filter = null, $flags = null)
624636
// Use $_GET directly here, since filter_has_var only
625637
// checks the initial GET data, not anything that might
626638
// have been added since.
627-
return service('superglobals')->get($index) !== null
639+
$superglobals = $this->superglobals;
640+
641+
return $superglobals->get($index) !== null
628642
? $this->getGet($index, $filter, $flags)
629-
: (service('superglobals')->post($index) !== null ? $this->getPost($index, $filter, $flags) : $this->getGet($index, $filter, $flags));
643+
: ($superglobals->post($index) !== null ? $this->getPost($index, $filter, $flags) : $this->getGet($index, $filter, $flags));
630644
}
631645

632646
/**

tests/system/HTTP/IncomingRequestTest.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,5 +1167,63 @@ public function testGetIPAddressThruProxyInvalidConfigArray(): void
11671167
$this->request->getIPAddress();
11681168
}
11691169

1170+
public function testInjectedSuperglobalsIsSecure(): void
1171+
{
1172+
$superglobals = new Superglobals(server: ['HTTPS' => 'on']);
1173+
1174+
$config = new App();
1175+
$uri = new SiteURI($config);
1176+
$request = new IncomingRequest($config, $uri, null, new UserAgent(), $superglobals);
1177+
1178+
$this->assertTrue($request->isSecure());
1179+
}
1180+
1181+
public function testInjectedSuperglobalsIsSecureNoOverride(): void
1182+
{
1183+
service('superglobals')->setServer('HTTPS', 'on');
1184+
1185+
$superglobals = new Superglobals(server: []);
1186+
1187+
$config = new App();
1188+
$uri = new SiteURI($config);
1189+
$request = new IncomingRequest($config, $uri, null, new UserAgent(), $superglobals);
1190+
1191+
$this->assertFalse($request->isSecure());
1192+
}
1193+
1194+
public function testInjectedSuperglobalsGetPostGet(): void
1195+
{
1196+
// Service has only GET data
1197+
service('superglobals')->setGet('key', 'service_get');
1198+
1199+
// Injected mock has only POST data
1200+
$superglobals = new Superglobals(post: ['key' => 'mock_post'], get: []);
1201+
1202+
$config = new App();
1203+
$uri = new SiteURI($config);
1204+
$request = new IncomingRequest($config, $uri, null, new UserAgent(), $superglobals);
1205+
1206+
// Mock says POST exists -> choose POST -> service has no POST -> null
1207+
// Without injection, would have found no POST in service, fallen back to GET -> 'service_get'
1208+
$this->assertNull($request->getPostGet('key'));
1209+
}
1210+
1211+
public function testInjectedSuperglobalsGetGetPost(): void
1212+
{
1213+
// Service has only POST data
1214+
service('superglobals')->setPost('key', 'service_post');
1215+
1216+
// Injected mock has only GET data
1217+
$superglobals = new Superglobals(get: ['key' => 'mock_get'], post: []);
1218+
1219+
$config = new App();
1220+
$uri = new SiteURI($config);
1221+
$request = new IncomingRequest($config, $uri, null, new UserAgent(), $superglobals);
1222+
1223+
// Mock says GET exists -> choose GET -> service has no GET -> null
1224+
// Without injection, would have found no GET in service, fallen back to POST -> 'service_post'
1225+
$this->assertNull($request->getGetPost('key'));
1226+
}
1227+
11701228
// @TODO getIPAddress should have more testing, to 100% code coverage
11711229
}

0 commit comments

Comments
 (0)