codeigniter4
feat: Improve `Superglobals` service
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:
- [x] Securely signed commits
- [ ] Component(s) with PHPDoc blocks, only if necessary or adds value
- [ ] Unit testing, with >80% coverage
- [ ] User guide updated
- [x] Conforms to style guide
I applied some suggestions. Questions remain:
- Do I need a
boolinInputParameters? If I look at_GET, I don't see the case. - Do I need the fluid object?
$p->set()->set() - Is there a better way to replace
bin2hex(random_bytes(8))inget()?
- 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
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)
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.
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.
@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.
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.
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.
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?
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.
The discussion did not work out. Should I abandon the idea? Apart from me, hardly anyone else will continue development.