feat: complete `Superglobals` implementation #9858

michalsn · 2025-12-28T21:30:10Z

Description
This PR completes the implementation of the Superglobals class and migrates the entire codebase to use it consistently.

I started work on this about a month ago, and it turned out to be a tedious task. The scope of changes is relatively large, as this migration could not be done incrementally.

Since this class is internal and not intended for end-user consumption, no changelog entry has been added.

See: #7866

Checklist:

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

system/HTTP/RequestTrait.php

neznaika0

I patched the latest commit 4.7 - there is an error.

1) CodeIgniter\Models\PaginateModelTest::testMultiplePager
Failed asserting that '<!-- DEBUG-VIEW START 1 SYSTEMPATH/Pager/Views/default_full.php -->\n
...
<!-- DEBUG-VIEW ENDED 1 SYSTEMPATH/Pager/Views/default_full.php -->\n
' [ASCII](length: 843) contains "?page_valid=1"" [ASCII](length: 14).

/development/tests/system/Models/PaginateModelTest.php:103

A lot of changes - I couldn't see them all at once.

UPD: Fixes #9392

system/Superglobals.php

paulbalandan · 2025-12-29T10:16:38Z

system/Superglobals.php

+    public function __construct(
+        ?array $server = null,
+        ?array $get = null,
+        ?array $post = null,
+        ?array $cookie = null,
+        ?array $files = null,
+        ?array $request = null,
+    ) {
+        $this->server  = $server ?? $_SERVER;
+        $this->get     = $get ?? $_GET;
+        $this->post    = $post ?? $_POST;
+        $this->cookie  = $cookie ?? $_COOKIE;
+        $this->files   = $files ?? $_FILES;
+        $this->request = $request ?? $_REQUEST;
    }


Can we use constructor property promotion here and instead of null default we use directly the superglobals.

Suggested change

public function __construct(

?array $server = null,

?array $get = null,

?array $post = null,

?array $cookie = null,

?array $files = null,

?array $request = null,

) {

$this->server = $server ?? $_SERVER;

$this->get = $get ?? $_GET;

$this->post = $post ?? $_POST;

$this->cookie = $cookie ?? $_COOKIE;

$this->files = $files ?? $_FILES;

$this->request = $request ?? $_REQUEST;

}

public function __construct(

private array $server = $_SERVER,

private array $get = $_GET,

private array $post = $_POST,

private array $cookie = $_COOKIE,

private array $files = $_FILES,

private array $request = $_REQUEST,

) {

}

Sadly, we can't do that with superglobals, but I introduced property promotion.

paulbalandan · 2025-12-29T10:18:07Z

system/Superglobals.php

+     *
+     * @return array<array-key, mixed>|float|int|string|null
+     */
+    public function server(string $key): array|float|int|string|null


For the getters, how about the idea of introducing a default parameter, liked mixed $default = null so that on invocation we can define already the default value in case missing instead of doing ?? <default>?

paulbalandan · 2025-12-29T10:25:08Z

system/Superglobals.php

+    private array $files;
+
+    /**
+     * @param array<string, array|float|int|string>|null $server


There's currently a disconnect in this PR for the types a superglobal can set vs the types it can get. The set types is currently less precise than the get types. To make sync easier, I suggest to add phpstan-types in the class declaration phpdoc for each superglobal, something like:

/** * @phpstan-type server_items array<array-key, mixed>|float|int|string * etc. */ class Superglobals

Then here and the setters and getters below we can simply plug the phpstan type, like so:

Suggested change

* @param array<string, array|float|int|string>|null $server

* @param array<string, server_items>|null $server

Done... I think. Please verify when you have time.

michalsn · 2025-12-29T15:00:18Z

@neznaika0 What do you mean by saying "I patched the latest commit 4.7"? Have you cloned this branch and run tests locally? This branch is working on the latest 4.7.

Anyway, I found the potential issue and hopefully fixed it. The strange thing is that it didn't pop up during tests.

neznaika0 · 2025-12-29T15:07:43Z

@michalsn I use few git features - it's easy to lose changes. I downloaded https://github.com/codeigniter4/CodeIgniter4/pull/9858.patch and I applied it on 4.7 at local.

michalsn · 2025-12-29T15:10:02Z

@neznaika0 Ok, please verify if the issue still exists after recent changes - thanks.

neznaika0 · 2025-12-29T15:12:17Z

system/Superglobals.php

+     * @return server_items|null
     */
-    public function server(string $key): array|float|int|string|null
+    public function server(string $key, mixed $default = null): array|float|int|string|null


We have an mixed argument, since this should match server_items (and other places), you can add PHPDoc.

I'm sorry if this looks like a complication. You can leave it as it is

Fair point - fixed.

neznaika0 · 2025-12-29T15:22:50Z

Good. Error is missing

neznaika0 · 2025-12-29T18:32:27Z

tests/system/SuperglobalsTest.php

+        $this->superglobals->setGlobalArray('invalid', ['key' => 'value']);
+
+        // Should not throw exception, just does nothing
+        $this->assertTrue(true);


It doesn't make sense. Maybe you should check:

Suggested change

$this->assertTrue(true);

$this->assertSame([], $this->superglobals->getGlobalArray('invalid'));

?

A better idea is probably throwing an exception for an invalid superglobal. I've sent the changes.

neznaika0 · 2025-12-31T08:09:46Z

tests/system/Filters/InvalidCharsTest.php

        $_GET    = [];
        $_POST   = [];
        $_COOKIE = [];


It doesn't look necessary.

Services::injectMock('superglobals', new Superglobals([], [], [], []));

neznaika0 · 2025-12-31T08:17:56Z

system/Superglobals.php

+     *
+     * @param server_items $value
+     */
+    public function setServer(string $key, array|float|int|string $value): void


Can we add a fluid interface for all setServer(), setPost(),...? This shortens the service('superglobals') call further in the code:

$superglobals = service('superglobals'); $superglobals->setGetArray([]); $superglobals->setPostArray([]); $superglobals->setCookieArray([]); $superglobals = service('superglobals') ->setGetArray([]) ->setPostArray([]) ->setCookieArray([]);

If you agree, you will need to specify the return type self.

And unset*() too

neznaika0 · 2025-12-31T08:25:07Z

tests/system/HTTP/CLIRequestTest.php

        $_POST = [];
        $_GET  = [];


It doesn't look necessary

neznaika0 · 2025-12-31T08:51:37Z

tests/system/HTTP/MessageTest.php


        putenv("CONTENT_TYPE={$originalEnv}");
-        $_SERVER = $original; // restore so code coverage doesn't break
+        $superglobals->setServerArray($original); // restore so code coverage doesn't break


So far, we can't globally enable backupGlobals in the config, but it works for the file. Here and further in the file you can delete $original.

#[Group('Others')] #[BackupGlobals(true)] final class MessageTest extends CIUnitTestCase

neznaika0 · 2025-12-31T08:54:17Z

tests/system/HTTP/RequestTest.php

        $_POST = [];
        $_GET  = [];


It doesn't look necessary.

Services::injectMock('superglobals', new Superglobals([], [], []));

neznaika0 · 2025-12-31T09:07:46Z

tests/system/Security/SecurityCSRFCookieRandomizeTokenTest.php

    {
        parent::setUp();

        $_COOKIE = [];


It doesn't look necessary.

Services::injectMock('superglobals', new Superglobals([], [], [], []));

neznaika0 · 2025-12-31T09:08:42Z

tests/system/Security/SecurityTest.php

    {
        parent::setUp();

        $_COOKIE = [];


It doesn't look necessary. As above

neznaika0 · 2025-12-31T09:09:24Z

tests/system/Session/SessionTest.php

    {
        parent::setUp();

        $_COOKIE  = [];


It doesn't look necessary. As above

neznaika0 · 2025-12-31T09:10:38Z

tests/system/SuperglobalsTest.php

+        parent::setUp();
+
+        // Clear superglobals before each test
+        $_SERVER = $_GET = $_POST = $_COOKIE = $_FILES = $_REQUEST = [];


It doesn't look necessary. As above

neznaika0 · 2025-12-31T09:16:37Z

system/Superglobals.php

+        $this->server ??= $_SERVER;
+        $this->get ??= $_GET;
+        $this->post ??= $_POST;
+        $this->cookie ??= $_COOKIE;
+        $this->files ??= $_FILES;
+        $this->request ??= $_REQUEST;


Check out the comments below. It seems that we need the Superglobals::createDummy() method with empty values for testing.

If you do not agree, you should set all zeroing of global variables in the tests as Services::injectMock.('superglobals', new Superglobals([], [], [], [], [], [])) or call setServerArray([]), setPostArray([])...

michalsn added 11 commits December 28, 2025 17:11

feat: superglobals

d60b0ab

continue with superglobals

1b1440f

update tests

0383f06

update phpstan baseline

d4977d0

get rid of getSuperglobals

b3eb0cc

add files handling to superglobals

294e62e

update tests for files superglobals

2cc0e58

reorder superglobals params

5ba09f2

update tests

670bbe5

update tests

9a583f2

phpstan baseline

d3bd139

michalsn added enhancement PRs that improve existing functionalities refactor Pull requests that refactor code 4.7 labels Dec 28, 2025

datamweb reviewed Dec 29, 2025

View reviewed changes

system/HTTP/RequestTrait.php Outdated Show resolved Hide resolved

michalsn added 2 commits December 29, 2025 06:59

cleanup

fd4bf4a

cleanup

79091a3

neznaika0 suggested changes Dec 29, 2025

View reviewed changes

system/Superglobals.php Outdated Show resolved Hide resolved

system/Superglobals.php Outdated Show resolved Hide resolved

system/Superglobals.php Outdated Show resolved Hide resolved

michalsn added 2 commits December 29, 2025 08:50

apply code suggestions

84c9cef

update tests

770afff

paulbalandan reviewed Dec 29, 2025

View reviewed changes

types rework for superglobals class

519e2a3

neznaika0 reviewed Dec 29, 2025

View reviewed changes

add PHPDocs for mixed params

3303443

neznaika0 reviewed Dec 29, 2025

View reviewed changes

prevent silent failure when accessing an invalid superglobal

863789d

neznaika0 suggested changes Dec 31, 2025

View reviewed changes

	* @param array<string, array\|float\|int\|string>\|null $server
	* @param array<string, server_items>\|null $server

	$this->assertTrue(true);
	$this->assertSame([], $this->superglobals->getGlobalArray('invalid'));

feat: complete Superglobals implementation #9858

Are you sure you want to change the base?

feat: complete Superglobals implementation #9858

Conversation

michalsn commented Dec 28, 2025

Uh oh!

Uh oh!

neznaika0 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michalsn commented Dec 29, 2025

Uh oh!

neznaika0 commented Dec 29, 2025

Uh oh!

michalsn commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neznaika0 commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: complete `Superglobals` implementation #9858

feat: complete `Superglobals` implementation #9858

neznaika0 left a comment •

edited

Loading