feat: Improve Superglobals service

[neznaika0]

Description
Fixes #7866
Fixes #9392

We need to discuss further actions.

I suggest an option for working with global variables: hide $_SERVER, $_POST, ... in Parameters objects.

It is necessary to decide which types of data to allow. Globally, Parameters can store anything.
InputParameters only have compatible values for $_POST, $_GET,... (string, int, float, bool).
ServerParameters for $_SERVER (it is possible to allow storage of objects?).

As soon as you say that you agree to such changes, you need to decide on the initialization location:
Should I replace it in Request, Superglobals, or only in Superlglobals?

Next, we'll update the methods for Superglobals:

service('superglobals')->query()->get('key')   // $_GET['key']
service('superglobals')->post()->get('key')    // $_POST['key']
service('superglobals')->server()->get('host') // $_SERVER['host']

// We are not updating $_GET['key'] yet.
// It's worth discussing how to update global variables back.
service('superglobals')->query()->set('key', 123)

service('superglobals')->query()->all()  // array

The userguide update can wait for now.
Ask questions.

P.S. I've looked at the Symfony code, and I think it can be adapted to our needs.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[neznaika0]

Can someone sync the v4.7 branch?

[paulbalandan]

Can someone sync the v4.7 branch?

Synced

[paulbalandan]

I know it's still a draft, but I have left initial comments on the design. No review yet on the implementation.

[paulbalandan]

I think this class should be abstract by itself, unless there's a superglobal that is being represented by this?

[neznaika0]

It will not be a global object. It might be worth making it abstract. Let's wait for the other participants.
But I wrote general tests for it.

[neznaika0]

I applied some suggestions.
Questions remain:

1. Do I need a bool in InputParameters? If I look at _GET, I don't see the case.
2. Do I need the fluid object? $p->set()->set()
3. Is there a better way to replace bin2hex(random_bytes(8)) in get()?

[paulbalandan]

I understand now the issue. You need to specify another template for this default. Like:

Suggested change

	* @param TKey $key
	* @param TValue $default
	*
	* @return TValue|null
	*/
	* @template TDefault
	*
	* @param TKey $key
	* @param TDefault $default
	*
	* @return TValue|TDefault|null
	*/

[neznaika0]

That's the problem.
I didn't plan on null in scalar data. $_GET['a'] = null from the request will never exist. Yes, this is acceptable in other places.

Are there any examples for null tests?

[paulbalandan]

This is the tinkering I made to allow strict default value and not allowing null for InputParameters.

diff --git a/system/HTTP/Parameters/InputParameters.php b/system/HTTP/Parameters/InputParameters.php
index 8aa0508075..a45f7b22bd 100644
--- a/system/HTTP/Parameters/InputParameters.php
+++ b/system/HTTP/Parameters/InputParameters.php
@@ -19,8 +19,9 @@ use CodeIgniter\Exceptions\RuntimeException;
 /**
  * @template TKey of string
  * @template TValue of scalar|array<(int|string), mixed>
+ * @template TDefault of scalar|array<(int|string), mixed>
  *
- * @extends Parameters<TKey, TValue>
+ * @extends Parameters<TKey, TValue, TDefault>
  *
  * @see \CodeIgniter\HTTP\Parameters\InputParametersTest
  */
@@ -35,7 +36,7 @@ class InputParameters extends Parameters
         }
     }
 
-    public function get(string $key, mixed $default = null): mixed
+    public function get(string $key, mixed $default = ''): mixed
     {
         if ($default !== null && ! is_scalar($default)) {
             throw new InvalidArgumentException(sprintf('The default value for the InputParameters must be a scalar type, "%s" given.', gettype($defau
lt)));
diff --git a/system/HTTP/Parameters/Parameters.php b/system/HTTP/Parameters/Parameters.php
index 20f9b7d66c..8629f750d2 100644
--- a/system/HTTP/Parameters/Parameters.php
+++ b/system/HTTP/Parameters/Parameters.php
@@ -19,8 +19,9 @@ use CodeIgniter\Exceptions\RuntimeException;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
- * @implements ParametersInterface<TKey, TValue>
+ * @implements ParametersInterface<TKey, TValue, TDefault>
  *
  * @see \CodeIgniter\HTTP\Parameters\ParametersTest
  */
diff --git a/system/HTTP/Parameters/ParametersInterface.php b/system/HTTP/Parameters/ParametersInterface.php
index 6873e78f78..21ccd09744 100644
--- a/system/HTTP/Parameters/ParametersInterface.php
+++ b/system/HTTP/Parameters/ParametersInterface.php
@@ -19,6 +19,7 @@ use IteratorAggregate;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
  * @extends IteratorAggregate<TKey, TValue>
  */
@@ -41,9 +42,9 @@ interface ParametersInterface extends IteratorAggregate, Countable
 
     /**
      * @param TKey   $key
-     * @param TValue $default
+     * @param TDefault $default
      *
-     * @return TValue|null
+     * @return TValue|TDefault|null
      */
     public function get(string $key, mixed $default = null): mixed;
 
diff --git a/system/HTTP/Parameters/ServerParameters.php b/system/HTTP/Parameters/ServerParameters.php
index c6e87d49ae..0bbf8f2b07 100644
--- a/system/HTTP/Parameters/ServerParameters.php
+++ b/system/HTTP/Parameters/ServerParameters.php
@@ -16,8 +16,9 @@ namespace CodeIgniter\HTTP\Parameters;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
- * @extends Parameters<TKey, TValue>
+ * @extends Parameters<TKey, TValue, TDefault>
  */
 class ServerParameters extends Parameters
 {

[paulbalandan]

1. Do I need a bool in InputParameters? If I look at _GET, I don't see the case.

I think yes. For queries like example.com/page.php?num=123&flag=true

[neznaika0]

No.
/?num=123&flag=true&nullable=null&float=10.13&int=2024&null
var_dump($_GET);

array (size=6)
  'num' => string '123' (length=3)
  'flag' => string 'true' (length=4)
  'nullable' => string 'null' (length=4)
  'float' => string '10.13' (length=5)
  'int' => string '2024' (length=4)
  'null' => string '' (length=0)

[paulbalandan]

No. /?num=123&flag=true&nullable=null&float=10.13&int=2024&null var_dump($_GET);

array (size=6)
  'num' => string '123' (length=3)
  'flag' => string 'true' (length=4)
  'nullable' => string 'null' (length=4)
  'float' => string '10.13' (length=5)
  'int' => string '2024' (length=4)
  'null' => string '' (length=0)

Isn't it default for $_GET to return string for these? Then, it will just be our job to convert them to the proper types.

[neznaika0]

Probably. I'm not a cool architect) Therefore, it is necessary to discuss.
Type converting can cause additional problems. How do you see this case? If we convert while saving to storage, we will get an incorrect $_GET during the dump.

$_GET = ['int' => '1000'];
$p    = new InputParameters($_GET);
$_GET = $p->all(); // or back set
$p->all();         // ['int' => 1000] I think it's bad

Symfony has additional methods for this, but I wouldn't want to go that far now.

[paulbalandan]

Ok. Let's leave this for discussion.

[neznaika0]

@michalsn @samsonasik @MGatner @lonnieezell do you have an opinion on the feature? We have few people to discuss this with, I hope we will come to a result.

[michalsn]

What was the general idea behind superglobals? I believe it was primarily intended to facilitate mocking.

For this reason, I feel that the proposed solution is somewhat overengineering. In my opinion, only string values should be returned, as the server does not typically handle other types in a normal scenario. We risk breaking existing applications if we suddenly start casting values to specific types.

Moreover, superglobals are meant to be used everywhere by default, including in $request->getPost() etc., where filters can be applied. These filters will convert values to strings anyway.

[neznaika0]

The reason for replacing global variables is the possibility of abstraction and, in general, their use is not recommended. Plus, kenjis offered the job, which means he accepted the situation.

Regarding typing, I also suggest a string type for GET POST, but the SERVER can have scalar types.

[michalsn]

The reason for replacing global variables is the possibility of abstraction and, in general, their use is not recommended.

At the moment, we don’t directly use superglobals in most places - instead, we use an additional layer to access them. The use of superglobals is discouraged mainly because of the lack of sanitization - which we are already handling.

Scalar types in PHP are well-defined, and typically, $_SERVER handles only one of them: strings. The exceptions are REQUEST_TIME and REQUEST_TIME_FLOAT, which are the only values in $_SERVER that are int/float (at least from what I know). There are no boolean values in $_SERVER by default.

But again - currently everything from $request->getServer() will be a string.

I'm not suggesting that superglobals (as a class) are a bad idea, but I would rather prefer a simpler implementation. In particular, I would extract the functionality we currently have in IncomingRequest and RequestTrait and build superglobals based on that.

The question is, what real problem are we trying to solve by implementing the superglobals class?

[neznaika0]

It's not that much of a problem. This is a natural movement forward. Similar to using PSR.

But testing based on _POST _FILES doesn't seem like a good example.