Latest News: (loading..)
BrockleyJohn

PHP7 Dynamic Class/Array Handling Gotcha

22 posts in this topic

There are lots of places where osc goes through a whole set of classes, instantiating and using them dynamically - most of the module stuff in fact. The vast majority of it is fine, but I just hit an oddity in admin/security_checks.php
 

    $secCheck = $$module['class'];

The above didn't work any more assigning the $secCheck object to a previously instantiated object. Subsequent use throws:

Warning: Creating default object from empty value in [...]/security_checks.php on line 82

Fatal error: Uncaught Error: Call to undefined method stdClass::pass() in [...]security_checks.php:87 Stack trace: #0 {main} thrown in [...]/security_checks.php on line 87

 

but

    $tmpclass = $module['class'];
    $secCheck = $$tmpclass;

works fine.

 

Maybe it's a php bug, maybe it's a deliberate change. Either way, it might save someone the load of time I just spent on it!

raiwa likes this

Share this post


Link to post
Share on other sites

@@BrockleyJohn,

 

Thanks a lot for this and your support.

 

I'm trying to get a working updated version for the public store side under PHP7 too. As this part is not yet published on GitHub, I would like to ask if you have a fix for the following:

message_stack.php class throws the following message on login attempt and similar on create account:

Fatal error: Uncaught Error: Call to undefined method messageStack::alertBlock() in C:\xampp_php_7_0\htdocs\2.3.4BS_Master_2016_04\includes\classes\message_stack.php:66 Stack trace: #0 C:\xampp_php_7_0\htdocs\2.3.4BS_Master_2016_04\login.php(84): messageStack->output('login') #1 {main} thrown in C:\xampp_php_7_0\htdocs\2.3.4BS_Master_2016_04\includes\classes\message_stack.php on line 66

Then I also can't get the box.php class updated.

 

Sorry, classes and class extensions still keep a lot of secrets to me :)

 

Any help and advice very appreciated.

 

Kindest regards

Rainer

Edited by raiwa

Share this post


Link to post
Share on other sites

@@raiwa my guess is you have renamed the constructor of alertBlock to __construct

 

The issue here is that old-style constructors may also be used as ordinary methods, as in this case on the line throwing the error

 

You should have done one of the following in the alertBlock class:

- if the class is ever instantiated directly eg

$whatever = new alertBlock();

then you need to create a new constructor that calls the original:

function __construct($params) {
  $this->alertBlock($params);
}

if it never gets instantiated, you can just stick in an empty constructor:

function __construct() {
}

function alertBlock(.......
raiwa and John W like this

Share this post


Link to post
Share on other sites

@@burt @@raiwa

 

Rainer please note Gary's looking for these changes in manageable chunks so don't create a pull request for the whole of the catalog side!

Share this post


Link to post
Share on other sites

@@BrockleyJohn,

 

Yes empty class constructer did it for now for me.

 

I also could fix the box.php class following these instructions:

http://forums.oscommerce.com/topic/408220-php-7/?p=1739026

 

Thank you very much.

 

rgds

Rainer

 

@@raiwa I think those instructions aren't totally correct... did you follow them to the letter and put a call to the parent constructor in the box class? ie

class box extends tableBox {

  function __construct($params) {
    parent::__construct();
    [... followed by the original code that was in the box() function ]

If you did, would you try taking the parent:: line out and see if it makes any difference? If I understand it properly, the line is not required and taking it out should make no change.

raiwa likes this

Share this post


Link to post
Share on other sites

@@BrockleyJohn @@raiwa I think it's more logical to do this with every class:

class hello {
  function hello($params){
  stuf....
  }
}

to this:

class hello {
  function _construct($params){
  stuf....
  }
  function hello($params){
    $this->__construct($params);
  }
}

I think this way everything should work in all situations.

Share this post


Link to post
Share on other sites

@@BrockleyJohn @@raiwa I think it's more logical to do this with every class:

class hello {
  function hello($params){
  stuf....
  }
}

to this:

class hello {
  function _construct($params){
  stuf....
  }
  function hello($params){
    $this->__construct($params);
  }
}

I think this way everything should work in all situations.

 

Sorry @@piernas that's really ugly and back-to-front.

 

Nor is it logical to do the same change to all modules. You still have to test all the code that has been changed, even if it doesn't get executed. This is much more work than doing it properly.

Share this post


Link to post
Share on other sites

@@BrockleyJohn of course it's not nice but I don't see any situation where code cold break with those changes, other than a typo (like in my example) :D

Share this post


Link to post
Share on other sites

It's  not better to have this

class hello {

  protected (or public) $params; (var in osc)

  public function _construct($params){
  stuf....
  }

  public function hello($params){
    $this->__construct($params);
  }
}
Edited by Gyakutsuki

Share this post


Link to post
Share on other sites

More info on the original problem in this thread from http://php.net/manual/en/migration70.incompatible.php
 

PHP 7 now uses an abstract syntax tree when parsing source files. This has permitted many improvements to the language which were previously impossible due to limitations in the parser used in earlier versions of PHP, but has resulted in the removal of a few special cases for consistency reasons, which has resulted in backward compatibility breaks. These cases are detailed in this section.


The evaluation order in PHP7 is left-to-right instead of right-to-left so you have to make the order explicit in the statement:

$secCheck = ${$module['class']};

This makes sure PHP5 and PHP7 use the same order.

John W, burt and annuity like this

Share this post


Link to post
Share on other sites

Thanks, guys. Someone's got to do the boring stuff.
 
@@raiwa if you're working on changes that you're going to feed back to the core, please put a post here https://github.com/gburton/Responsive-osCommerce/issues/288
to say which unit of work you're doing first.

 

I'm going to be picking up some of the catalog side next and we want to avoid working on the same thing!

Share this post


Link to post
Share on other sites

Hello my fellow Oscommercers. Hope you had a good Christmas and are looking forward to the New Year. I'm a shop owner and to cut a long story short i've been forced to update PHP on my server, which you've guessed it, breaks my site. So rather than update the old girl yet again i'm putting her out to pasture and taking the plunge starting a new one. I've uploaded 2.3.4 edge onto my server and am in the process of working through all the php 7 warnings and errors. The shop side is error free for now and i'm working my way through the admin side only i have one issue that i'm unable to resolve and was hoping the community could help if possible please.

 

If i update the class constructor in object_info.php in the normal way it fixes the class warning put then throws up a fatal error Call to to undefined method objectInfo::objectInfo() in banner manager. The offending line in banner manager is:

$bInfo->objectInfo($banner);

I'm unsure whether the problem lies in my update of the class constructor or the line in banner manager. For reference this is how i updated object info:

 class objectInfo {

// class constructor
    function __construct($object_array) {
      reset($object_array);
      while (list($key, $value) = each($object_array)) {
        $this->$key = tep_db_prepare_input($value);
      }
    }
  }

Please could someone help me fix this issue so i can move forward. Thanks in advance.

Edited by annuity

Share this post


Link to post
Share on other sites

Hello my fellow Oscommercers. Hope you had a good Christmas and are looking forward to the New Year. I'm a shop owner and to cut a long story short i've been forced to update PHP on my server, which you've guessed it, breaks my site. So rather than update the old girl yet again i'm putting her out to pasture and taking the plunge starting a new one. I've uploaded 2.3.4 edge onto my server and am in the process of working through all the php 7 warnings and errors. The shop side is error free for now and i'm working my way through the admin side only i have one issue that i'm unable to resolve and was hoping the community could help if possible please.

 

If i update the class constructor in object_info.php in the normal way it fixes the class warning put then throws up a fatal error Call to to undefined method objectInfo::objectInfo() in banner manager. The offending line in banner manager is:

$bInfo->objectInfo($banner);
I'm unsure whether the problem lies in my update of the class constructor or the line in banner manager. For reference this is how i updated object info:

 class objectInfo {

// class constructor
    function __construct($object_array) {
      reset($object_array);
      while (list($key, $value) = each($object_array)) {
        $this->$key = tep_db_prepare_input($value);
      }
    }
  }
Please could someone help me fix this issue so i can move forward. Thanks in advance.

 

 

If you are using Edge, you don't need to work through all the changes individually, follow the link in my signature below to get the full set of changes on in a branch on github, which is currently a little behind the latest Edge but can be merged with no conflicts.

 

Alternatively, you can download it all and use it as a reference for applying the changes yourself but be warned that there are a lot!

 

As of today the place that the link will lead you to download it is here but this will change next time I bring it up to date.

 

And the solution to your specific problem is that you need to keep a method with the class name and call it in the constructor like this:

  class objectInfo {
// class constructor
    function __construct($object_array) {
		  $this->objectInfo($object_array);
    }
    function objectInfo($object_array) {
      reset($object_array);
      while (list($key, $value) = each($object_array)) {
        $this->$key = tep_db_prepare_input($value);
      }
    }
  }
annuity and bruyndoncx like this

Share this post


Link to post
Share on other sites

as a little FYI for @@BrockleyJohn and @@burt

 

since I asked/posted about PHP7

1) I have read more about PHP7 changes in detail (especially the variable variables and execution order)

2) I decided it is all pretty low risk changes, I have not moved further to install on a raspberrypi, but I'm gonna be brave . Worst case, I just switch back to php 5.6, in 2 mins

3) I have checked out Johns PHP7 comp pack 02 branch onto my server and used BCompare to apply the changes in the code on the  CATALOG side (all the relevant changes in the subdirectories, not the catalog pages themself as I have not integrated with Bootstrap)

4) I'm monitoring all errors on my live system to see if anything odd pops up (like multisafepay code seems to work, but is not fully compatible yet - depracated errors)

 

So far so good.

 

5) Started on the admin code, but again, can't find anything not working at this time on the admin side, just notices and deprecated warnings I need to cleanup in my custom code/modules

6) if you use APCu caching, apc_xxxx functions are no longer supported, need to change to apcu_xxx

 

Question

What is the minimum required PHP version for responsive oscommerce, 5.x ?

Can we remove the specific php 4.x code pieces ?

Share this post


Link to post
Share on other sites

Question

What is the minimum required PHP version for responsive oscommerce, 5.x ?

 

 

Install tests for

PHP_VERSION >= 5.3

Share this post


Link to post
Share on other sites

What is the minimum required PHP version for responsive oscommerce, 5.x ?

Can we remove the specific php 4.x code pieces ?

 

Though the installer tests for 5.3, I think I have a couple of pieces of code that are 5.4 only.  

So the minimum would be 5.4

Yes get rid of any 4.x coding.

Share this post


Link to post
Share on other sites

Though the installer tests for 5.3, I think I have a couple of pieces of code that are 5.4 only.  

So the minimum would be 5.4

Yes get rid of any 4.x coding.

 

I just found a bit of code in 2.3.1 that needs php5.4 to work properly!

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now