Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: slash_item() errors when asked for a configuration item which doesn't exist #5967

Closed
Tras2 opened this issue May 5, 2022 · 5 comments · Fixed by #6058
Closed

Bug: slash_item() errors when asked for a configuration item which doesn't exist #5967

Tras2 opened this issue May 5, 2022 · 5 comments · Fixed by #6058
Labels
bug Verified issues on the current code behavior or pull requests that will fix them good first issue Good for newcomers

Comments

@Tras2
Copy link

Tras2 commented May 5, 2022

PHP Version

8.1

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cli-server (PHP built-in webserver)

Database

No response

What happened?

slash_item in Sys\Common errors when the configuration item doesn’t exist, which is contrary to expected outcome as per the documention

Steps to Reproduce

<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function poc()
    {
        # returns the baseURL as expected
        echo slash_item('baseURL');

        # this generates an error - detailed below
        echo slash_item('item_does_not_exist');
    }
}

The error that is displayed is this:-

ErrorException
Undefined property: Config\App::$item_does_not_exist
SYSTEMPATH/Common.php at line 987

980      *
981      * @return string|null The configuration item or NULL if
982      *                     the item doesn't exist
983      */
984     function slash_item(string $item): ?string
985     {
986         $config     = config(App::class);
987         $configItem = $config->{$item};
988 
989         if (! isset($configItem) || empty(trim($configItem))) {
990             return $configItem;
991         }
992 
993         return rtrim($configItem, '/') . '/';
994     }

Expected Output

As per the documentation, the return should be null when a configuration doesn't exist

Anything else?

I've never done a pull request before, but can attempt to do my first one if required

However I have identified what I believe to be the cause of this and here is the updated slash_item code for Common.php

if (! function_exists('slash_item')) {
    // Unlike CI3, this function is placed here because
    // it's not a config, or part of a config.
    /**
     * Fetch a config file item with slash appended (if not empty)
     *
     * @param string $item Config item name
     *
     * @return string|null The configuration item or NULL if
     *                     the item doesn't exist
     */
    function slash_item(string $item): ?string
    {
        $config     = config(App::class);
        
        if (property_exists($config, $item))
        {
            $configItem = $config->{$item};
        }
        
        if (! isset($configItem) || empty(trim($configItem))) {
            return null;
        }

        return rtrim($configItem, '/') . '/';
    }
}
@Tras2 Tras2 added the bug Verified issues on the current code behavior or pull requests that will fix them label May 5, 2022
@Tras2 Tras2 changed the title Bug: Bug: slash_item errors when asked for a configuration item which doesn't exist May 5, 2022
@Tras2 Tras2 changed the title Bug: slash_item errors when asked for a configuration item which doesn't exist Bug: slash_item() errors when asked for a configuration item which doesn't exist May 5, 2022
@kenjis
Copy link
Member

kenjis commented May 5, 2022

Thank you for taking time.

You are correct. This is a bug, but for what do you use this function?
I don't see use cases.

@kenjis
Copy link
Member

kenjis commented May 5, 2022

This function is used in HTML helper, so we can't remove.

@kenjis kenjis added the good first issue Good for newcomers label May 5, 2022
@Tras2
Copy link
Author

Tras2 commented May 6, 2022

I was looking for the quickest/simplest way to display a config item value in a view, and was evaluating this function when I came across the issue.

My use-case has since changed and I no longer use this function, but still thought it a simple bug worth raising

@aletoropov
Copy link

I'll take it

@MGatner
Copy link
Member

MGatner commented May 8, 2022

What a strange function. I assume this is some CI3 code that rolled in and hasn't had a pass in a long time. It's also somewhat erroneous, because the empty() check will cause 0 or false values (like $negotiateLocale) to return null. I recommend that we deprecate this function for removal in v5.

@aletoropov Thanks for stepping in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them good first issue Good for newcomers
Projects
None yet
4 participants