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: when running on CLI, two Request objects were used in the system #6089

Merged
merged 10 commits into from
Jun 26, 2022
11 changes: 9 additions & 2 deletions system/API/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,15 @@ protected function format($data = null)
$mime = "application/{$this->format}";

// Determine correct response type through content negotiation if not explicitly declared
if (empty($this->format) || ! in_array($this->format, ['json', 'xml'], true)) {
$mime = $this->request->negotiate('media', $format->getConfig()->supportedResponseFormats, false);
if (
(empty($this->format) || ! in_array($this->format, ['json', 'xml'], true))
&& $this->request instanceof IncomingRequest
) {
$mime = $this->request->negotiate(
'media',
$format->getConfig()->supportedResponseFormats,
false
);
}

$this->response->setContentType($mime);
Expand Down
12 changes: 5 additions & 7 deletions system/CodeIgniter.php
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,7 @@ public function setRequest(Request $request)
}

/**
* Get our Request object, (either IncomingRequest or CLIRequest)
* and set the server protocol based on the information provided
* by the server.
* Get our Request object, (either IncomingRequest or CLIRequest).
*/
protected function getRequestObject()
{
Expand All @@ -594,12 +592,12 @@ protected function getRequestObject()
}

if ($this->isSparked() || $this->isPhpCli()) {
$this->request = Services::clirequest($this->config);
Services::createRequest($this->config, true);
} else {
$this->request = Services::request($this->config);
// guess at protocol if needed
$this->request->setProtocolVersion($_SERVER['SERVER_PROTOCOL'] ?? 'HTTP/1.1');
Services::createRequest($this->config);
}

$this->request = Services::request();
}

/**
Expand Down
4 changes: 3 additions & 1 deletion system/Config/BaseService.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
* @method static CLIRequest clirequest(App $config = null, $getShared = true)
* @method static CodeIgniter codeigniter(App $config = null, $getShared = true)
* @method static Commands commands($getShared = true)
* @method static void createRequest(App $config, bool $isCli = false)
* @method static ContentSecurityPolicy csp(CSPConfig $config = null, $getShared = true)
* @method static CURLRequest curlrequest($options = [], ResponseInterface $response = null, App $config = null, $getShared = true)
* @method static Email email($config = null, $getShared = true)
Expand All @@ -105,6 +106,7 @@
* @method static Format format(ConfigFormat $config = null, $getShared = true)
* @method static Honeypot honeypot(ConfigHoneyPot $config = null, $getShared = true)
* @method static BaseHandler image($handler = null, Images $config = null, $getShared = true)
* @method static IncomingRequest incomingrequest(?App $config = null, bool $getShared = true)
* @method static Iterator iterator($getShared = true)
* @method static Language language($locale = null, $getShared = true)
* @method static Logger logger($getShared = true)
Expand All @@ -114,7 +116,7 @@
* @method static Parser parser($viewPath = null, ConfigView $config = null, $getShared = true)
* @method static RedirectResponse redirectresponse(App $config = null, $getShared = true)
* @method static View renderer($viewPath = null, ConfigView $config = null, $getShared = true)
* @method static IncomingRequest request(App $config = null, $getShared = true)
* @method static IncomingRequest|CLIRequest request(App $config = null, $getShared = true)
* @method static Response response(App $config = null, $getShared = true)
* @method static Router router(RouteCollectionInterface $routes = null, Request $request = null, $getShared = true)
* @method static RouteCollection routes($getShared = true)
Expand Down
49 changes: 47 additions & 2 deletions system/Config/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public static function cache(?Cache $config = null, bool $getShared = true)
* a command line request.
*
* @return CLIRequest
*
* @internal
*/
public static function clirequest(?App $config = null, bool $getShared = true)
{
Expand Down Expand Up @@ -470,16 +472,59 @@ public static function renderer(?string $viewPath = null, ?ViewConfig $config =
}

/**
* The Request class models an HTTP request.
* Returns the current Request object.
*
* @return IncomingRequest
* createRequest() injects IncomingRequest or CLIRequest.
*
* @return CLIRequest|IncomingRequest
*
* @deprecated The parameter $config and $getShared are deprecated.
*/
public static function request(?App $config = null, bool $getShared = true)
{
if ($getShared) {
return static::getSharedInstance('request', $config);
}

// @TODO remove the following code for backward compatibility
return static::incomingrequest($config, $getShared);
}

/**
* Create the current Request object, either IncomingRequest or CLIRequest.
*
* This method is called from CodeIgniter::getRequestObject().
*
* @internal
*/
public static function createRequest(App $config, bool $isCli = false): void
{
if ($isCli) {
$request = AppServices::clirequest($config);
} else {
$request = AppServices::incomingrequest($config);

// guess at protocol if needed
$request->setProtocolVersion($_SERVER['SERVER_PROTOCOL'] ?? 'HTTP/1.1');
}

// Inject the request object into Services::request().
static::$instances['request'] = $request;
}

/**
* The IncomingRequest class models an HTTP request.
*
* @return IncomingRequest
*
* @internal
*/
public static function incomingrequest(?App $config = null, bool $getShared = true)
{
if ($getShared) {
return static::getSharedInstance('request', $config);
}

$config ??= config('App');

return new IncomingRequest(
Expand Down
10 changes: 7 additions & 3 deletions system/Debug/Exceptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use CodeIgniter\API\ResponseTrait;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\CLIRequest;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\Response;
use Config\Exceptions as ExceptionsConfig;
Expand Down Expand Up @@ -50,9 +51,9 @@ class Exceptions
protected $config;

/**
* The incoming request.
* The request.
*
* @var IncomingRequest
* @var CLIRequest|IncomingRequest
*/
protected $request;

Expand All @@ -63,7 +64,10 @@ class Exceptions
*/
protected $response;

public function __construct(ExceptionsConfig $config, IncomingRequest $request, Response $response)
/**
* @param CLIRequest|IncomingRequest $request
*/
public function __construct(ExceptionsConfig $config, $request, Response $response)
{
$this->ob_level = ob_get_level();
$this->viewPath = rtrim($config->errorViewPath, '\\/ ') . DIRECTORY_SEPARATOR;
Expand Down
2 changes: 1 addition & 1 deletion system/HTTP/CLIRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,6 @@ protected function parseCommand()
*/
public function isCLI(): bool
{
return is_cli();
return true;
}
}
2 changes: 1 addition & 1 deletion system/HTTP/IncomingRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public function negotiate(string $type, array $supported, bool $strictMatch = fa
*/
public function isCLI(): bool
{
return is_cli();
return false;
}

/**
Expand Down
1 change: 1 addition & 0 deletions tests/system/CommonSingleServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public static function serviceNamesProvider(): iterable
static $services = [];
static $excl = [
'__callStatic',
'createRequest',
'serviceExists',
'reset',
'resetSingle',
Expand Down
3 changes: 1 addition & 2 deletions tests/system/HTTP/IncomingRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,7 @@ public function testCanGrabGetRawInput()

public function testIsCLI()
{
// this should be the case in unit testing
$this->assertTrue($this->request->isCLI());
$this->assertFalse($this->request->isCLI());
}

public function testIsAJAX()
Expand Down
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ See all the changes.
.. toctree::
:titlesonly:

v4.2.2
v4.2.1
v4.2.0
v4.1.9
Expand Down
7 changes: 5 additions & 2 deletions user_guide_src/source/changelogs/v4.2.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ Release Date: Unreleased
BREAKING
********

none.
- Now ``Services::request()`` returns ``IncomingRequest`` or ``CLIRequest``.
- The method signature of ``CodeIgniter\Debug\Exceptions::__construct()`` has been changed. The ``IncomingRequest`` typehint on the ``$request`` parameter was removed. Extending classes should likewise remove the parameter so as not to break LSP.

Enhancements
************
Expand All @@ -23,11 +24,13 @@ Changes
*******

- Fixed: ``BaseBuilder::increment()`` and ``BaseBuilder::decrement()`` do not reset the ``BaseBuilder`` state after a query.
- Now ``CLIRequest::isCLI()`` always returns true.
- Now ``IncommingRequest::isCLI()`` always returns false.

Deprecations
************

none.
- The parameters of ``Services::request()`` are deprecated.

Bugs Fixed
**********
Expand Down