[BUG]: Cookies::get cause an error if it not properly instanced

[gtux]

Description
I need to work with a Cookies class in order to use their OOP approach. But, my code caused an error, after debugging I'd notice the method get could be more robust

To Reproduce
Instance a Cookies service and then use the get method.

Steps to reproduce the behavior:

class Test 
{
   // ... another stuff
   public function getValueFromCookie(): string
   {
      $cookies = new Cookies(false);
      if ($cookies->has("CookieName")) {
         $cookie = $cookies->get("CookieName"); // this cause an fatal error
         // ... logic with $cookie
      }
   }
}

If you look at the Cookies class:

class Cookies extends AbstractInjectionAware implements CookiesInterface
{

    /**
     * Gets a cookie from the bag
     */
    public function get(string! name) -> <CookieInterface>
    {
        var container, encryption, cookie;

        /**
         * Gets cookie from the cookies service. They will be sent with response.
         */
        if fetch cookie, this->cookies[name] {
            return cookie;
        }

        /**
         * Create the cookie if the it does not exist.
         * It's value come from $_COOKIE with request, so it shouldn't be saved
         * to _cookies property, otherwise it will always be resent after get.
         */

        // ======================
        // ==> Here it's the line that cause the error, because this->container is null, so called the
        // method get produces a fatal error
        let cookie = <CookieInterface> this->container->get("Phalcon\\Http\\Cookie", [name]),
            container = this->container;

        // ==> Here it's ok, check if container it's an object, but it must be evaluated before
        if typeof container == "object" {
            /**
             * Pass the DI to created cookies
             */
            cookie->setDi(container);

            let encryption = this->useEncryption;

            /**
             * Enable encryption in the cookie
             */
            if encryption {
                cookie->useEncryption(encryption);
                cookie->setSignKey(this->signKey);
            }
        }

        return cookie;
    }
}

In order to avoid the error the instance must be setup with container before the call.

class Test 
{
   // ... another stuff
   public function getValueFromCookie(): string
   {
      $cookies = new Cookies(false);
      $cookies->setDI(Di::getDefault()); // this avoid the fatal error
      if ($cookies->has("CookieName")) {
         $cookie = $cookies->get("CookieName"); 
         // ... logic with $cookie
      }
   }
}

Expected behavior
Maybe the code has the right behavior, but the container must be evaluated before the called to create a new Cookie or get the cookie and trigger a more informative error/exception.

Details

• Phalcon version: (5.6 but the current version has the same code)

[raicabogdan]

While I do see your point, I'm unsure if this should be something that we should fix or needs a fix. The Cookies class is indeed meant to be used more as a service provider from what I understand. Meaning that the container needs to exist before you can make use of it.

As such the way you're making use of it is actually the correct approach. That or extending Cookies and set the container in the constructor or initialize() function.

Though I would have liked a more descriptive error message as well instead of

Uncaught RuntimeException: Trying to call method get on a non-object in test.php

[gtux]

Hi @raicabogdan

Yeah! You are right, this is not really a bug really indeed, because the reasons you already mentioned. What I mean, is could be a better approach capturing the errors and be more verbose about that. It could be stay like that? Yeah, it does not matters, as you mentioned: this is a service and must be used as it.