Merge pull request #6089 from kenjis/fix-4.2-cli-filter-bug

fix: when running on CLI, two Request objects were used in the system
codeigniter4 · Jun 26, 2022 · 8a70700 · 8a70700
2 parents 1699a51 + caa2aad
commit 8a70700
Show file tree

Hide file tree

Showing 11 changed files with 81 additions and 21 deletions.
diff --git a/system/API/ResponseTrait.php b/system/API/ResponseTrait.php
@@ -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);

diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php
@@ -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()
  {
@@ -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();
  }
 
  /**

diff --git a/system/Config/BaseService.php b/system/Config/BaseService.php
@@ -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)
@@ -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)
@@ -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)

diff --git a/system/Config/Services.php b/system/Config/Services.php
@@ -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)
  {
@@ -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(

diff --git a/system/Debug/Exceptions.php b/system/Debug/Exceptions.php
@@ -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;
@@ -50,9 +51,9 @@ class Exceptions
  protected $config;
 
  /**
- * The incoming request.
+ * The request.
  *
- * @var IncomingRequest
+ * @var CLIRequest|IncomingRequest
  */
  protected $request;
 
@@ -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;

diff --git a/system/HTTP/CLIRequest.php b/system/HTTP/CLIRequest.php
@@ -212,6 +212,6 @@ protected function parseCommand()
  */
  public function isCLI(): bool
  {
- return is_cli();
+ return true;
  }
 }
diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php
@@ -347,7 +347,7 @@ public function negotiate(string $type, array $supported, bool $strictMatch = fa
  */
  public function isCLI(): bool
  {
- return is_cli();
+ return false;
  }
 
  /**

diff --git a/tests/system/CommonSingleServiceTest.php b/tests/system/CommonSingleServiceTest.php
@@ -95,6 +95,7 @@ public static function serviceNamesProvider(): iterable
  static $services = [];
  static $excl = [
  '__callStatic',
+ 'createRequest',
  'serviceExists',
  'reset',
  'resetSingle',

diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php
@@ -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()

diff --git a/user_guide_src/source/changelogs/index.rst b/user_guide_src/source/changelogs/index.rst
@@ -12,6 +12,7 @@ See all the changes.
 .. toctree::
  :titlesonly:
 
+ v4.2.2
  v4.2.1
  v4.2.0
  v4.1.9

diff --git a/user_guide_src/source/changelogs/v4.2.2.rst b/user_guide_src/source/changelogs/v4.2.2.rst
@@ -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
 ************
@@ -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
 **********