Non-core contributed modules

Review PHP 5.3.0 deprecated functions usage in MRBS block and update accordingly

Details

  • Type: Task Task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 2.0
  • Component/s: Block: Mrbs
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

Mike,

I received an email from a user about some notices that they were getting which pertains to their using PHP 5.3.0 which deprecated a number of functions (http://php.net/manual/en/migration53.deprecated.php). I have created a quick patch but do not have time at the moment to test it. I would like to see if we could go through and ensure that the MRBS block is good to go for PHP 5.3.0 so we should check and see how any of the deprecated functions were being used.

Peace - Anthony

Issue Links

This issue will be resolved by:
MDL-19415 PHP 5.3 compatibility fixes META Minor Closed
This issue has a non-specific relationship to:
MDL-20821 Evaluate best way to handle PHP deprecation of ereg_replace and eregi_replace function in PHP 5.3.0 Blocker Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Anthony Borrow added a comment -

Mike - The patch only looks for ereg_replace and split. Note, that I think the javascript use of split is still OK but I have not checked the other functions. Peace - Anthony

Show
Anthony Borrow added a comment - Mike - The patch only looks for ereg_replace and split. Note, that I think the javascript use of split is still OK but I have not checked the other functions. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Mike - I am going to assign this to you but if you do not have time feel free to assign it back. Peace - Anthony

Show
Anthony Borrow added a comment - Mike - I am going to assign this to you but if you do not have time feel free to assign it back. Peace - Anthony
Hide
Permalink
Ian Wild added a comment -

Hi Anthony and Mike,

I've just been looking at including this module for one of my clients and I'm wondering if you can simply use preg_replace without modifying the $pattern parameter? The pattern ereg_replace() requires is not the same as preg_replace(). In my code I've replaced

$cookie_path = ereg_replace('[^/]*$', '', $cookie_path);

with

$cookie_path = preg_replace('#[^/]/$#', '', $cookie_path);

...which I think does the same thing. But I'm no PHP expert so I'd be more than happy to be corrected.

Many thanks in advance,

Ian.

Show
Ian Wild added a comment - Hi Anthony and Mike, I've just been looking at including this module for one of my clients and I'm wondering if you can simply use preg_replace without modifying the $pattern parameter? The pattern ereg_replace() requires is not the same as preg_replace(). In my code I've replaced $cookie_path = ereg_replace('[^/]*$', '', $cookie_path); with $cookie_path = preg_replace('#[^/]/$#', '', $cookie_path); ...which I think does the same thing. But I'm no PHP expert so I'd be more than happy to be corrected. Many thanks in advance, Ian.
Hide
Permalink
Anthony Borrow added a comment -

Ian - At this point, in good conscience, I cannot recommend the MRBS block in its current state for any production server. I would have read up on the particulars of preg_replace. Unfortunately I do not do PHP coding full time so I am always having to remind myself of things. I vaguely recall a discussion about preg_replace. I know in core, you can look at how it was handled at MDL-20821 which showed a similar approach to what you are suggesting (replacing ereg_replace with preg_replace) but there was a fair amount of discussion which you may find interesting about cases that were problematic. Peace - Anthony

Show
Anthony Borrow added a comment - Ian - At this point, in good conscience, I cannot recommend the MRBS block in its current state for any production server. I would have read up on the particulars of preg_replace. Unfortunately I do not do PHP coding full time so I am always having to remind myself of things. I vaguely recall a discussion about preg_replace. I know in core, you can look at how it was handled at MDL-20821 which showed a similar approach to what you are suggesting (replacing ereg_replace with preg_replace) but there was a fair amount of discussion which you may find interesting about cases that were problematic. Peace - Anthony
Hide
Permalink
Ian Wild added a comment -

Hi Anthony - many thanks for your prompt response. I've just noticed that my fragment of PHP code was wrong anyway It should be something like...

$cookie_path = preg_replace('/[^\/]*$/', '', $cookie_path);

Is your concern that the code hasn't been modified to work with PHP 5.3 or is there more to it than that (e.g. security problems, that the MRBS code included with the block is out of date) ?

Thanks in advance,

Ian.

Show
Ian Wild added a comment - Hi Anthony - many thanks for your prompt response. I've just noticed that my fragment of PHP code was wrong anyway It should be something like... $cookie_path = preg_replace('/[^\/]*$/', '', $cookie_path); Is your concern that the code hasn't been modified to work with PHP 5.3 or is there more to it than that (e.g. security problems, that the MRBS code included with the block is out of date) ? Thanks in advance, Ian.
Hide
Permalink
Anthony Borrow added a comment -

I believe this was fixed in the Moodle 2.0 version. If not, please create another bug and I'll take a look at any notices. Peace - Anthony

Show
Anthony Borrow added a comment - I believe this was fixed in the Moodle 2.0 version. If not, please create another bug and I'll take a look at any notices. Peace - Anthony

People

Vote (1)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.