Referencing:

There's quite some discussion about how to use php_eval vs eval and how to get rid of associated errors. However, could someone explain to me why we use php_eval or eval in the first place? I dug through the code and all I saw was php_eval being run on an array that is empty anyway. Besides that, there's usually a way to do something without eval. I like what views PDF could do for me but I don't really want to hand over a site to my client with the php module enabled as it could cause some serious damage if used wrongly.

Thanks in advance.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

killua99’s picture

Priority: Normal » Major

We can keep this post to discuss all about php_eval and eval stuff.

I need to speak with the other maintainers, and find the best solution to remove php_eval and and eval. This have to be done with some alter/hook system more save.

The others two post keep it close. Because that is the original design of the module. Maybe we need to do another version, and removing all this bugs and dangerous code.

killua99’s picture

Status: Active » Needs review
FileSize
185.48 KB
3.37 KB

This is a patch that have to be applied in the git branch 7.x-1.x

NOTE: For user that are not familiarity with git, please read the next. If you know what's git and work with it Jump to the next.

Install git if you haven't done yet. install git

Then follow the next code.

You have to replace the current module with git.

mv views_pdf views_pdf-old
 git clone --branch 7.x-1.x http://git.drupal.org/project/views_pdf.git
cd views_pdf
wget https://drupal.org/files/why_php_eval-2040739-2.patch
git apply why_php_eval-2040739-2.patch


Explanation

Maybe with this image it's more easy to explain how to use this "fast solution" for this feature.

views pdf php_evail & eval.png

As you can see, I add two easy option to use the php_eval or the eval.

When you're using the php_eval you must write the <?php ?>, because this is completely evaluate like a PHP file, away form the current functions and so on.

Kingdutch’s picture

Priority: Major » Normal
Status: Needs review » Needs work

Instead of rushing for a patch could you explain the need for the funtions like I asked in my original question?
What I see at the moment is only to add extra content which could be simple HTML right? In that case a normal textarea that accepts HTML would be the solution and (php_)eval would no longer be needed.

Besides that I don't think this has major priority as there's nothing broken at the moment, it's just bad practice to use eval, mostly there's other options.

killua99’s picture

Some people using it to run code, some people don't know how to make a module to make a preprocess or alter. So they use this php box to evaluate thing (not only HTML) and get the output they want.

I rush to create this patch, because people who use this feature with this patchs is broken.

Hope I explain the needed to keed the (php_)eval in this 1.x version. I want to remove it. But need to find options for those who use this feature.

killua99’s picture

Status: Needs work » Needs review

Can I leave this with needs review? for those who use this feature. (I know this need huge work)

Kingdutch’s picture

Status: Needs review » Needs work

Ok I understand why you'd want to keep the eval in there, I'll leave my personal opinion to the side.

Why would we want to use eval instead of php_eval? I think the original issue was that php_eval needed the php module. I think a better fix for that issue would be to add a help text to the render before/after field and (I hadn't even found those fields yet, hence my question as to where it was needed) that the php module must be enabled. This allows the php_eval code to be wrapped in an

if (module_enabled(php)) 

(Not sure about the exact syntax). This means that the php module is no longer a requirement and the module will behave as expected for users entering php in that text field. If the php module was disabled the before and after input could even be rendered as is (which would mean that people not using the php module but still entering php code would see the plain <?php tags and people who didn't inject php code would see everything working as expected.)

I hope you understand my reasoning and proposed solution.

killua99’s picture

The module require core PHP module active, that issue was fixed put it in the .info file the lines dependencies = php. The default option or setup is with the php_eval so it's required. We don't need evaluate if the module is active or not because is a default behaviors of the module.

For the next version or big changes if we remove the eval to handle stuff or w/e the php module will be removed from the requirements. So no needed to evaluate this.

candelas’s picture

thanks @killua99 i will test this today by sure and report.
very well documented, by the way. if you need help on documentation, i can give a hand :)

candelas’s picture

thanks very much!
i could make the invoice that i needed to end one work with this release.
i could make it work with eval.
thanks a lot for your work and i will be pleased to collaborate on this module.
i think it is very good for invoices saved in the server and many people will use it :)

killua99’s picture

Status: Needs work » Closed (won't fix)

Dear fellow Drupal enthusiasts,

This issue is now lasting for a very long time in the issue queue and was unfortunately never solved. As Drupal is a open source project, everyone is helping on a voluntary basis. That this was not solved is nothing personal and means no harm. But perhaps no one had time to deal with this issue, maybe it is too complex, or the problem was not described comprehensibly.

But this issue is not the only one. There are thousands of issues on Drupal.org that have never been worked on or could not be processed. This means that we are building a wave that is unmanageable and it is a problem for the Drupal project as a whole. Please help us keep the issue queue smaller and more manageable.

Please read again, "Making an issue report" and see if you can improve the issue. Test the problem with the current Core and modules. Maybe the problem doesn't exist anymore, is a duplicate or has even been solved within this issue but never closed.

Help can also be found for it on IRC and in the user groups.

In order to close this issue, I have set this issue to "Closed (won't fix)".

If there is new information, please re-open the issue by changing the status to active.

--
This issue was edited with the help of Issue Helper

killua99’s picture

Status: Closed (won't fix) » Fixed

Ops ... I use an automatic button to close the issue but actually is fixed.

kumkum29’s picture

Hello,

i used this patch (why_php_eval-2040739-2.patch ) on the last dev version of views pdf. I get new errors in views admin page :

    Notice : Undefined offset: 0 dans views_pdf_plugin_style_unformatted->render_grouping_sets() (ligne 48 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_plugin_style_unformatted.inc).
    Notice : Undefined index: bypass_eval_alter dans PdfTemplate->renderRow() (ligne 530 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: eval_alter dans PdfTemplate->renderRow() (ligne 534 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined variable: content dans eval() (ligne 2 dans /srv/data/web/vhosts/www.domain.fr/htdocs/modules/php/php.module(80) : eval()'d code).
    Notice : Undefined index: bypass_eval_alter dans PdfTemplate->renderRow() (ligne 530 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: eval_alter dans PdfTemplate->renderRow() (ligne 534 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: bypass_eval_alter dans PdfTemplate->renderRow() (ligne 530 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: eval_alter dans PdfTemplate->renderRow() (ligne 534 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: bypass_eval_alter dans PdfTemplate->renderRow() (ligne 530 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: eval_alter dans PdfTemplate->renderRow() (ligne 534 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: bypass_eval_alter dans PdfTemplate->renderRow() (ligne 530 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).
    Notice : Undefined index: eval_alter dans PdfTemplate->renderRow() (ligne 534 dans /srv/data/web/vhosts/www.domain.fr/htdocs/sites/all/modules/views_pdf/views_pdf_template.php).

When i generate a view pdf, fields no longer appear...

Thanks.
(i use php 5.4.4)

killua99’s picture

I hope with the next compiling dev it will be fixed. You can pull the last dev branch ?

candelas’s picture

i got today last dev and works perfect for me, without warnings. i had to clear cache. PHP Version 5.3.10

i can use eval in a field with
$content = strtoupper($content);

@kumkum29 do you have php filter module enabled?

candelas’s picture

@kumkum29 can you test only with one field and $content = strtoupper($content); in "PHP Code Before Output" please? maybe the problem comes from other thing.

Status: Fixed » Closed (fixed)

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