Was about to follow #2274543-120: [7.x-1.x] Remove usage of deprecated create_function() ( thanks @josepholstad ) but seems simple update from 1.x to 2.x branch has fixed the issue for me.

CommentFileSizeAuthor
#193 views_php-remove_create_function-2274543-193-D7-interdiff.txt624 bytesBerdir
#193 views_php-remove_create_function-2274543-193-D7.patch9.97 KBBerdir
#180 interdiff-175-180.txt195 bytesLiam Morland
#180 views_php-remove_create_function-2274543-180-D7.patch9.97 KBLiam Morland
#176 interdiff-172-175.txt1.46 KBLiam Morland
#175 views_php-remove_create_function-2274543-175-D7.patch9.97 KBLiam Morland
#172 views_php-remove_create_function-2274543-172-D7.patch9.98 KBLiam Morland
#158 views-php-version-create-function-2274543-158.patch7.99 KBbisonbleu
#153 views-php-version-create-function.patch8.95 KBquak
#108 test_php.tgz29.09 KBjoseph.olstad
#100 interdiff_95-100.txt2.17 KBMustangGB
#100 views_php-2274543-100.patch14.47 KBMustangGB
#95 views_php-2274543-95.patch14.55 KBjoseph.olstad
#95 interdiff-2274543-81_to_95.txt873 bytesjoseph.olstad
#93 interdiff-2274543-81_to_93.txt729 bytesjoseph.olstad
#93 views_php-2274543-93.patch14.56 KBjoseph.olstad
#87 closure_library_patch-2274543-87.patch1.34 KBjoseph.olstad
#84 closure_library_patch-2274543-84.patch987 bytesjoseph.olstad
#81 interdiff-2274543-68-to-81.txt535 bytesjoseph.olstad
#81 views_php-2274543-81.patch14.49 KBjoseph.olstad
#68 views_php-2274543-68.patch14.46 KBspelcheck
#54 views_php-2274543-54.patch14.12 KBMustangGB
#53 views_php-2274543-53.patch14.1 KBMustangGB
#38 views_php-2274543-38.patch9.16 KBMustangGB
#35 views_php-2274543-35.patch9.19 KBMustangGB
#18 views_php-create_function-update-2274543-18.patch9.62 KBcirrus3d
#15 views_php-create_function-update-2274543-15.patch9.42 KBskylord
#13 views_php-create_function-update-2274543-13.patch4.01 KBalb404
#11 views_php-create_function-update-2274543-11.patch8.84 KBmonnerat
#8 views_php-create_function-update-2274543-8.patch9.5 KBAndreyMaximov
#4 views_php_no_create_function.patch8.12 KBbkat
#3 views_php-create_function-update-2274543-3.patch8.1 KBNWOM
views_php-create_function-update-1-1.patch7.96 KBjienckebd

Issue fork views_php-2274543

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jienckebd’s picture

Issue summary: View changes
bkosborne’s picture

Title: Anonymous Function Update and create_function() » Remove usage of deprecated create_function() calls for PHP 7.2+ future proofing
Issue tags: +PHP 7.0 (duplicate), +PHP 7.2

I've changed the title to reflect that this should be removed for PHP 7.2+ future proofing as well.

PHP's create_function() has been deprecated as of PHP 7.2.

The patch provided uses the new method of declaring an anonymous function, but that's only supported as of PHP 5.3, and Drupal 7 officially supports PHP 5.2.5+.

I wonder if the module can just declare real functions to use instead of anonymous functions. I think that's the only method that will work with PHP 5.2.5+.

If not, this module will have to declare that it requires PHP 5.3.

NWOM’s picture

FileSize
8.1 KB

Thank you very much for the patch!

However, it appears the above patch is based on 7.x-2.x-dev from the module page, but not from HEAD (https://www.drupal.org/node/1043008/git-instructions/7.x-2.x/nonmaintainer).

The following error is shown when attempting to apply the patch:

Checking patch plugins/views/views_php_handler_area.inc...
Checking patch plugins/views/views_php_handler_field.inc...
error: while searching for:
   */
  function pre_render(&$values) {
    if (!empty($this->options['php_output'])) {
      $this->php_output_lamda_function = create_function('$view, $handler, &$static, $row, $data, $value', ' ?>' . $this->options['php_output'] . '<?php ');
    }
  }


error: patch failed: plugins/views/views_php_handler_field.inc:204
error: plugins/views/views_php_handler_field.inc: patch does not apply
Checking patch plugins/views/views_php_handler_filter.inc...
error: while searching for:
  function php_post_execute() {
    // Evaluate the PHP code.
    if (!empty($this->options['php_filter'])) {
      $function = create_function('$view, $handler, &$static, $row, $data', $this->options['php_filter'] . ';');
      ob_start();
      foreach ($this->view->result as $i => $row) {
        $normalized_row = new stdClass;

error: patch failed: plugins/views/views_php_handler_filter.inc:79
error: plugins/views/views_php_handler_filter.inc: patch does not apply
Checking patch plugins/views/views_php_handler_sort.inc...
Checking patch plugins/views/views_php_plugin_cache.inc...
Checking patch views_php.module...

I have updated the patch, and also took liberty to make it more drupal conform (the patch had tabs and spaces, rather than double spaced).

Please review.

bkat’s picture

This patch saved me a ton of time on a Fedora 28 system with PHP 7.2. I did find one problem though. The value was always null because the code for evaluating 'php_value' was missing a return

in php_post_execute

      $code = $this->options['php_value'];
      $function = function($view, $handler, &$static, $row) use ($code) {
        eval($code);
      };

should be

      $code = $this->options['php_value'];
      $function = function($view, $handler, &$static, $row) use ($code) {
        return eval($code);
      };

I'm attaching an updated patch file.

akolahi’s picture

I am getting Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc) with Patch #4.
If I revert the patch the error goes away.

Digging a bit and it seems to only be an issue on Views that have both Views Bulk Operations and vews_php.

To replicate:

  1. Apply Patch
  2. Create View with a views php field and VBO
  3. Visit the View
NWOM’s picture

Status: Needs review » Needs work

Setting to Needs Work due to the problems mentioned in #5.

pipicom’s picture

#4 works for me. I do not have Views Bulk Operations installed.

AndreyMaximov’s picture

It is just like @akolahi described it.

The patch should help.

AndreyMaximov’s picture

Status: Needs work » Needs review
sistro’s picture

I still was having warnings on line 212 on views_handler_field#pre_render

I removed line:
$this->php_output_lamda_function = create_function('$view, $handler, &$static, $row, $data, $value', ' ?>' . $this->options['php_output'] . '<?php ');

And added this lines:
$code = $this->options['php_output'];
$this->php_output_lamda_function = function($view, $handler, &$static, $row, $data, $value) use ($code) {
return eval($code);
};

Seems to work.

monnerat’s picture

This patch for 7.x-2.x emulates create_function() without a use() clause, thus reducing the number of eval() calls to recompile the code.
It takes care of unsetting non-local Closure objects to avoid serialization errors.

joe_pixelrow’s picture

#11 patch eliminated all of the errors for me

Thank You monnerat

alb404’s picture

Hi.

I made a patch that combines #8 and #6 from this issue: https://www.drupal.org/project/views_php/issues/2300573

ab_early@hotmail.com’s picture

#11 patch eliminated all of the errors for me

Thanks monnerat

skylord’s picture

Here is #11 for 7.x-1.0-alpha3

lgough’s picture

Thanks so much skylord (and mannerot), #15 worked perfectly on 7.x-1.0-alpha3

Neo13’s picture

Status: Needs review » Reviewed & tested by the community

#15 works for me too

cirrus3d’s picture

Here is #15 for 7.x-1.x-dev

glynster’s picture

+1 latest patch works a treat!

TrevorBradley’s picture

+1 on #15 here as well!

lne_topos’s picture

+1 on #18. Thanks for providing for 7.x-1.x-dev.

PeterStubRaindrop’s picture

I'm a bit confused here...Using 7.x-1.x-dev - and php 5.6.32.

Used patch #18 and got this...:
Hunk #1 succeeded at 63 (offset 7 lines).
Hunk #2 succeeded at 84 (offset 7 lines).
patching file views_php.module
Hunk #2 succeeded at 158 with fuzz 1 (offset 10 lines).

I get tons of these:
Notice: Undefined variable: params i {closure}() (linje 6 af /Applications/.../modules/contrib/views_php/views_php.module(166) : eval()'d code).

I also did a test run with php 7.2 and 7.1 - not luck...

Cant get all those Notices out of here...Can somebody help out?

EDITED: Solved - it was our own mistakes. In the view that used php there was an undefined variable - as cirrus3d / #23 pointed out - THANKS ALOT.... So feel free to delete my comment as its not really relevant for this issue?

cirrus3d’s picture

Hi,
It seems to me that these notices are being generated from invalid code inside your View (for example if you have a PHP field or filter). Could you check Line 6 of your custom code?

PeterStubRaindrop’s picture

Thank you very much...@cirrus3d #23

youngwolf0’s picture

just wondering if this patch will ever be rolled into the main module since it was last updated on the 13th of November 2015. Our infrastructure is not set up in a way that makes patching easy so would be better for me if it was in a proper update.

Welsby’s picture

#23 works a charm, thanks.

And a +1 for this to be rolled into a release please.

cmseasy’s picture

#15 for 7.x-1.x-dev works ok.
Please commit

steveoriol’s picture

#15 for 7.x-1.0-alpha3 works.

rachelf’s picture

#15 for 7.x-1.0-alpha3 works for me too

fred1978’s picture

For me, using #15 for 7.x-1.0-alpha3 breaks the sorting of my view. I use PHP to calculate a new variable, and then sort the results according to this new variable and also existing variables.
The sort code looks like this
if ($row1->php < $row2->php) {
return -1;
}
else if ($row1->php > $row2->php) {
return 1;
}
...(sort on other fields results)...
else
return 0;
Could this be related to the "unset($this->php_sort_function); /* Closure objects are not serializable. */" in the ob_start part of the code ?

monnerat’s picture

... breaks the sorting of my view.

What do you mean by "breaks"? Is it wrongly sorted or does it crash ?
If you have a crash, then probably you have a syntax error in your sort code, or an exception occurring in it.

Could this be related to the "unset($this->php_sort_function); /* Closure objects are not serializable. */" in the ob_start part of the code ?

No: $this->php_sort_function is set just before this ob_start() sequence and only used by function php_sort() that is called by the usort() before the unset(). Unsetting $this->php_sort function only prevents it from being serialized later and should not impact the logic.

Note that your problem might be related to $row->php being a structure and you have to extract the comparison value from its subfields.

fred1978’s picture

Thank you monnerat, I will investigate based on your comment. No crash in my views result, but results are wrongly sorted.

fred1978’s picture

My mistake : there has been a problem while patching my file, the usort() line before the unset() has been removed. Thanks monnerat for pointing the function to me. Patch #15 for 7.x-1.0-alpha3 works well then.

salvis’s picture

Version: 7.x-2.x-dev » 7.x-1.x-dev

The RTBC refers to #15 and #18 for 7.x-1.x-dev — please commit #18 to 7.x-1.x-dev!

There are ten times as many users on 1.x than on 2.x and 1.x has at least an alpha release. The 2.x branch only has a snapshot and its state is uncertain, according to the front page.

MustangGB’s picture

#18 has some formatting and coding style issues, also the removal of php_output_lamda_function() seems like a API breaking change that's not needed.

So here's a version I'm testing against 7.x-1.x-dev to address these things.

MustangGB’s picture

Title: Remove usage of deprecated create_function() calls for PHP 7.2+ future proofing » [PHP 7.2] Remove usage of deprecated create_function()
Priority: Normal » Major

The future is now.

salvis’s picture

I agree about the formatting and coding style issues, but you've introduced a new glitch of your own:

diff --git a/plugins/views/views_php_handler_sort.inc b/plugins/views/views_php_handler_sort.inc
old mode 100644
new mode 100755

While #35 looks siimilar to #18, the changes aren't just cosmetic — this needs reviewing and testing from scratch.

MustangGB’s picture

FileSize
9.16 KB

Darn it, nice catch.

akzahid’s picture

I am getting Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /home/project/public_html/drupal_site/includes/cache.inc) with Patch views_php-2274543-38.patch

I did the same work as akolahi #5 to reprduce the issue

[ If I revert the patch the error goes away.

Digging a bit and it seems to only be an issue on Views that have both Views Bulk Operations and vews_php.

To replicate:

Apply Patch
Create View with a views php field and VBO
Visit the View

]

DamienMcKenna’s picture

@akzahid: If you're writing code that sophisticated you might want to consider moving it into a module instead of burying it in the Views interface.

akzahid’s picture

@DamienMcKenna Thanks, Yes that what I am working on and redesigning it in drupal 8 . But for the time being I am working to make sure the current used site work properly using pHP 7.2

nickonom’s picture

I was pulling the dev version with `drush dl views_php --dev` command and failing to get the patch applied several times, until I paid enough attention to see the patch was for 7.x.-1.x. But then the project page says:

7.x-1.x will only receive critical fixes once 7.x-1.0 has been released.
7.x-2.x will continue heavy development, including bug fixes, and may not be backwards compatible with 7.x-1.x.

and that's really confusing.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Yes this is back to RTBC thanks @MustangGB

salvis’s picture

... and that's really confusing.

Trust the facts rather than the text...

7.x-2.x is just not there yet and may never get "there".

7.x-1.x hasn't quite gotten to 7.x-1.0 yet either, but it's what people are using and what needs to be fixed.

nickonom’s picture

salvis, so every new user has to get to the facts only after sweating some time over this. imho, it's more usable to get the project description aligned with the "facts". i am not editing it only because i am not sure about the project developer's stance on this.

salvis’s picture

<offtopic>
I agree with you, nickonom — in a perfect world the maps always match the terrain.

In open source, however, with a lot of volunteer work, "customers" should look at the wares before downloading them.

I'm not picking on open source, quite the contrary, because in open source you can look at the stuff before you "buy", which is a huge advantage over commercial software, but open source is not free like free beer. Customers need to realize that they also have to "pay" for open source offerings, e.g. by taking on the responsibility for selecting what they download. It's just part of doing your homework.
</offtopic>

nickonom’s picture

I do confirm #4's finding:

Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc)
akzahid’s picture

I Still have this following issue. I use the latest patch #38

"Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc)"

Akzahid

akzahid’s picture

Status: Reviewed & tested by the community » Needs review
DamienMcKenna’s picture

Status: Needs review » Reviewed & tested by the community

@akzahid: Please see if you can track down the view and then custom PHP code which triggers the above error.

akzahid’s picture

Status: Reviewed & tested by the community » Needs review

I use the follwing php code in global php in view

if($data -> _field_data['nid']['entity'] -> field_job_type['und'][0]['target_id'] = 24) {
if(!isset($data -> field_field_document[0]['raw']['entity'] -> field_title)) {
    return;
} 

if(!isset($data->field_field_cr_document_title[0]['raw']['value'])) {

      echo($data -> field_field_documen[0]['raw']['entity'] -> field_title['und'][0]['value']);
}
}

and also using

Global: Send e-mail (Send e-mail)

and this view shows the following error: (used the latest patch #38)

"Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc)"

Thanks

gravisrs’s picture

Patch won't work due to caching.

eval() created functions are Anonymous functions that are implemented using the Closure class - and thus cannot be serialized for the purpose of caching.

So if you use a view within a block, or anywhere where Drupal caches the output - it will throw an Exception like in #48

We need different approach

Dirty hack to use for now is @create_function(...

MustangGB’s picture

FileSize
14.1 KB

In response to #52 maybe the approach to take, rather than trying to work around closures not being serializable, is to allow them to be serializable, and therefore keep cache functionality and such.

Here is a patch to try this strategy, it uses the Opis Closure library to do this.

Perhaps a hard dependency on Libraries API isn't wanted or needed, but it served as suitable for testing the idea at least.

To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php

Also for review purposes it's worth noting that although the library, which serves as a wrapper, says it allows direct access to the closure, it recommends going through getClosure(), and indeed this was needed in our case due to parameters being passed by reference, might be something that could be resolved upstream, but not the end of the world at any rate.

MustangGB’s picture

FileSize
14.12 KB

Fixed typo.

khaldoon_masud’s picture

The patches #15 #18 and #54 are still reporting below warning on my local machine:
Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (line 202 of /app/drupal/sites/all/modules/contrib_modifed/views_php/plugins/views/views_php_handler_field.inc).

I am using a linux alpine distribution on a docker container on MacOs. It doesn't make sense cause I can't find create_function anywhere in the code, BUT when I pushed same code to our dev server running Ubuntu OS, it started working without errors.

I applied patch #15 on 7.x-1.0-alpha3. Couldn't apply patch #18, there was a spacing issue.

stacypendell’s picture

#54 is working for me - "Exception: Serialization of 'Closure'" errors stopped appearing.

akzahid’s picture

#54 is working for me - "Exception: Serialization of 'Closure'" errors stopped appearing for me as well.

Note: After this my site is not working on PHP 5.6 - Works fine on PHP 7.2


resaons:
Parse error: syntax error, unexpected '(' in /root/---/---/drupal_folder/sites/all/modules/views_php/views_php.module on line 34

Thanks
akzahid

hiramanpatil’s picture

I am getting these output after applying patch #53 and #54 both.

Skipped patch 'plugins/views/views_php_handler_area.inc'.
Skipped patch 'plugins/views/views_php_handler_field.inc'.
Skipped patch 'plugins/views/views_php_handler_filter.inc'.
Skipped patch 'plugins/views/views_php_handler_sort.inc'.
Skipped patch 'plugins/views/views_php_plugin_cache.inc'.
Skipped patch 'views_php.info'.
Skipped patch 'views_php.module'.

MustangGB’s picture

@hiramanpatil
Probably you're trying to apply the patch from the wrong folder or to the wrong branch, either way your comment is unrelated to this issue.

alimc29’s picture

... apparently I can't delete my own comment - this was posted in error

spelcheck’s picture

@MustangGB - I'm currently on 2.x-dev (sadly) and there's no easy way for me to get back to 1.x-dev. I reworked your latest patch to work for 2.x-dev but unsure whether you'd want me to post in here, as it might be confusing to the thread. Let me know what you'd like to do. As you're a major contributor to this fix, I'll leave it to your judgement. Thank you!

MustangGB’s picture

@spelcheck
The done thing these days seems to be to create a new issue and add a relationship between the two, so I'd say to go with that.

MustangGB’s picture

Regarding PHP 5.6 backwards compatibility as per #57.

Probably we must make changes along these lines:

- $function->getClosure()($this->view, $this, $this->view->result);
+ $closure = $function->getClosure();
+ $closure($this->view, $this, $this->view->result);

And so on for the rest of the getClosure() references.

I tried to keep it as similar to the original code as possible, and to make it an easy switch to remove the explicit getClosure() calls (if/when the aforementioned "by reference" issue is addressed), however backwards compatibility is more important than looking pretty.

MustangGB’s picture

Status: Needs review » Needs work

Needs work, to make the changes noted in #63.

spelcheck’s picture

2.x-dev ports of contributed patches @ https://www.drupal.org/project/views_php/issues/3055655. I'll do my best to keep them updated with whatever is posted here.

monnerat’s picture

The done thing these days seems to be to create a new issue and add a relationship between the two, so I'd say to go with that.

This is an interesting comment in an issue that was originally created for 7.x-2.x and hijacked for 7.x-1.x, then simply reassigned to 7.x-1.x with all 7.x-2.x attachments hidden !!!

spelcheck’s picture

Concerning #5 and #39, I also run into issues when using VBO. Was there any workaround or recommendation?

On simply visiting the view with VBO:

Warning: file_get_contents(/var/www/d7site/web/sites/all/modules/contrib/views_php/views_php.module(197) : eval()'d code): failed to open stream: No such file or directory in Opis\Closure\ReflectionClosure->getFileTokens() (line 631 of /var/www/d7site/web/sites/all/libraries/closure/src/ReflectionClosure.php).

It's attempting to file_get_contents() on "....views_php/views_php.module(197) : eval()'d code" instead of the filename by itself.

On attempt to actually use VBO within the view:

Notice: unserialize(): Error at offset 1711218 of 3039193 bytes in DrupalDatabaseCache->prepareItem() (line 449 of /var/www/d7site/web/includes/cache.inc).

Error: Unsupported operand types in form_get_cache() (line 525 of /var/www/d7site/web/includes/form.inc).

spelcheck’s picture

FileSize
14.46 KB

@MustangGB - Updated your patch views_php-2274543-54.patch according to instructions on #63. Forgive me for hijacking the patch, I'm hoping to keep momentum so that there might be some time to attack VBO issues.

MustangGB’s picture

No problem, I didn't have time yet, so was hoping someone would roll in the changes.

tt12’s picture

After applying patch #68 and visiting the view with views_php, i encounter an error:

Error: Class 'Opis\Closure\SerializableClosure' not found in function views_php_create_function() (line 188 in file (...)/views_php/views_php.module).

MustangGB’s picture

@tt12
You need to install the library, as mentioned in #53.

newtoid’s picture

Confirming that the patch also works for the latest alpha version of views_php.

James Feng’s picture

views_php-2274543-38.patch ,It suits me very well. my version:php7.2; Views PHP
7.x-1.0-alpha3; Thanks.

patoshi’s picture

So i need to install the Libraries api module? And then what before doing the #68 patch?

Or do I just use patch #38?

i tried patching #38 with v1.x dev views_php but i'm getting error of:

can't find file to patch at input line 5

Perhaps you should have used the -p or --strip option?
The text leading up to this was:

-------------------------- |diff --git a/plugins/views/views_php_handler_area.inc b/plugins/views/views_php_handler_area.inc |index 0296373..ed14bcf 100644 |--- a/plugins/views/views_php_handler_area.inc |+++ b/plugins/views/views_php_handler_area.inc --------------------------
File to patch: ^C

liquidcms’s picture

I'm surprised that this patch is for 1.x branch and that people are still using this. The 1.x branch never really worked as advertised and the 2.x branch is a major improvement (as it does what the module suggests). Ahh.. but i see a link to 2.x patch for this.. yay!! :)

patoshi’s picture

@liquidcms so your saying 1.x doesn't work at all with the patches above?

FYI, I've applied patch #38 to the 1.x dev code base as I needed to patch it manually as the patch file wasnt working.

Here is the file: https://transfer.sh/Qc3we/views_php_201921_38.tar --- nevermind it doesnt work...

MustangGB’s picture

The patches work fine with 1.x, I believe he's simply speaking in the abstract.

Perhaps if some of the people using 2.x in the wild would chime in on what the release blockers might be we could all get back on the same page, but this is not really the issue for such discussions me thinks.

joseph.olstad’s picture

Status: Needs work » Active

I am using patch #68 with the library mentioned in #53 and it works in both php 7.2.x and php 7.3.x

however the patch breaks php 5.6.x with the following error:
Parse error: syntax error, unexpected '(' in sites/all/modules/contrib/views_php/views_php.module on line 34
line 34 is this:

  $access = (bool) $function[$view_name . ':' . $display_id]->getClosure()($view_name, $display_id, $account);
  // php 5.6.x is BARFING ON ->getClosure()();

here is what I found on php.net might be related, if anyone is really keen they might figure this out:
https://www.php.net/manual/fr/functions.anonymous.php#119388

here is a sandbox php 5.6 environment with php 7.1.x as well, shows precisely the same issue:
// php 5.6.x is BARFING ON ->getClosure()();
see example code:
https://3v4l.org/P1MRc

source code from the 3v4l.org example:


class A {
    public function foo() { 
        echo __FUNCTION__;
    }
}

class B extends A {
    public function getClosure() {
        return function() {
            parent::foo();
        };
    }
}

class C {
    public function __construct(B $b) {
        $b->getClosure()();
    }
}

new C(new B);

Output for php 7.1.x / 7.2.x / 7.3.x
foo

Output for php 5.6.x

Parse error: syntax error, unexpected '(' in /in/P1MRc on line 19

Process exited with code 255.
FeyP’s picture

Status: Active » Needs work

Thanks Joseph for testing the patch with different versions of PHP. I'm setting the issue status back to needs work, since we have a patch and we just need to work on it a bit more to fix the remaining issues with it.

MustangGB’s picture

The solution to #78 (and #57) was already posted in #63.

These were intended to be resolved in #68, however it looks like one got missed, so should be straightforward to resolve.

joseph.olstad’s picture

Assigned: Unassigned » joseph.olstad
Status: Needs work » Active
FileSize
14.49 KB
535 bytes

Thanks @MustangGB,
here is the new patch based on the solution posted in #63.

interdiff is included for readability.

joseph.olstad’s picture

Assigned: joseph.olstad » Unassigned
Status: Active » Reviewed & tested by the community

Ok, back to RTBC, I tested patch 81 with both php 5.6.x AND php 7.3.x , works beautifully in both cases.

joseph.olstad’s picture

Status: Reviewed & tested by the community » Needs work

crap ya I'm having the same issue as spelcheck with VBO.

see #2274543-67: [7.x-1.x] Remove usage of deprecated create_function()

with that said, patch 81 is the best so far.

joseph.olstad’s picture

Status: Needs work » Needs review
FileSize
987 bytes

Ok I've patched the library, the warning no longer shows up, HOWEVER, I'm still reviewing the results....

Here's the patch for the closure project.

I've forked the closure project if you prefer you may use my fork:
https://github.com/joejoseph00/closure

MustangGB’s picture

Nice job, thanks for chasing this one, I noticed you submitted a pull request upstream as well.

joseph.olstad’s picture

ya the author of the closure library already replied, some minor formatting and code style changes were made to the solution, either way works.

https://github.com/opis/closure/pull/38

he is reviewing this , the travis ci is passing.

I haven't found any regressions with this solution "yet" however I do not really entirely understand what this closure library is doing. If I find a regression I will report back, however so far so good.

joseph.olstad’s picture

FileSize
1.34 KB

I had to adjust the patch for the closure library.. It's improved.
still have not found a regression with this.

use the closure library with the closure library patch:

and views_php version 7.x-1.x with this patch from #81

works on php 5.6.x AND php 7.3.x

with or without views_bulk_operations

I am still searching for possible regressions however have not yet found any.

joseph.olstad’s picture

***EDIT***
NEVERMIND this comment... I re-read this entire thread... Cross fingers that the patch I provided for the closure library does not have any regressions.......
ALTERNATIVELY:

check the Drupal 8 version of this solution. It doesn't use the closure library AT ALL....

see the custom function it uses instead:

/**
 * Internal support: create_function() emulation using anonymous functions.
 */
function views_php_create_function($args, $code) {
   return eval('return function (' . $args . ') {' . $code . '};');
 }

Why can't we use this function instead of getClosure() and avoid using the closure library altogether?

Please check the Drupal 8 version of this solution: #3026201-3: Function create_function() is deprecated in Drupal\views_php\Plugin\views\field\ViewsPhp->render() (line 122 of modules/contrib/views_php/src/Plugin/views/field/ViewsPhp.php patch number 3, no closure library is required.. might be a lot simpler and a lot lower risk.

DamienMcKenna’s picture

Something to consider..

Create a new branch of the module that only works with PHP 7.x (pick a branch) and mark 7.x-1.x as unsupported.

joseph.olstad’s picture

@DamienMcKenna, can you please elaborate on this very interesting idea? I like the discussion.

DamienMcKenna’s picture

Leave the 7.x-1.x branch focused on PHP 5.x with complete disregard for PHP 7.

Create a 7.x-2.x branch. Decide which specific version of PHP to stick with - 7.0, 7.1, 7.2, etc, and list that in the info file.

Write this patch for the 7.x-2.x branch, i.e. PHP 5 compatibility doesn't matter anymore.

Profit.
(well, no, not really, but YKWIM)

joseph.olstad’s picture

Update: I spent more time working on the suggestions from the upstream developers of the opis/closure library and the opis/stream library.

The TESTED successful approach:

patch 81 and my forked closure library is working.
I had to adjust the patch for the closure library.. It's improved.
still have not found a regression with this.

OR if you like patching, use the non-forked opis/closure library with the closure library patch:

OR use this forked closure repo that includes the closure patch (no need to patch this fork of the opis/closure library):
https://github.com/entreprise7pro/closure

and views_php version 7.x-1.x with this patch from #81

works on php 5.6.x AND php 7.3.x

with or without views_bulk_operations


The FAILED but suggested by library maintainers approach.

The opis/stream library approach does not work, it is passing seralizable functions and causes a core crash here:
Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /path/to/drupal/includes/cache.inc).

or get this from another one of their suggestions Warning: Parameter 3 to Opis\Closure\SerializableClosure::{closure}() expected to be a reference, value given in Opis\Closure\SerializableClosure->__invoke() (line 109 of web/sites/all/libraries/closure/src/SerializableClosure.php).

joseph.olstad’s picture

Ok slight change to the patch

see interdiff, I am changing the vendor of the closure library from opis to entreprise7pro.

this patch 94 is intended to be used with the entreprise7pro forked version of the closure library

joseph.olstad’s picture

new patch forthcomming.

joseph.olstad’s picture

ok, the upstream closure library maintainers were very helpful and pointed out how to implement this without needing to patch their closure library.
So, use the opis/closure library version 3.4.0 or newer (you must upgrade if you are using an older version of this)

and use this patch:

ibakayoko’s picture

@joseph.olstad thanks for the patch.

After applying the patch i get a new error when i try to run a VBO action

Error: Unsupported operand types in form_get_cache() (line 525 of /Users/../../docroot/includes/form.inc).

joseph.olstad’s picture

@ibakayoko , what version of php are you running when this error occurs ?

joseph.olstad’s picture

also, what version of the closure library are you using? 3.4.0 ?

joseph.olstad’s picture

ibakayoko , if you're using memcache, did you flush your memcache after applying the patch and correctly installing the closure library?

MustangGB’s picture

Awesome, I think it's a bit clearer if we write it this way.

Also standardised the int cast spacing with the rest of the codebase, and restored some whitespace that was removed unnecessarily.

DrupalYedi’s picture

The patch #81 is working for me to hide the "deprecated error message"... - BUT in my case individual fields under "available variables" are not working. The value of individual fields is always the node ID instead of the field value. Only the Avaiable variables "$row->nid" and "$row->title" are working correct.
Does anyone have the same problem? Any idea how to solve it?

joseph.olstad’s picture

 @DrupalYedi, you should use the patch # 95 with the closure library, install the closure library as mentioned in the above thread, get the latest release of the closure library, apply the patch 95, should work.

I haven't tested MustangGBs patch however it could also do the job.

I have been using patch 95 in combination with VBO , works fine as does regular views php without vbo.
We're using php 7.3.x

re-patch from 7.x-1.x head of dev branch

joseph.olstad’s picture

Some related issue with php 7.3.x (7.2.x) compatibility

I have a VBO form with views_php field in an ajax form

1st clue is this error message:

Error: Unsupported operand types in form_get_cache() (line 525 of web/includes/form.inc).

I put a debug into line 525 of includes/form.inc (core) and dumped the '$cached' object, the 'data' attribute is empty..

stdClass Object                                                          
                                        (                                                                        
                                            [cid] => form_state_form-Z9lwJhZrN77eVDpAEqnWCaxPWiRyqnu_bEOdzYmkUss 
                                            [data] =>                                                            
                                            [created] => 1572579877                                              
                                            [expire] => 1572601477                                               
                                            [serialized] => 1                                                    
                                        )  

Going further back, it appears to be something with the cache system not being able to unserialize our closure.

2nd clue:

Warning: Class __PHP_Incomplete_Class has no unserializer in DrupalDatabaseCache->prepareItem() (line 449 of web/includes/cache.inc).

3rd clue:

Notice: unserialize(): Error at offset 118005 of 301163 bytes in DrupalDatabaseCache->prepareItem() (line 449 of web/includes/cache.inc).

So the cache system is unable to unserialize our closure as per includes/cache.inc

If someone has a solution for this, please share.

I think it is related to the cache system in core not liking php 7.3.x / 7.2.x and unserializing closures.

I'm looking at what looks to be a related core patch #2819375-94: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)

however it is an 'opt-in' patch, which allows configuring which forms get 'excluded' from cache, unfortunately this is impossible with the views_php vbo form ajax I'm using because the form id changes every cache clear.

joseph.olstad’s picture

Wow , I cleared caches, re-applied the core patch, WOOT fist pumps, it works!
#2819375-94: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)

I did some more testing, php 5.6.x does not have this problem, works fine with the closure library. the issue I was observing was only occuring for me in the php 7.3.x (I didn't test 7.2.x but it probably same thing happens there).

SO, for those of you that use this, add the core patch too

joseph.olstad’s picture

ah Sorry for the bad news, but my last test I was running php 5.6.x , after I switched back to php 7.3.x I got the error described in comment #103

So no, that core patch, I'd have to configure the random form id in the variable as per the core patch, which changes dynamically, so meh, need to review this some more.

joseph.olstad’s picture

Ok, update, good news, the core patch 42 by Quicksketch does the job.

here it is: https://www.drupal.org/files/issues/drupal-no_cache_form-2819375-42.patch
#2819375-42: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)

So to summarize, use patch 100 by MustangGB here
and the closure library as discussed above

and core patch 42 from 2819375

joseph.olstad’s picture

Ok, status summary:

To use views_php with php 7.2 is pretty easy

Step 1) download the latest 7.x-1.x branch (see other issue for the 2.x branch) dev release or latest release probably works too
Step 2) apply MustangGBs patch 100 above #2274543: [7.x-1.x] Remove usage of deprecated create_function() to the views_php module
Step 3) install the latest opis closure library To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php
Step 4) Make sire to have prerequisite libraries module installed and enabled.

Optional steps
Step 5) if you have issues with ajax forms in vbo , apply this core patch which resolves massive form caching issues.
Step 6) if you are using php 7.3 or higher you will need this diff applied to core AFTER the patch from step 5 is applied after applying that core patch:

Step 7) clear caches, have fun.

joseph.olstad’s picture

FileSize
29.09 KB

ok everybody, more followup.

I repeat, no errors in php 5.6 , php 7.0 , php 7.1 , php 7.2

everything is just peachy in those versions of php

With php 7.3 I found the following:

I'm seeing this on Drupal 7 with php 7.3.x using views_php and the closure library.

The error happens in includes/cache.inc

    // If the data is permanent or not subject to a minimum cache lifetime,
    // unserialize and return the cached data.
    if ($cache->serialized) {
      $cache->data = unserialize($cache->data);

the error is as follows:

Warning: Class __PHP_Incomplete_Class has no unserializer in DrupalDatabaseCache->prepareItem() (line 451 of /var/web/includes/cache.inc).

next is a notice:
Notice: unserialize(): Error at offset 255029 of 338002 bytes in DrupalDatabaseCache->prepareItem() (line 451 of /var/web/includes/cache.inc).

I originally got the error at offset 133000 something, then I cranked up my php memory limit to 2048 MB , it went to a higher number offset, then I cranked it up to 4096 MB as a test, the offset seems to be similar

but it's definately the same serialized data that is not wanting to unserialize.

Wwe're using the opis/closure library to replace create_function in views_php at the above latest patch of course .

I've debugged far enough to get the serialized nastiness that throws this, I had to copy all the lines into a test.php file and use the multiline variable technique and managed to unserialize it without errors using the unserialize() function. The serialized output had too many escape characters and unquotes to be handled another way. Weird that only php 7.3 barfs though.

$var = << ALL THE NASTY STUFF GOES HERE

more nasty stuff
NASTYMULTILINESERIALIZATIONWITHVIEWS_PHP;

then
echo unserialize($var); // works fine

but it crashes in cache.inc , ONLY in php 7.3 , does not crash in php 7.2.

Attached is what I'm 99% sure the serialized data that is blowing up on, it's got views_php serialized , see test_php.tgz
however, run the php from the command line, there's no issue, it's only an issue in includes/cache.inc

see the reported similar core issue: #2882456: Drupal 8.4.x and PHP7 unserialize compiled route - __PHP_Incomplete_Class error
people noticed this in D8 at some point, but it was not really the same issue. Same symptoms but different source of the issue.

***EDIT***
the previous comment refers to errors observed with php 7.3.8.1
I'm hopeful that php 7.3.9.x will resolve the issues, seems like a bug in php, why would it all of a sudden have problems with unserialize when all the previous versions didn't have this problem?

***END EDIT***

MustangGB’s picture

@joseph.olstad Related to https://bugs.php.net/bug.php?id=77302 perhaps?

If so:

why would it all of a sudden have problems with unserialize when all the previous versions didn't have this problem?

Might be explained by:

7.2 and below are affected by the same problem.
The old versions don't fail, but silently produce incorrect result (not the same as was serialized).

joseph.olstad’s picture

@MustangGB , ya , sounds like the same bug for sure, there's a lot of bug fixes in 7.3.9.0 , as soon as I upgrade I'll re-test after I upgrade from 7.3.8.1

joseph.olstad’s picture

In D8 there is a core issue that looks related, so I made a D7 child issue from that one.
#3093110: D7 - EntityType objects cannot be reliably serialized without DependencySerializationTrait

rgpublic’s picture

It seems on Symfony they also talked about this:

https://github.com/symfony/symfony/issues/29459

And they also finally point to the bug already mentioned by MustangGB:

https://bugs.php.net/bug.php?id=77302

I think it's all the same bug. It's not in Drupal 7/8/whatever but definitively in PHP. AFAIU the bug is not introduced in PHP 7.3 but this version is more strict and crashes. In earlier version the unserialization is wrong but probably as long as the unserialized data isn't used somehow, the error doesn't cause any user-visible problems.

The import comment is probably:

I plan to propose and implement a new custom object serialization mechanism for PHP 7.4, to replace the Serializable interface and all the problems that come with it.

For now, all I can suggest is to rewrite your code in a way that does not use parent::serialize(). I don't think there is anything we can do to fix Serializable itself, unfortunately.

I cannot imagine, though, that at least D8 will remain incompatible with all PHP 7.3 versions due to this. For D7 I wonder, wouldn't it be enough to apply the workaround for those classes where the error is most prevalent?

joseph.olstad’s picture

From what I gather, in D8 they work around this by using DependencySerializationTrait
see #3093110: D7 - EntityType objects cannot be reliably serialized without DependencySerializationTrait and the parent D8 issue, the original.

I'm not sure what all this means though, sounds like a workaround is needed in some cases. What boggles me is according to my tests I don't see any issue until I switch to php 7.3.8.1. php 7.2.x does not throw any warning or error messages.

Something changed in the php 7.3.x serialize() function that makes it behave differently in some cases.

rgpublic’s picture

BTW: I wonder whether it might be advisable to not use the Closure library to serialize the closure because actually the code should be available as a string before we even turn it into a Closure. Perhaps the closure could be replaced with a proxy class that holds the closure and the original code. When it's time to serialize/sleep we only serialize the code string we already have and discard the closure. And on wake-up we create the closure from the code string again. This way we'd avoid having to serialize/unserialize the closure in the first place - removing the dependency on OPIS lib as well. Wouldn't that work? Don't know much about this module though, so it's just a thought. Came here from the general PHP 7.3 support issue.

joseph.olstad’s picture

@rgpublic , ya I think you might be on to something, upstream they mentioned problems occur when something gets serialized twice, once maybe by closure, then again by the cache.inc when something gets shoved into the cache?
I'm guessing of course on this.

The same issue exist in PostAuthenticationGuardToken::serialize because the token is serialized twice.

rgpublic’s picture

@joseph.olstad: Yeah, that's weird. But I guess to achieve a *solution* for those problems it doesn't actually matter whether the problem is during serialization or unserialization - even though I understand it'd be interesting from a "scientific" POV.

I think that the important part from DependencySerializationTrait that avoids this error is just that __sleep() is implemented and the object is somehow cleaned of anything that's not strictly needed during __wakeup(). In D8 I also encountered some similar problems in the past when there are e.g. variables pointing to services etc. But, of course, you can remove those and add them back on wake-up because the services are readily available (just an example). So IMHO it boils down to: If you have problems serializing objects of a specific class, make sure to implement __sleep (be it via a trait or directly) in that class and use it to remove any cruft (i.e. variables) that are on your object (variables pointing to other objects in particular) to avoid serializing a huge complicated interdependent "network" of connected objects which will inadvertetly "stress-test" PHP's serialiation engine . This should also cut down on the size of the final serialization string.

MustangGB’s picture

If the issue is to do with double encoding and/or storing in the database, then the base64 wrapper trick might work: https://davidwalsh.name/php-serialize-unserialize-issues

Would be good to have some steps to reproduce this though, as it's not something I've seen on my own sites.

joseph.olstad’s picture

@MustangGB, thanks for the suggestion, I'm trying to figure out an easy way to reproduce this bug.

Not entirely sure if it's related to the closure or not, there's a lot of things going on and a very complicated setup.

With that said, I did try the base64_decode and base64_encode in the closure library, it didn't seem to make any difference , even after flush cache / cache rebuild.

diff --git a/src/SerializableClosure.php b/src/SerializableClosure.php
index bef71a3..2dff64b 100644
--- a/src/SerializableClosure.php
+++ b/src/SerializableClosure.php
@@ -164,6 +164,7 @@ class SerializableClosure implements Serializable
             $this->scope = null;
         }
 
+        $ret = base64_encode($ret);
         return $ret;
     }
 
@@ -239,7 +240,7 @@ class SerializableClosure implements Serializable
             $data = $data['closure'];
         }
 
-        $this->code = \unserialize($data);
+        $this->code = \unserialize(base64_decode($data));
 
         // unset data
         unset($data);

so for that reason, I think that the issue I'm observing is not actually related to the closure library OR views_php but rather some entity relationship of some kind.

, the site I'm debugging is using workflow (rules) , entity , editablefields (ajax form in here, I'm thinking it's more editablefields that is the culprit now that I'm getting closer, but I haven't created yet clear steps to reproduce, so it's all tentative guessing at this point).

so I'm going to look at editablefields and zero in on this.

Thanks for your assistance.

I'm thinking that this views_php patch is good, the closure library is good, with that said, I think this base 64 encoding is probably a good idea after serialize and base 64 decode before unserialize.

See the above patch code, might be worth reviewing just for kicks, see what the closure guys say upstream. However, ya probably not even needed at this point, it might however be a good safeguard.

MustangGB’s picture

It might produce the same result as what you posted, but I was thinking more like this:

- $closure = $function->getClosure();
+ $closure = base64_decode($function->getClosure());
...
  function views_php_create_function($args, $code) {
    libraries_load('closure');
    $closure = \Opis\Closure\SerializableClosure::createClosure($args, $code);
-   return new \Opis\Closure\SerializableClosure($closure);
+   return base64_encode(new \Opis\Closure\SerializableClosure($closure));
  }
joseph.olstad’s picture

Ok, status summary update again:

The php 7.3.x issue I mentioned above is most likely not related to views_php or the closure library, I am still investigating but now I'm looking into the editablefields module which is being used on the site that I observed the issue..

I have tested patch 100 and the opis/closure library to work with views_php and php 5.6.x , 7.0, 7.1, 7.2, 7.3.

Step 1) download the latest 7.x-1.x branch (see other issue for the 2.x branch) dev release or latest release probably works too
Step 2) apply MustangGBs patch 100 above #2274543: [7.x-1.x] Remove usage of deprecated create_function() to the views_php module
Step 3) install the latest opis closure library To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php
Step 4) Make sure to have prerequisite libraries module installed and enabled.

Step 5) clear caches, have fun.

joseph.olstad’s picture

@mustanggb, I am really looking at all solutions and so I did try your most recent suggestion just now, it throws the following:

so I get :
Warning: base64_encode() expects parameter 1 to be string, object given in views_php_create_function() (line 190 of /web/sites/all/modules/contrib/views_php/views_php.module).

--- a/views_php/views_php.module
+++ b/views_php/views_php.module
@@ -31,7 +31,7 @@ function views_php_check_access($php_access, $view_name, $display_id, $account =
   }
 
   ob_start();
-  $closure = $function[$view_name . ':' . $display_id]->getClosure();
+  $closure = base64_decode($function[$view_name . ':' . $display_id]->getClosure());
   $access = (bool) $closure($view_name, $display_id, $account);
   ob_end_clean();
   return $access;
@@ -187,5 +187,5 @@ function views_php_libraries_info() {
 function views_php_create_function($args, $code) {
   libraries_load('closure');
   $closure = \Opis\Closure\SerializableClosure::createClosure($args, $code);
-  return new \Opis\Closure\SerializableClosure($closure);
+  return base64_encode(new \Opis\Closure\SerializableClosure($closure));
 }

I'm actually thinking that the problem I'm observing with serialize/unserialize is related to editablefields which caches an ajax form and some entity data along with the views_php closure, possibly a double serialize somewhere, not sure.

joseph.olstad’s picture

@MustangGB, and everyone else, thanks for your assistance, I ended up finding a way to work around the unserialize() / serialize() issue with php 7.3.x in my complicated ajax form which also happens to be using the editablefields module in addition to views_php.

It turns out that the editablefields module is rebuilding forms from cache, something happens which I'm not sure of somewhere, that blows up in php 7.3.x

to work around the issue, I set the form_state 'rebuild' array index value to FALSE

this totally avoids the form_cache and no longer needs to serialize / unserialize whatever it was trying to do (probably a double serialize() )

so, using the above views_php patch, the opis/closure library, and the core patch for ajax forms, plus a patch to editablefields , my php 7.3.x environment is working prestinely.

here's the patch for editablefields

diff --git a/editablefields.module b/editablefields.module
index 38ef90e..b3f8a65 100644
--- a/editablefields.module
+++ b/editablefields.module
@@ -177,7 +177,7 @@ function editablefields_field_ui_display_overview_multistep_submit($form, &$form
   $values = $form_state['values']['fields'][$field_name]['settings_edit_form']['settings'];
   $form_state['formatter_settings'][$field_name] = $values;
 
-  $form_state['rebuild'] = TRUE;
+  $form_state['rebuild'] = FALSE;
 }
 
 /**
@@ -527,7 +527,7 @@ function editablefields_form_submit_edit_mode(&$form, &$form_state) {
   array_pop($parents);
   array_pop($parents);
   _editablefields_set_edit_mode($form_state, TRUE, $parents);
-  $form_state['rebuild'] = TRUE;
+  $form_state['rebuild'] = FALSE;
 }
 
 /**
@@ -558,7 +558,7 @@ function editablefields_form_submit(&$form, &$form_state) {
   entity_form_submit_build_entity($entity_type, $entity, $element, $form_state);
   entity_save($entity_type, $entity);
 
-  $form_state['rebuild'] = TRUE;
+  $form_state['rebuild'] = FALSE; // PHP 7.3.x can break if the form is rebuilt from cache.
   // Put back the updated entity for used during form rebuild.
   $form_state['editablefields_entities'][$entity_type][$entity_id] = $entity;
 }

Sorry for the noise, refer to comment #120 for how to use views_php with the new opis/closure library which works wonderfully with php 7.2 and php 7.3

Thanks!

joseph.olstad’s picture

ah, actually, during my test I was using a small core patch to includes/form.inc which works around a php 7.3.x

unsupported operand types in form_get_cache

, there still may be an empty cache in php 7.2 and previous versions of php, but they do not throw an unsupported operand types like php 7.3.x does.

not sure if this is closure related or not but seems to affect only php 7.3.x

here's the patch to core:

https://www.drupal.org/files/issues/2019-11-04/php7.3_edge_case_closure_...

basically , I changed one line in includes/form.inc

-        $form_state = $cached->data + $form_state;
+        $form_state = (empty($cached->data)) ? $form_state : $cached->data + $form_state;

So this, in addition to the editablefields patch for editablefields resolved my issue.

see the full explanation here: #3093423: php 7.3.x compatibility, unexpected operands in form_get_cache and too few arguments

MustangGB’s picture

Ah of course, yea ignore me, was just a thought, but you're right, it would have to be after the serialization.

joseph.olstad’s picture

joseph.olstad’s picture

Ok, the patch that I thought fixed the issue, just masked it again, it all came back to the opis/closure library and the views_php module.

As soon as I disable the views_php module, the editablefields module works fine in all ways.

I've openned a support request with the opis/closure team.

https://github.com/opis/closure/issues/43

rgpublic’s picture

Still, i don't understand at all why opis/closure is used at all. It seems to me way overkill to bring in this library for no reason. It is far easier to get a serializable closure in PHP. See the following proof-of-concept:

https://gist.github.com/rgpublic/499071d10bfe067ffc271f60f75ec132

The library, as far as i can see, extracts the code of the closure from wherever it is in the source code. This is why it is so complicated. IMHO we don't need that here as the code is already readily available as a string. The original problem was only to replace the create_function() call because it's deprecated. See my example above how that is easily achievable without any huge additional library/dependency.

MustangGB’s picture

How can it be done without eval() though?

rgpublic’s picture

@MustangGB: Why should it be done without "eval"? "create_function" is deprecated in PHP 7.2 - not "eval".

MustangGB’s picture

As per the issue summary and PHP 7.2 release notes, create_function() was deprecated because it uses eval() internally, being that it has performance, memory, and security implications and the world has been migrating away from eval() for sometime, it doesn't seem logically to me to be introducing another instance of what we're trying to be rid of when another viable option is available.

Also I wouldn't be surprised if eval() were to itself be deprecated at some point, for the same reasoning, it'd be nice to have this issue resolved, without needing to comeback to it again in the near future.

I've not testing it, but opis/closure also notes:
You can serialize/unserialize any closure unlimited times, even those previously unserialized (this is possible because eval() is not used for unserialization)

rgpublic’s picture

Well, eval probably doesn't go anywhere for the time being. Many things in the PHP world are relying on this. And nothing like that has even been officially announced or created as an RFC. So, the idea that eval is going away is currently your very own speculation. Yes, eval has indeed major security implications but it is a misunderstanding that they are magically solved by opis/closure. If you see the documentation of that library you will notice that it doesn't claim to do that.

What this library does (AFAICT) is: Create a temporary in-memory php file and include that. And if we speculate that eval is going away then we might as well speculate that the support for including files from in-memory php files via a stream wrapper is going away. And to be honest, I think that support for the latter is very ugly and support for that might leave us sooner than eval. What the library does is cleverly masking the fact that untrusted code is run. It's no better than "eval".

The security problem of eval is that a string/user data is turned into code. I agree, this is quite terrible for a multitude of reasons. But, then again, this is what this modules (views_php) is all about. If you want to avoid the security hole of mixing user data and code execution that you cannot use this module. It's as simple as that. This module is a very bad idea security-wise and I myself don't use it for this very reason but there's nothing to be gained whatsoever by using the opis/closure library. It just brings in LOTS of new code that might have additional security problems as well and it doesn't solve any current actual problem.

MustangGB’s picture

Fair enough, if we accept that eval() is the way to go, then perhaps we can look at adding some of the ideas from SuperClosure to the #127 code, for starters I'm wondering if it should implement Serializable and if __invoke() could use func_get_args().

Although SuperClosure does again mention the issue of:
Cannot serialize closures that are defined within eval()'d code. This includes re-serializing a closure that has been unserialized.

Are we sure this won't be something that we hit when caching for example?

rgpublic’s picture

@MustangGB: I think (might be wrong) __invoke cannot use func_get_args() because from the source-code it looks we need call-by-reference. The references are destroyed with func_get_args. That's why I'm faking this with arg1...arg9.
Serializable (or __sleep/__wakeup) must be implemented if you store the closure in a variable of that object. I'm currently recreating it everytime in my GIST example. It might in fact have performance benefits if the closure is only created once and then cached. OTOH you can also just store the cached closure as a function-local static variable:

function __call(...) {
   static $closure;
}

I think(!) it won't be serialized in that case and you can save yourself from implementing Serializable completely. Implementing Serializable is only necessary if the variables you want to serialize are somehow different from the variables the object has on it naturally.

For the other features in super-closure: What features do you particularly find interesting? For the most part I guess they are not needed since in our case it's *us* who are actually building the closure. The user-provided code we make our closure out of probably shouldn't/wouldn't use e.g. $this etc. because this could not possibly have worked before with create_function anyway.

DamienMcKenna’s picture

Just to mention it, if folks are worried about whether or not to use eval() maybe they it shouldn't be possible to store PHP code in a Views configuration field?

DamienMcKenna’s picture

It's also worth noting that Views itself uses eval() in several files:

  • includes/admin.inc line 2083
  • includes/view.inc line 1996
  • plugins/views_plugin_argument_default_php.inc line 66
  • plugins/views_plugin_argument_validate_php.inc line 66

So using eval() is safe for now.

skylord’s picture

Agree with previous posts. Tools like Closure library are certainly overkill for discussed purpose. Actually the whole views_php module is just a UI wrapper over eval() invoсation. If someone wants to avoid eval() - he has to write usual views field handler.
@rgpublic is right - the way Closure works seems like a rough hack and mentioned problems with php 7.3 and etc are proofs of that.

MustangGB’s picture

For the record, I'm not "worried" per se, only asking the questions as to why one approach might be preferable or not over the other.

I don't see the use of a stream wrapper based include as hacky, each implementation has pros and cons, if none of the cons of eval() effect us, and it's a lighter solution, then so be it.

ruslan_drupaler’s picture

#4 Worked smoothly for me. Thanks a lot!

yurg’s picture

joseph.olstad’s picture

eval() is probably the way to go. Either solution probably has similar related limitation that occurs when serializing certain objects . if a view has a form for instance like when editablefields module provides an ajax interface on a view and is used with views_php, the form is cached by core and serialized when cached and unserialized after loading from cache.

Forms like this contained in views_php result in or expose a limitation of the serialize function.

In versions of php prior to php 7.3, the serialize failures were silent, but as of 7.3 now they throw warnings.

This is pretty much what I observed. So to resolve I moved the views_php code out of the views ajax form that is created by the editablefields module.

DrupalYedi’s picture

@joseph.olstad #102...thank you for your support and your engagement in that thread!
For me patch #95 or#100 are both working (PHP 7.2) :-) but aren't showing the values of the fields in the view correct :-(:
Example:
Vou have a view with:
field_myPicture
field_myDescription
field_myButton
field_myPHP1 //Print something like print 'xyz';

and then you have another PHP field in the view and try to render the individual fields from that field with the placeholder normal avaiable variables:
print $row->field_myPicture; //RESULT: node ID
print $row->field_myDescription; // RESULT: node ID
print $row->field_myButton; // RESULT: node ID
print $row->myPHP1;// RESULT: EMTPY
...
only $row->nid and $row->title have the correct values.

with print_r($row) one can see, that the values in the $row Array are not correct. Any Idea?

eit2103’s picture

Came in here for this issue. Would there be a new release soon with this patch or do you suggest that I just use the patch.

eit2103’s picture

By the way, patch #4 throws the following error.

ParseError: syntax error, unexpected end of file in _registry_check_code() (line 249 of /home/eit/public_html/sites/all/modules/views_php/plugins/views/views_php_handler_field.inc).

phrk’s picture

I have tried not only patch #100, but almost all patches posted above. But I simply cannot make it work with php 7.3 and DrupaI 7.69. I keep getting this error:

Error: Class 'Opis\Closure\SerializableClosure' not found in function views_php_create_function() (row: 198 in file (...)/sites/all/modules/views_php/views_php.module).

I have tested all forks of Closure library mentioned above. I have views_php 7.x-1.x-dev installed. I have cleared all caches every time. I have also tested suggestions mentioned in comments of similar issue 3055655 with views_php 7.x-2.x-dev.

I have been trying to solve that for many hours with almost no success... I would be very grateful for any idea anyone may have :)

MustangGB’s picture

@phrk

Sounds like you haven't got the library installed correctly, if it helps version 3.4.1 is what I currently have.

You should have it installed such that the following file exists sites/all/libraries/closure/autoload.php, it's just an example to show you the correct directory structure, all the files should be included.

When you visit /admin/reports/libraries it should also appear as installed.

phrk’s picture

Thank you, MustangGB! I downgraded closure library from current 3.5.1 to 3.4.1 and that solved my problem.

joseph.olstad’s picture

@phrk could you please open a followup issue on github in the opis closure project reporting the regression and the fact that you had to downgrade to 3.4.1 from 3.5.1?
The developers there should respond and they have been helpful previously for us.

Thanks

phrk’s picture

Anybody’s picture

Hi all, is #100 the test to review now to take things forward here?

Is there an active maintainer in this project willing to commit this after RTBC?

MustangGB’s picture

@Anybody

Hi all, is #100 the test to review now to take things forward here?

#100 is working fine, as mentioned in #146 there might be a bit of tweaking to get it working with newer library versions.

However I think the next thing to do would be to try @rgpublic's approach from #112-#133 to see if this works without the need of an additional library. I haven't bothered to try this because as mentioned, #100 is working fine for my purposes at the moment.

As mentioned in #89, if we go the library route, it might make more sense to do it as a new branch.

Is there an active maintainer in this project willing to commit this after RTBC?

Doesn't look like there are any active maintainers at the moment :'(

Mitnick’s picture

+1 on #18. Thanks a lot for providing the patch for 7.x-1.x-dev, @cirrus3d.

shubhangi1995’s picture

Assigned: Unassigned » shubhangi1995
quak’s picture

I created this simple patch to solve problem without use Opis Closure library.

shubhangi1995’s picture

patch #15 as well as patch #153 solved the error for me without the library.

shubhangi1995’s picture

Assigned: shubhangi1995 » Unassigned
Status: Needs review » Reviewed & tested by the community
sgroundwater’s picture

Patch #153 worked for me. I am thankful for not needing any additional library files. Thanks for the help quak.

cmseasy’s picture

Patch 153 is a patch from the root of drupal. Please create a patch from the module folder, https://www.drupal.org/node/707484

bisonbleu’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
7.99 KB

Refactoring @quak's patch in #153 from the module's directory.

cmseasy’s picture

Status: Needs review » Needs work
Issue tags: +PHP 7.3

After applying patch #158 to latest dev and echo "Hello World"; in the output code construction field the 'Hello World' was not rendered. Using php 7.3. Removed the patch and rendering is back.

Changed status to 'Needs work'.

guistoll’s picture

#15 for 7.x-1.0-alpha3 works.

jfuentes’s picture

I've applied the patches, but i still have the issue Warning: Class __PHP_Incomplete_Class has no unserializer a DrupalDatabaseCache->prepareItem()

In my case the view has views_php & editablefields, and the error happens when the editable field is configured to be editable on click

siramsay’s picture

I tested #158, even though the Deprecated function message stopped, the views PHP no longer worked correctly, it changed the PHP code into sanitized values for output.

Both recipe at #120 with opis closure library installed and #15 worked.

I went with #15 with 7.x-1.0-alpha3 for ease of use.

chill5-0’s picture

+1 on #15 for 7.x-1.0-alpha3

frazac’s picture

+1 on #15 for 7.x-1.0-alpha3

MustangGB’s picture

If you read the issue you'll know what the problem with #15 is, namely, as mentioned in #52, that it breaks when using caches, which are used everywhere.

If you'd like to move the issue forwards the next steps are to implement rgpublic's suggestion, see #133 and previous comments.

puddyglum’s picture

Until there is a stable version available (using method from #133), using instructions from #120 is working for us.

Liam Morland’s picture

Title: [PHP 7.2] Remove usage of deprecated create_function() » [7.x-1.x] Remove usage of deprecated create_function()
Liam Morland’s picture

What is the problem with #13? That looks like the most straight-forward replacement of create_function() with anonymous functions.

rgpublic’s picture

@Liam-Morland: See #165

Liam Morland’s picture

So #13 has the same problem as #15. Which caches are the problem? In #11#8, the anonymous functions are created right before they are used. They are not stored in object properties. I had the impression that storing closures in object properties was the problem.

rgpublic’s picture

@Liam Morland. I see. Ha! Funny. Could it really be that simple? That the solution was always right in front of us in the first place? I have do admit that I also came quite late to the party. When reading the comments again now, it seems like the solution #11 was just ignored and everyone kept going on as if #11 never happened. Anyone's really seeing any problems with #11? I don't have a decent way to test it at the moment, cause I'm not using the module. Anyone getting caching errors?

Liam Morland’s picture

Status: Needs work » Needs review
FileSize
9.98 KB

Here is a patch most similar to #8. It uses regular anonymous functions, declared just before they are used and not stored in object properties.

This patch is also similar to #158 except that this one runs eval() on each code block instead of just returning the code. It also uses use to pass the code into the anonymous functions.

rgpublic’s picture

@Liam Morland - on a cursory glance, that approach makes a lot of sense to me. In hindsight, it's terrible what kind of a detour this has taken. Unreal. Due to the long discussion that ensued and the proposal to even add the dependency to a full closure serialization library, I was always under the impression, that it's unavoidable that the closure ends up in the serialized data because of some obscure reasons how caches in Drupal work.

Just to make sure: Did you test this with cached views and dev mode disabled that the view is still properly cached?

BTW: There're still some minor spelling errors in the comments: "Ecexute"

Liam Morland’s picture

Yes, I noticed the spelling errors. I am inclined to fix those in a separate commit so that this patch only addresses this issue.

My testing of this has not been extensive. I don't know what all has to be tested to be sure there are no problems with caching, etc. This patch is informed mostly by my understanding that the modern equivalent to create_function() is the anonymous functions. The long detour suggests that I may have missed something.

Liam Morland’s picture

This is #172 with a couple of small errors fixed.

I have tested it and it works for all the Views that we use, but we only use some of the features of views_php, so this cannot be considered a comprehensive test.

Liam Morland’s picture

FileSize
1.46 KB
zipymonkey’s picture

I tested the patch in #175 once several of our sites and the patch works for me. These sites are using the views_php_handler_field.inc and views_php_handler_field.inc plugins. I did not test the area, filter or cache plugins.

JGReidy’s picture

After applying patch #175, I got
Error: Call to undefined function is_boolean() in views_php_handler_field->php_click_sort() (line 179 of .../sites/all/modules/views_php/plugins/views/views_php_handler_field.inc).

Liam Morland’s picture

Thanks. Please replace is_boolean() with is_bool().

Liam Morland’s picture

This is #175 but with is_bool().

@JGReidy: Please give it a try and post your results. Thanks.

spjsche’s picture

I am currently using Views PHP 7.x-1.0-alpha3 with PHP 7.1.32 with no issues on production.

I have updated my DEV system to PHP 7.3.16 and I am now getting the dreaded ->

"Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (line 202 of modules\views_php\plugins\views\views_php_handler_field.inc)."

My query is will the associated patch mentioned be committed once testing is finalised, or will it stay as a patch.

I will implement the patch on our Dev environment to test it, and will comment accordingly.

Thanking you in anticipation...

kevster’s picture

#180 worked for me thank you after getting

Error: Call to undefined function views_php_create_function() in views_php_handler_sort->php_post_execute() (line 73 views_php/plugins/views/views_php_handler_sort.inc).

whthat’s picture

#180 Looks great here, moving from PHP 7.0.27 to 7.2.11

benjamin_dk’s picture

With the patch in #180 applied to version = "7.x-1.0-alpha3" my filters no longer work. Even though TRUE is returned the row is not removed.

PHP 7.2.18

Liam Morland’s picture

Can you identify which of the changes from the patch is the problem?

benjamin_dk’s picture

The whole function creation logic is over my head. I would guess the post_execute part, as this is the one I am using.

diff --git a/plugins/views/views_php_handler_filter.inc b/plugins/views/views_php_handler_filter.inc
index 79bbee2..797d92b 100644
--- a/plugins/views/views_php_handler_filter.inc
+++ b/plugins/views/views_php_handler_filter.inc

@@ -79,7 +82,10 @@ class views_php_handler_filter extends views_handler_filter {
   function php_post_execute() {
     // Evaluate the PHP code.
     if (!empty($this->options['php_filter'])) {
-      $function = create_function('$view, $handler, &$static, $row, $data', $this->options['php_filter'] . ';');
+      $code = $this->options['php_filter'] . ';';
+      $function = function($view, $handler, &$static, $row, $data) use ($code) {
+        eval($code);
+      };
       ob_start();
       foreach ($this->view->result as $i => $row) {
         $normalized_row = new stdClass;
BasH’s picture

I am currently testing Views PHP 7.x-1.0-alpha3 with PHP 7.4.7, my issue before applying the patch is:

Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (regel 202 van C:\..\drupal\htdocs\sites\all\modules\views_php\plugins\views\views_php_handler_field.inc).

my issue after applying the patch is:

ParseError: syntax error, unexpected '?>', expecting function (T_FUNCTION) or const (T_CONST) in _registry_check_code() (regel 219 van C:\..\drupal\htdocs\sites\all\modules\views_php\plugins\views\views_php_handler_field.inc).

Line 219 however is in the pre-render function that is commented out. Clearing the cache before and after this patch doesn't change the issue, after clearing the "views" cache the site doesn't work at all anymore (every page only showing "something went wrong, try again later" and the syntax-error in the watchdog-table)

Sorry, I had to completely remove the pre-render function instead of comment it out, the patch in #180 works for me also now.

c_archer’s picture

I can confirm that the patch in #180 worked for me against version 7.x-1.0-alpha3

BasH’s picture

I was having the same problem as @benjamin_dk in #186 : the php code in the filter was correct but TRUE was never (or only the first time?) returned. My solution was to add a return statement before the eval statement in the post_execute function:

  function php_post_execute() {
     // Evaluate the PHP code.
     if (!empty($this->options['php_filter'])) {
-      $function = create_function('$view, $handler, &$static, $row, $data', $this->options['php_filter'] . ';');
+      $code = $this->options['php_filter'] . ';';
+      $function = function($view, $handler, &$static, $row, $data) use ($code) {
+        return eval($code);
+      };
       ob_start();

after this change my filter worked again. Really don't know when to use the return statement or not in the other proposed changes, but so far in the other cases I didn't find any problems (yet), I will continue to test my views.

kwfinken’s picture

Issue tags: +PHP 7.4
lazzyvn’s picture

i have warning in php 7.4

Deprecated function: Array and string offset access syntax with curly braces is deprecated In include_once() (line 3493 of /Volumes/data/www/drupal/includes/bootstrap.inc).

I debug it comes from
sites/all/modules/contrib/views_php/plugins/views/views_php_handler_field.inc

so

foreach ($this->view->style_plugin->rendered_fields{$this->view->row_index} as $field => $value) {
          $normalized_row->$field = $value;
        }

it must be

foreach ($this->view->style_plugin->rendered_fields[$this->view->row_index] as $field => $value) {
          $normalized_row->$field = $value;
        }

please add it in your patch

Liam Morland’s picture

@#191 This should be a separate ticket.

Berdir’s picture

Adding the missing return to the filter plugin as suggested by others.

joseph.olstad’s picture

Thanks @Berdir

we could probably put this latest patch into dev

John Pitcairn’s picture

Status: Needs review » Reviewed & tested by the community

Thanks. Patch also applies to 7.x-1.0-alpha3 and fixes the deprecation warning.

kwfinken’s picture

It appears that there are now 8 patches RTBC, but there has been no update of the module for almost 5 years. Is there any active maintainer of the module? Is it a abandoned? Does anyone want to claim ownership (see https://www.drupal.org/node/251466 for details)?

Patch Last update on patch
[7.x-1.x] Remove usage of deprecated create_function() 2 weeks 1 day
Function create_function() is deprecated in Drupal\views_php\Plugin\views\field\ViewsPhp->render() (line 122 of modules/contrib/views_php/src/Plugin/views/field/ViewsPhp.php 3 months 3 weeks
Global php does not render available variable in drupal 8.2 versions 4 months 3 weeks
Missing argument 1 4 months 3 weeks
Coding standard changes 1 year 4 days
Pager fix causes performance and memory regression 1 year 6 days
Update Dependencies to new Format in .info.yml 1 year 11 months
Remove depreciated methods in code base. 2 years 8 months

Latest module release: 13 November 2015

I really don't have time to go through all 8 patches and make sure they work with each other, but perhaps someone else does?

Liam Morland’s picture

Besides this issue, there is only one other RTBC patch for 7.x-1.x: #2628014: Pager fix causes performance and memory regression. All other RTBC patches are for 8.x-1.x.

Anybody’s picture

Confirming RTBC for #193. Looking forward to a new release with that fix.

yookoala’s picture

Any news on this change?

Liam Morland’s picture

There is no active maintainer, so there is no one to make the commit.

kwfinken’s picture

Is someone willing to take ownership? If not, I will try to squeeze it in.

rar’s picture

Patch #193 worked for me with PHP-7.4.3

git clone https://git.drupalcode.org/project/views_php.git

wget https://www.drupal.org/files/issues/2020-08-24/views_php-remove_create_function-2274543-193-D7.patch

cd views_php

git apply ../views_php-remove_create_function-2274543-193-D7.patch
joseph.olstad’s picture

193 seems like the right patch

if someone generously becomes maintainer here, please put 193 in and also would be nice to also get the RTBC patch in #2628014: Pager fix causes performance and memory regression

Anybody’s picture

@kwfinken I guess you will have to try to squeeze this in. ;) Also see #203.
Thanks a lot in advance, I already did the same for many other abandoned projects, so I'm out this time.

SilviuChingaru’s picture

Liam Morland’s picture

I created an issue fork and merge request with the patch in #193.

noopal’s picture

Can someone upload the 7.x-1.x version with the patch applied?

Thanks

yookoala’s picture

Is there any way to adopt this orphan module? There are still many people using this module, and it needs to be maintained.

DamienMcKenna’s picture

The standard process still applies: https://www.drupal.org/node/251466

Liam Morland’s picture

Status: Reviewed & tested by the community » Fixed
DamienMcKenna’s picture

Thanks Liam!

MustangGB’s picture

Why only Liam Morland and Berdir given credit?

Liam Morland’s picture

Sorry, adding credit.

MustangGB’s picture

<3

Also thanks for taking this on.

noopal’s picture

Thank you for committing this.

After years I now no longer get that error in my logs.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.