Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[HttpFoundation] IpUtils::$checkedIps is never cleared #50032

Copy link
Copy link
Closed
@danielburger1337

Description

@danielburger1337
Issue body actions

Symfony version(s) affected

5.4, 6.2

Description

Foreword: I don't really know how I can make this bug report more concise while still presenting my entire thought process.
Sorry for the long read!

To avoid duplicate work, the IpUtils utility class caches its IP validation results but never clears its cache.

Because Symfony is also supporting "infinite" runtimes like swoole or FrankenPHP, the memory usage of this array may become pretty sizeable if either an application checks many IP Adresses per request or the runner is running for a long time.

I might be missing something because I would be kind of surprised if no one encountered this memory usage issue before. So there might be a way already built in by the runtimes that garbage collect static arrays like this.

The best comparison I can think of is the ArrayAdapter from the cache component which would have the same problem if it wouldnt implement the ResettableInterface which inherits from ResetInterface.

How to reproduce

I had many issues setting up runtimes like swoole or FrankenPHP back when I tried it, so I kind of try to avoid it right now,
but I am pretty sure that the fine people contributing to this project can get the gist of the potential issue I described above.
Otherwise Im sorry that I couldnt express my thoughts more clearly.

Possible Solution

Now, we could register the IpUtils utility as a service in the FrameworkBundle and tag it with "kernel.reset", but I think it would be a huge overkill because all methods of IpUtils are currently static and therefor the extra cost of adding it to the container for literally every single Symfony application in the world is just not worth it in my opinion.

Instead I would propose the following two (very similar) options:

  1. Add something along the lines of IpUtils::clearCache and call it in Kernel::boot (before L113 or after L115)
    public function boot()
    {
    if (true === $this->booted) {
    if (!$this->requestStackSize && $this->resetServices) {
    if ($this->container->has('services_resetter')) {
    $this->container->get('services_resetter')->reset();
    }
    $this->resetServices = false;
  2. Add something along the lines of IpUtils::clearCache and call it in the ServicesResetter directly. This wouldnt be ideal because the service may be overwritten or not registered at all. Both is really rare (I think) but worth the consideration.

Everyone who uses a runtime such as swoole (notably not FrankenPHP) is basically forced to use a reverse proxy and Request::isFromTrustedProxy is therefor called on every request (even if they use REMOTE_ADDR as trusted proxy value).

Somehow not invalidating the cache on every request could be quite benefitial because of this.
We could implement something like a probabilistic cache invalidation or just clear the cache when a fixed size is reached (we know that only boolean values with string keys are cached).

Registering IpUtils as a service would definitly be the more "Symfony" way of handling this issue but I think the Kernel::boot hack is worth it and NOT totally unprecedented since Kernel::preBoot actually sets the trusted proxies / hosts.

Additional Context

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.