codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug: Empty Classname variable check #8127

Closed Slamoth closed 1 year ago

Slamoth commented 1 year ago

PHP Version

8.1

CodeIgniter4 Version

4.4.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

Postgres

What happened?

Nothing special default installation. When we close the source with sourceguardian empty classname variable produces error messages concerning empty class name because Config/Services.php file does not contain a class name in default installation.

PHP Fatal error:  Uncaught Error: Class "" not found in /srv/www/htdocs/vendor/codeigniter4/framework/system/Config/BaseService.php:385
Stack trace:
#0 /srv/www/htdocs/vendor/codeigniter4/framework/system/Config/BaseService.php(267): CodeIgniter\\Config\\BaseService::buildServicesCache()
#1 /srv/www/htdocs/vendor/codeigniter4/framework/system/Config/BaseService.php(252): CodeIgniter\\Config\\BaseService::serviceExists()
#2 /srv/www/htdocs/public/index.php(66): CodeIgniter\\Config\\BaseService::__callStatic()
#3 /srv/www/htdocs/public/index.php(19): sg_load()
#4 {main}\n  thrown in /srv/www/htdocs/vendor/codeigniter4/framework/system/Config/BaseService.php on line 385

Steps to Reproduce

default installation and close the source code with sourceguardian.

Expected Output

Outpu should be the web page but Internal Error occurs (500).

Anything else?

the error can be fixed in vendor/codeigniter4/framework/system/Config/BaseService.php by a small modification in function buildServicesCache as below

protected static function buildServicesCache(): void
    {
        if (! static::$discovered) {
            if ((new Modules())->shouldDiscover('services')) {
                $locator = static::locator();
                $files   = $locator->search('Config/Services');
                // Get instances of all service classes and cache them locally.
                foreach ($files as $file) {
                  $classname = $locator->getClassname($file);
                  // check classname length
                  if (strlen($classname)>0) {
                    if ($classname !== Services::class) {
                        self::$serviceNames[] = $classname;
                        static::$services[]   = new $classname();
                    }
                  }
                }
            }

            static::$discovered = true;
        }
    }
kenjis commented 1 year ago

Can you send a PR? and why is empty class called?

Slamoth commented 1 year ago

Actually when the code is not closed with sourceguardian there is no problem but when its obfuscated and closed error occurs. I dont think it will hurt to add a small check to the code. I will take care of PR

kenjis commented 1 year ago

What is the value of $file for the empty class? and if it is only "", the condition $classname !== '' is better.

Slamoth commented 1 year ago

Very sorry for late reply. The file is Services.php and left as original -> app/Config/Services.php. https://github.com/codeigniter4/CodeIgniter4/blob/develop/app/Config/Services.php

<?php

namespace Config;

use CodeIgniter\Config\BaseService;

/**
 * Services Configuration file.
 *
 * Services are simply other classes/libraries that the system uses
 * to do its job. This is used by CodeIgniter to allow the core of the
 * framework to be swapped out easily without affecting the usage within
 * the rest of your application.
 *
 * This file holds any application-specific services, or service overrides
 * that you might need. An example has been included with the general
 * method format you should use for your service methods. For more examples,
 * see the core Services file at system/Config/Services.php.
 */
class Services extends BaseService
{
    /*
     * public static function example($getShared = true)
     * {
     *     if ($getShared) {
     *         return static::getSharedInstance('example');
     *     }
     *
     *     return new \CodeIgniter\Example();
     * }
     */
}
kenjis commented 1 year ago

Thank you for the info. I wonder if sourceguardian behaves incorreclty with an empty class? I recommend you send a bug report to sourceguardian.

In any case, it is not a bug in CI4, but if you send a PR, we will consider. https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Slamoth commented 1 year ago

I will surely do that. I understand that has nothing to do with CI but empty classes should not initialize :) You can close this issue since it is not an issue with CI.

kenjis commented 1 year ago

@Slamoth See 4.5 branch. I think this issue has been resolved.