Jump to content



Photo
- - - - -

PHP7 Dynamic Class/Array Handling Gotcha


  • Please log in to reply
19 replies to this topic

#1   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 09:45

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!


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#2   raiwa

raiwa
  • Community Sponsor
  • 1,804 posts
  • Real Name:Rainer Schmied
  • Gender:Male
  • Location:Sant Iscle de Vallalta, Barcelona, Spain

Posted 10 April 2016 - 10:50

@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, 10 April 2016 - 10:51.


#3   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 11:39

@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(.......

BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#4   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 11:41

@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!


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#5   raiwa

raiwa
  • Community Sponsor
  • 1,804 posts
  • Real Name:Rainer Schmied
  • Gender:Male
  • Location:Sant Iscle de Vallalta, Barcelona, Spain

Posted 10 April 2016 - 12:53

@BrockleyJohn,

 

Yes empty class constructer did it for now for me.

 

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

http://forums.oscomm...hp-7/?p=1739026

 

Thank you very much.

 

rgds

Rainer



#6   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 13:35

@BrockleyJohn,

 

Yes empty class constructer did it for now for me.

 

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

http://forums.oscomm...hp-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.


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#7   raiwa

raiwa
  • Community Sponsor
  • 1,804 posts
  • Real Name:Rainer Schmied
  • Gender:Male
  • Location:Sant Iscle de Vallalta, Barcelona, Spain

Posted 10 April 2016 - 13:39

@BrockleyJohn,

 

Seems you are right, took off the line and it still works flawless. :thumbsup:



#8   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 15:28

Thanks @raiwa that saves me writing a set of test classes! :thumbsup:


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#9   piernas

piernas
  • Members
  • 466 posts
  • Real Name:Juanma
  • Gender:Male
  • Location:Madrid

Posted 10 April 2016 - 18:21

@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.



#10   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 10 April 2016 - 18:28

@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.


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#11   piernas

piernas
  • Members
  • 466 posts
  • Real Name:Juanma
  • Gender:Male
  • Location:Madrid

Posted 10 April 2016 - 19:51

@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



#12   Gyakutsuki

Gyakutsuki
  • Members
  • 599 posts
  • Real Name:Loic Richard
  • Gender:Male
  • Location:Montreal

Posted 10 April 2016 - 21:40

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, 10 April 2016 - 21:40.

Regards

 

-----------------------------------------

Loïc

Contact me by skype for business

Contact me @gyakutsuki for an answer on the forum


#13   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 11 April 2016 - 07:55

More info on the original problem in this thread from http://php.net/manua...ncompatible.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.


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#14   burt

burt

    I drink and I know things

  • Community Team
  • 12,492 posts
  • Real Name:G Burton
  • Gender:Male
  • Location:UK/DEV/on

Posted 11 April 2016 - 08:28

@BrockleyJohn - just wanted to say thank you for working on this.  Whole community will benefit from your hard work.


This is a signature that appears on all my posts.  It is not specifically aimed at you.

 

IF YOU MAKE A POST REQUESTING HELP...please state the exact version of osCommerce that you are using. THANKS
 
If you are still on the old style osCommerce, it is time to move to Responsive.

 


#15   douglaswalker

douglaswalker
  • Members
  • 267 posts
  • Real Name:Doug
  • Gender:Male
  • Location:Western Australia

Posted 11 April 2016 - 14:57

Big thanks here too



#16   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 11 April 2016 - 19:23

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/g...erce/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!


BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#17   annuity

annuity
  • Members
  • 35 posts

Posted 30 December 2016 - 04:14

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, 30 December 2016 - 04:27.


#18   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted 30 December 2016 - 07:06

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);
      }
    }
  }

BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x


#19   bruyndoncx

bruyndoncx

    osCommerce Teenager

  • Members
  • 3,786 posts
  • Real Name:Carine Bruyndoncx
  • Gender:Female
  • Location:Belgium/ Antwerp/ Turnhout/ Arendonk

Posted Yesterday, 12:30

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 ?


KEEP CALM AND CARRY ON
FYI Upgrade to the highest PHP version you can( PHP 5.5/5.6 or 7.1  and get big performance improvements for free)

But be aware php 5.5 is more strict about things.
UTF8-without BOM, no extra spaces allowed at the beginning or end of your php file, or your redirects wont work.
No double declarations of functions allowed - used to slip through the cracks ...

 

Find the most frequent unique errors to fix:

grep "PHP" php_error_log.txt | sed "s/^.* PHP/PHP/g" |grep "line" |sort | uniq -c | sort -r > counterrors.txt


#20   BrockleyJohn

BrockleyJohn
  • Community Sponsor
  • 495 posts
  • Real Name:John Ferguson
  • Gender:Male
  • Location:sarf Landin

Posted Today, 07:35


Question

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

 

 

Install tests for

PHP_VERSION >= 5.3

BS Edge for PHP7 - here on github: https://github.com/B...i/PHP-7-changes

Bootstrap addons - one per branch on github: https://github.com/B...iew-of-Branches

 

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later integration at 2.3.x