Is using eval() ok in this scenario

1 Views

I am trying to include user input includes using “get_template_part()” The includes are added in the theme options eg a user inputs. 

get_template_part('content/block', 'branding');
get_template_part('content/block', 'footer');

I need to execute this code as php and have a custom hook for add_before_content() which runs just before content and sidebars.

Read More

In my layouts class, I have a method that checks the user input and then uses this to add the includes using eval() 

function add_before_content() {
    global $options;
    $string = "get_template_part('content/block', 'branding');";
    return eval($string);
}

add_action('before_content', 'add_before_content');

This works perfectly, however I get nervous when using eval() especially when it involves user entered content.

My thinking in this scenario is that it should be avoided because it allows users to enter any php code and it will be executed.

However, the only access is from the admin login.

I have actually since implemented a different solution, but wanted to put this question out there as it will be good to get some feedback.

Would eval() be a bad idea in this scenario?

Further clarification

I am trying to achieve the same result as opening the index.php file of the theme and adding get_template_part() to include a template file. For example:

<?php get_template_part('content/block', 'header'); ?>

But allowing the user to do this from the theme options without having to open the themes files.

The contents of the file are not really important, its just a normal php include that should be included and run when called.

In my theme options I have an editor element (extended textarea) I use for a few features so code can be added into the theme from the theme options without the user needing to open the theme files.

This works perfectly fine for html that I have added so far. The value is stored in the database and when its echoed in the theme, it echoes the html, no problem.

However, when I wanted to allow the user to add the includes which is php from the theme editor its where I got stuck.

There are 2 places the user would add these includes. Above and Below the content which there are also hooks for which is why I use them.

Here is a screenshot of the editor with the includes defined.

http://cl.ly/3n3r3H381g1U1a2t3T3g

What I need is for the files branding and header to be included, just as if I had opened the index.php file and entered the get_template_part includes in the file manually, instead of the editor.

Just echoing the value using the way you explained outputs it on the page as a string like this

http://cl.ly/3I251I0B282i0c0E0U2j

Instead of actually including those template files.

Using eval() means that the saved value of the editor which is a string, would be evaluated as php and it adds the includes just as if I added it in the index.php, hence why it works and is the only way I could figure it out.

I hope this makes things a lot clearer.

Thanks for your patience 🙂

The concept

This particular feature I am talking about here is the only place I was doing something quite like this where I am trying to add php and have since gone a different route, but posted this question about eval() as I wanted to confirm my thinking.

This is a parent theme and when first installed only has a content area and sidebars… nothing above or below. Like a “clean slate” to start from when building a site. The theme has something called a “block builder” which allows users to define and add/remove “building blocks” to their website and then define the content to show in these building blocks. eg: they can add a block called “branding”

A building block is just like any “section” of content on a website. A logo area, a navigation area, footer shelf, toolbar etc.

The building blocks concept is taken from the Joomla template framework we developed which has building blocks at its core.

www.getmorph.org

Once a block is defined, it can be set to output in one of 4 ways.

  1. Widgets (if set to widgets a new widget pos with the block name is created)
  2. Featured posts (can featured any posts on the post page to the block or pull from set categories and display as a grid, carousel, slider etc)
  3. Custom html (they can enter any html and some [tags] for popular elements like [logo] [sitename] etc that come with the theme. Yes this can be done using widgets as well, so they can set widgets as the output and put html in the widget.. its the same result, this is just a little easier but open to user preference.
  4. Disable block.. turns it off and unpublishes it.

The block creation is fully automated, when a new block is added, a new include with all the relevant options is created and a theme options page is also created on the fly using ajax.

Obviously this will not work without actually calling the block in the theme somewhere… this is the only manual step and where the whole point of my original question comes to light.

It is easy for any user to open the index.php or better create a child theme and add an index.php and add the code to call the block

<?php get_template_part('content/block', 'header'); ?>

This is even how it is explained in the documentation and a code snippet is even generated with the block name, they just click to copy it. Instructions are given on how to add the block.. this is the only thing they need to do. Very simple indeed.

However, while doing alpha testing with a closed team of users, a few asked if it would be possible to not have to open the theme files to add the block include… hence my question.

Note: the building blocks are optional to use.. users don’t have to use them, they can just use the theme like any other theme, but for many users this will make building and modding their website a lot!! easier.

It means instead of defining 70 widget positions just in case a user needs it. Our approach is scalable, it starts off with only the sidebar widget position.

As a theme is developed using slate as a base, unlimited widget positions can be added as needed by adding the required blocks.

Post Views: 1

Related posts

Leave a Reply

3 comments

  1. I see what you’re trying to do … and I don’t like it at all. You’re including way too much in terms of options for the theme, particularly if you’re allowing freeform PHP code on an options screen.

    Why this is a bad idea

    The mantra of the entire WordPress project is “decisions, not options.” Fact is, the more options you have for a theme, the more confusing it can be to manage things. If you really need to have that much customization available, build a parent theme and have people build custom child themes on top of that.

    Think Genesis or Thesis. Very powerful themes, with a wide variety of design options available via customized child themes.

    If you insist, here’s what you can do instead

    You say you want to give users the option to queue up display blocks in the site without editing the theme files directly:

    In my theme options I have an editor element (extended textarea) I use for a few features so code can be added into the theme from the theme options without the user needing to open the theme files.

    This sounds exactly like another system that already exists within WordPress: widgets.

    A widget is just a pre-defined content template that you can drag-and-drop anywhere on a widgetized surface within WordPress. Some people have just one sidebar that can house widgets. Others have several widgetized regions in their themes (I’ve seen at least one with 70 different widget-ready locations!).

    It seems like you already have your content blocks defined (the template parts). I recommend you do one of two things:

    1. Have the users edit the files directly. You’re already having them write PHP on your options panel. It’s not a huge step for them to edit the PHP directly.
    2. Widgetize your theme. These don’t have to be typical WordPress widgets … but your custom template-parts are predefined blocks of content similar to the way widgets are. Don’t reinvent the wheel. If you want people to position them in different locations, give them an interface to do so.

    jQuery makes it really easy to drag and drop items from one region to another. Then you can save the sequencing in the options table and call things directly … without using eval().

  2. I don’t see any reason whatsoever to use eval() in this circumstance.

    I need to execute this code as php and have a custom hook for add_before_content() which runs just before content and sidebars.

    So, you’ve provided the user a text input, or textarea, and need to output that content in the template. And you want to do so at a custom hook. All good so far. Here’s where you go awry:

    In my layouts class, I have a method that checks the user input and then uses this to add the includes using eval()

    function add_before_content() {
    global $options;
    $string = "get_template_part('content/block', 'branding');";
    return eval($string);
}

    add_action(‘before_content’, ‘add_before_content’);

    What’s the point of this?

    First: why would you be writing user input to a PHP file, instead of storing it in the database, as would be appropriate?

    Second: You’re already inside of a custom hook callback; why not simply output the user input directly from your callback?

    Let’s say you’ve called this option content_branding. Try something like this:

    function add_before_content() {
    global $options;
    // Sanitize, for safety
    $content_branding = wp_kses_post( $options['content_branding'] );
    // Echo (or return) the result
    echo $content_branding;
}

add_action('before_content', 'add_before_content');

    What am I missing?

    EDIT

    The reason I say the contents are not important is because it can be anything and they all differ. Some are just outputting a widget positions, others featured posts. But the same scenario would be true if the file just had Whatever the php is inside the included file is, it should just run as if its included normally.

    Based on this clarification, I am going to reiterate (and up-vote) Eric Mann’s suggestion that, rather than use your current approach, you instead use dynamic sidebars, and custom Widgets.

  3. You can either place entire code into an array as below:

    $safe= array(

    'get_template_part('content/block', 'branding');',
    'get_template_part('content/block', 'footer');'
);

    And then introduce the below check into your function to make sure that no kind of code other than what you already listed can be sent to eval:

    function add_before_content() {
    global $options;
    global $safe;
    $string = (this comes from user)
    if(in_array($string,$safe))
    {
        return eval($string);
    }
}

add_action('before_content', 'add_before_content');

    Or, just modify the function and check against different types of template that can be included:

    $safe= array(

    'branding',
    'footer',
);

function add_before_content() {
    global $options;
    global $safe;
    $string = (this comes from user)
    if(in_array($string,$safe))
    {
        return eval("get_template_part('content/block', '".$string."');");
    }
}

add_action('before_content', 'add_before_content');

    In either way, there will be no possibility for the user to be able to send eval() anything other than what you allowed, so it will be safe.

    I prefer the latter one since its cleaner.