$app->persistent (Session bag) fatal error on 3.2.0 (Invalid superglobal)

[stamster]

Expected and Actual Behavior

Trying to use persistent global variable as usual in Phalcon Micro

Now this is truly "the bug". Everything working fine on 3.1.2. From 3.2.0 entire app breaks with fatal error.

2017/06/21 12:36:13 [error] 399#399: *209 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Uncaught Exception: Invalid superglobal in /var/www/html/api/public/tryout.php:22
Stack trace:
#0 [internal function]: Phalcon\Session\Adapter->get('Phalcon\\Mvc\\Mic...')
#1 [internal function]: Phalcon\Session\Bag->initialize()
#2 [internal function]: Phalcon\Session\Bag->set('auditID', 1)
#3 /var/www/html/api/public/tryout.php(22): Phalcon\Session\Bag->__set('auditID', 1)
#4 {main}
  thrown in /var/www/html/api/public/tryout.php on line 22" while reading response header from upstream, client: 178.xxx.xxx.xx, server: mytld.com, request: "GET /tryout.php HTTP/1.1", upstream: "fastcgi://unix:/run/php/php7.0-fpm.sock:", host: "mytld.com:9005"

Provide minimal script to reproduce the issue

$app = new \Phalcon\Mvc\Micro();
$app->get(
    "/say/hello/{name}",
    function ($name) {
        echo "<h1>Hello! $name</h1>";
    }
);
$app->persistent->auditID = 1;

Details

• Phalcon version: (3.2.0)
• PHP Version: (PHP 7.0.18)
• Operating System: Debian GNU/Linux
• Installation type: Compiling from source
• Zephir version (if any): (0.9.8-6335775f25)
• Server: Nginx
• Other related info (Database, table schema): n/a

Everything working fine on 3.1.2. From 3.2.0 entire app breaks with fatal error.

[Jurigag]

Well as a workaround for issue:

$di->set('session', function(){
    $session = new \Phalcon\Session\Adapter\Files();
    $session->start(); // we need to start session
    return $session;
});

$app = new \Phalcon\Mvc\Micro($di);

 // rest of code

Not sure what happens exactly and why on previous version it was working correctly. It looks like start was called or not?

[stamster]

Here we need hot fix rather than workaround as people cannot change their code base that easily.

If the simplest scenario (steps to reproduce) does not work as expected, i.e. throwing a fatal error, that's bad for a release declared as stable (3.2.0) and must be fixed ASAP.

[Jurigag]

But i don't know what workaround? For me it's working as it should be. In PHP you can't set $_SESSION values until you do session_start(); The same is with setting them - if you don't set them then it won't work. Not sure what should be done any why it was working previously(was it?)

Tbh even on php 7 and phalcon 3/3.1, your code won't work as expected - yea it would save to $_SESSION. But i won't be accessed in other request - beacause no session_start. To be honest i like this fatal error - it just tells you for sure that session wasn't started.

[stamster]

You provided a workaround - didn't you? #12918 (comment)

I really don't see the point what you're trying to explain with PHP sessions etc.
You realize that this was working 'till 3.2.0? That was from 2.0.0 Phalcon as I remember.

Yeah I like fatal error too - better fatal error than SEGFAULT. But the question is why this is broken and how to hot fix it.

[Jurigag]

For me it's not broken, it works as it should be imho beacause you didn't start session so $_SESSION is unknown at this point.

[Jurigag]

It wasn't really working - you could set value like that to $_SESSION but since you didn't started it anywhere its doing pretty much nothing in other request you can't access it.

Just in 3.1.2 it wasn't working too as expected - you just didn't have any error at all. You would know that it wasn't working as expected when accessing this value from bag in other request.

[sergeyklay]

@stamster Actually we can check session status on __set and __get. Could you please submit a PR with hotfix?

[Jurigag]

What i mean - it's stupid to fix it to NOT throw fatal error or anything - because this code doesn't really work as it should be. Yea it will set $_SESSION value, but it won't do what you expect to do without session_start.

Imho adding checks if sessions was started will introduce overhead - so it should stay as it is. Only i would consider one thing - throw this error on all versions or don't throw it.

[stamster]

@sergeyklay If I'm able to find out root cause and not to trigger BC issues, for sure I'll do a PR. I believe this issue deserves label also.

@Jurigag You simply try to ignore simple fact that even back with Phalcon v2.0.x this code was working w/o any issues, all until now with 3.2.0. And precisely - the bag is being populated with the data once it is set in the simplest example provided here, and it still does work on 3.1.2 on live API.
Remember, persistent is internal stuff, shared between controllers / handlers which extend or implement certain Phalcon interfaces / classes.

[Jurigag]

@stamster but it WASN'T working without any issues - it wasn't working as exepected, like after refresh there wasn't really value set in $_SESSION. You need to start session to set any $_SESSION value - end of story for me. If zephir somehow started to detecting it - then great.

I checked it on both phalcon 2 and 3 - without session_start you don't have really value set in $_SESSION obviously, so if there is exception right now - im fine with it.

Yes it's internal stuff, but it's based on SESSION, if you didn't started session, then setting any data to persistent is pretty much out of point. You can use registry for this if you need some place to set something during request time but you don't need it in other request or just use some cache like memcache etc.

[Jurigag]

It's stupid to argue that your code is not working in phalcon 3.2 - because it's not working in 3.1 and 3 and 2 too really as expected(save value in $_SESSION and access it in other request) - because you simply don't have session started, this exception right now at least tells you that it doesn't work actually.

[Jurigag]

Also to add - why this is 100% correct exception for me and it's actually feature - not a bug. If you do var_dump($_SESSION) in php without session_start you will have Notice: Undefined variable: _SESSION. Then pretty obvious zephir don't recognize _SESSION and it's invalid superglobal indeed because it's just undefined and not initialized by session_start

Imho the best will be in Phalcon 4 to remove session from FactoryDefault and don't check in any method if session was started - just developer have to start it when defining service, adding overhead by checking if it was started and if not - start it is kind of bad.

[sergeyklay]

Close in favor of #12921

[Jurigag]

Well @sjinks could at least answer if this superglobal detection is something which was added or it's just some mistaken code?

https://github.com/phalcon/zephir/pull/1532/files#diff-91d00b5ec72fef64318f760cae59fbdaR116 im guessing that this is mainly thing making a difference, that we return failure here and previously there were success and that's why your code was "working"?

[sergeyklay]

Refs: #10530

[stamster]

For now, I've added this code snippet on a central location:

!$app->session->isStarted() && $app->session->start();

And now it does work as before 3.2.0.

Even better solution. Kudos to @Jurigag for giving a hint for Registry class.

use \Phalcon\Registry as RegistryBag;

// Overriding default functionality by simpler one, w/o need to re-factor code base
$di->setShared('persistent', function(){
    return new RegistryBag();
});

[Jurigag]

You needed this to work persistent as it should anyway like start session(to have session variables available in other request). If you don't then Registry is exactly for you imho. This is why i was posting that you need to start session anyway and this is not really a bug, there was just change in zephir code to return failure on not recognized superglobal.

[stamster]

You're just wrong big time on this one - that it wasn't working at all, even with previous Zephir / Phalcon. It was, and it's so easy to prove it, just install 3.1.2 and see for yourself on simplest example provided here - steps to reproduce. IDK who started session internally - it wasn't me for sure, was it Zephir or Phalcon internally - it doesn't really matter, but with persistent variables I stored all of the data in DB - so one more proof it worked like a charm.
Anyway, Registry class seems much more KISS and with Phalcon it's so easy to override stuff w/o refactoring like I've shown in the above solution, so I'll stick to that.

[Jurigag]

I installed and it wasn't working - when accessing this variable in other request it wasn't there because session wasn't started. There were bug in zephir which was just allowing to use it like this. Now it's corrected.

For example in php if you don't do session_start() then $_SESSION is undefined variable. If you will set anything to it - it will be just normal variable like $anything. If you will try to access it in other request it won't be there.

[Jurigag]

Just do in one request:

$di = new \Phalcon\Di\FactoryDefault();
$app = new \Phalcon\Mvc\Micro();
$app->get(
    "/say/hello/{name}",
    function ($name) {
        echo "<h1>Hello! $name</h1>";
    }
);
$app->persistent->auditID = 1;

And in other:

$di = new \Phalcon\Di\FactoryDefault();
$app = new \Phalcon\Mvc\Micro();
$app->get(
    "/say/hello/{name}",
    function ($name) {
        echo "<h1>Hello! $name</h1>";
    }
);
var_dump($app->persistent->auditID);

You will have null returned. In latest zephir it was fixed to not allow like this.

[stamster]

You're obviously missing the point while desperately trying to prove your points are right - technically.
Now you went off topic - I never ever mentioned different requests, and anyway how is that suppose to work with stateless clients like remote API's connecting to our backend, they don't accept Cookies at all.

So one more time in case you have troubles to see it correctly:

1. Remote systems making requests to the API. Yeah, those are all HTTP(S) normal requests, JSON format.
2. persistent is used per single request, to store some internal data which are temporary data for exclusive use for internal application components
3. At the end of the request, data from persistent is used and saved in a DB.
4. Request ends, and of course at this point persistent does not exist alongside with entire application

That way system still works in a production, but instead you'll try to prove me wrong that I don't see stuff as I haven't actually started the Session. What has more value? Your tech. explanation or my 24/7 proven system?

Anyway, thanks for a hint for Registry which should prevent such issues in the future versions of Phalcon/Zephir/Whatever gets in my way.

[Jurigag]

But persistent is using $_SESSION. So it wasn't working how it should, you were just using wrong thing to your purpose.

There was only one issue - that zephir was allowing you to use persistent and save session variable without starting it, now it detects invalid superglobals(which $_SESSION is when trying to use it without session_start()) and throw exception.

[stamster]

Conclusion that something is used in a technically "wrong manner" does not disapprove the fact that it worked (and still does) as expected from app logic perspective.

Case closed.

[Jurigag]

Yea, and now it isn't, as it should be because session was not started and $_SESSION is wrong superglobal.