Problem/Motivation

Having the PHP module in Drupal Core encourages new users of Drupal to enable and use it. When people use the PHP module, it can result in:

  1. Security holes when used improperly
  2. Instability when upgrading their site because they may be using some PHP code that might change between upgrades
  3. Performance issues as PHP code in nodes cannot be cached properly
  4. Shared systems, like Drupal Gardens, must hide it from their users in order to ensure safe usage of their service

Proposed resolution

  1. Remove the PHP module from Drupal core
  2. Move it to a contributed module at drupal.org/project/php
  3. People who do want to use it at least have both the access and knowledge to install new modules. If they have this knowledge, it is assumed that they know what they're getting themselves into by using it.
  4. Ensure security updates and improvements are maintained on the contributed module

Remaining tasks

  • Patch needs to be reviewed
  • The contributed PHP module needs additional maintainers

User interface changes

  • The PHP module is no longer in the modules list
  • The contributed module needs to be installed in order to have it back in there

Related

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

droplet’s picture

Why?

pillarsdotnet’s picture

Should be in contrib. The namespace is available.

Garrett Albright’s picture

droplet: Because with PHP module in core, people actually use it. And make their sites slow and/or insecure in the process.

There's nothing the PHP module does that can't be done by writing a proper module. All it does is let people cheat and shortcut around doing things the correct - fast and secure - way.

Agreed with pillarsdotnet that it should be a contrib module if anything - with a whole lot of warnings attached to its use.

pillarsdotnet’s picture

Here's a good example.

pillarsdotnet’s picture

FileSize
18.12 KB

Another one.

Status: Needs review » Needs work

The last submitted patch, php.patch, failed testing.

catch’s picture

Possible route towards an upgrade path for those who would not like to install the PHP contrib module: http://drupal.org/project/modulate

Tests fail because we're using PHP module as 'a random module' in lots of tests apparently.

pillarsdotnet’s picture

Another alternative would be to patch the PHP filter to require that the node author/editor be uid==1 or be a member of the 'administrator' role.

pillarsdotnet’s picture

webchick’s picture

Hm. I'm not sure about this. One really nice thing about PHP module being in core is that as of D7, all modules can use it to hinge their "Use PHP to control X aspect of this module" ... Block visibility, dynamic field values, etc. If it's shut off (and forced off through Paranoia module or similar), all of those other potential holes vanish with it.

Very few modules are going to want to add an external php module dependency, however, so we'll be right back to losing the battle with "Administer FOO PHP PERMISSION" again...

catch’s picture

I don't see how a move to contrib affects existing modules all that much, if the PHP stuff is optional, they're using module_exists('php'). If it's not optional, the whole point of the module is putting PHP in textareas and they're specifying dependencies[] = php already - so they don't need to do anything?

Having said that, I'd be fine with a two-step process. Remove the filter from core first - since that's the part that pillarsdotnet demonstrated so amusingly above is not doing anyone any good. Then discuss the general "use PHP for" stuff separately.

coderintherye’s picture

+1.

We removed PHP from core when creating a D6 distribution for our university. The reason was because we wanted to deploy thousands of sites, and give site administrators full control of the site, but without allowing them to execute arbitrary PHP code on the server. How much of a real security benefit this was would be hard to quantify, but it was definitely a big step forward in getting Drupal accepted for wide-scale deployment. It also had the added benefit of making it easier to convince people to code actual modules to solve use cases (since they couldn't just throw some random PHP in a block), and thus we could run coder over their code to help them follow best practices.

catch’s picture

Title: Remove PHP module from core » Remove PHP text filter from core

Alright I'm re-purposing this a bit.

PHP module currently does two things:

1. Provides drupal_eval() and an associated "Use PHP for settings" permission. This is the part that contrib modules are able to re-use.

2. Provides an text filter that lets you embed PHP in nodes, comments, custom blocks etc. this is the part that allows admins to easily give anonymous users permission to execute PHP on their sites, as so beautifully illustrated above.

While I don't think the former has to live in core, neither is it doing a massive amount of harm there.

However the text filter is actively harmful, and if moved to a contrib module would probably only be used by sites that were upgrading and had lots of legacy PHP content already (and couldn't be bothered to properly convert that into custom blocks). So removing that should be a lot less controversial, and fix most of the issues.

That leaves a PHP module that doesn't actually do anything useful by itself, but sometimes it's better to do nothing :)

sun’s picture

Status: Needs work » Needs review
Issue tags: +Framework Initiative
FileSize
14.26 KB

We need to find a maintainer that cares for issues like #486094: Allow to pass local variables to php_eval() -- without resolving issues like this, I somewhat fail to see the actual general purpose usefulness of php_eval() in the first place.

However, we can definitely drop the input filter. Which also solves #826550: PHP module should not create a text format in one shot.

catch’s picture

Issue tags: -Framework Initiative
FileSize
26.41 KB

Two weeks later maybe there's a chance.

chx’s picture

Status: Needs review » Reviewed & tested by the community

If the bot agrees, i agree as well.

sun’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Framework Initiative

For now, I'd limit it to the patch in #14 though - for the reasons @catch outlined in #13.

Status: Needs review » Needs work

The last submitted patch, bad_judgement.patch, failed testing.

catch’s picture

I will 'maintain' this module in contrib if it is removed, if at least one person offers to co-maintain it with me.

In maintaining I will do at least the following:

- review patches for 8.x ports

- pointing people to http://drupal.org/project/modulate so they can transition away from it.

- probably add something to shout at people if they enable it.

msonnabaum’s picture

Sounds like a good idea to me.

I would be willing to co-'maintain' said module with catch.

catch’s picture

Note current proposal is to remove the PHP filter, not drupal_eval() and the permission.

Akaoni’s picture

Subscribing.

droplet’s picture

I'm still don't understand....

Security
why #4 ,#5 are security issue. If someone config Drupal into insecure, does it a problem of Drupal? Even writing code in .modules, it can be insecure code too ??

PHP module move to contrib. does it harm own site ?? One good thing of drupal is some guys monitor modules security issue, why we allow a harmful module on d.org ?

Because I'm one using PHP on block on existing sites, really want to know it.
Who can broken your site ?? users with permission or everyone ??

Performance.
It's not an issue of Drupal. every modules & everything can get it slow down, none of your business. (Only enable the modules won't slow down the site ?)

Best practices ..etc
Why you limit users own style?

Can't it make something to improve the PHP filter ??

"pointing people to http://drupal.org/project/modulate"
If this is the good solution & replacement. Can't it get into Core & combine with PHP filter to do some automatically method to get them done.

Garrett Albright’s picture

droplet, removing the filter from core will slow down people who would use it, and maybe give them time to find a better solution in the form of a contrib module which will do what they're looking for, or creating a custom module properly. Those like you who are in the bad habit of using it will still be able to do so with a contrib module if they so choose to do so, but hopefully this will slow down those new to Drupal.

The question is, when you have a problem to solve that requires writing PHP code, what's the proper way to solve it? Is it to use the PHP filter, or to write a proper module? The answer is always to write a module; no exceptions. However, when we have the filter in core, people see it and think that using it isn't that bad; encouraged, even. Removing the filter will avoid that misconception.

catch’s picture

Status: Needs work » Needs review
FileSize
14.26 KB

Re-uploading #14 so we can start again from there. Several people in irc did not want drupal_eval or the permission reviewed (including Earl for Views), so one step at a time.

The test failures look dodgy to me.

Status: Needs review » Needs work

The last submitted patch, drupal8.php-filter.14.patch, failed testing.

sun’s picture

coderintherye’s picture

The module dependency test fail seems like it could be legit, although the test itself looks bunk or out of date, since forum doesn't really rely on PHP does it or perhaps it does in a round about way.

That said, I'd say we should rewrite the module dependency test to enable some other module which has a dependency, but not a dependency on the PHP module. There are plenty of others that we could use in that test.

Feel free to ignore this reasoning if it seems incorrect. Here's the relevant code for that test

Module dependencies (ModuleDependencyTestCase) [Module]
Dependency chain created. Other system.test 467 ModuleDependencyTestCase->testModuleEnableOrder()

 448   /**
 449    * Tests that module dependencies are enabled in the correct order via the
 450    * UI. Dependencies should be enabled before their dependents.
 451    */
 452   function testModuleEnableOrder() {
 453     module_enable(array('module_test'), FALSE);
 454     $this->resetAll();
 455     $this->assertModules(array('module_test'), TRUE);
 456     variable_set('dependency_test', 'dependency');
 457     // module_test creates a dependency chain: forum depends on poll, which
 458     // depends on php. The correct enable order is, php, poll, forum.
 459     $expected_order = array('php', 'poll', 'forum');
 460 
 461     // Enable the modules through the UI, verifying that the dependency chain
 462     // is correct.
 463     $edit = array();
 464     $edit['modules[Core][forum][enable]'] = 'forum';
 465     $this->drupalPost('admin/modules', $edit, t('Save configuration'));
 466     $this->assertModules(array('forum'), FALSE);
 467     $this->assertText(t('You must enable the Poll, PHP filter modules to install Forum.'), t('Dependency chain created.'));
quicksketch’s picture

I will 'maintain' this module in contrib if it is removed, if at least one person offers to co-maintain it with me.

I'll help maintain it also. I did the same thing by separating out the Webform PHP module from the Webform project. I maintained it for 1.5 years and then marked it abandoned, but immediately someone else came along to continue supporting it. Removing a module from core is of course a much bigger deal, but something as simple as PHP Filter would be a breeze to maintain in contrib. I'm also behind catch's suggestions to only remove the filter. I'm fine with drupal_eval() and the permission staying in core.

AaronMcHale’s picture

Will a contributing PHP module just add a filter to nodes, comments, etc, and core would still contain the ability to run PHP code in blocks etc?

Garrett Albright’s picture

Aaron, no. If this is done, it will not be done halfway. But there's no reason the contrib module version won't be able to continue to allow PHP code in blocks.

AaronMcHale’s picture

Garret - thanks for that.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
18.04 KB

Well, just to clarify, the current consensus in this issue (and implemented by the patch in #25) would remove the ability to have PHP code in the body of a block, but it does not remove the feature where you can use PHP to set block visibility; that would still remain in core.

Here's a reroll to apply to the latest HEAD, which also:

  • Fixes the broken tests from above. (Re #28, the test there was only failing because it was looking for the string "You must enable the Poll, PHP filter modules to install Forum" but the patch renames the module from "PHP filter" to "PHP". The test itself is OK and not out-of-date; although perhaps for cleanliness it should use its own modules rather than having the test rely on altering the core dependencies, but that's a separate issue.)
  • Fixes up some problems with the module's help text (e.g., outdated references to "PHP filter") and also changes the help text to mention block visibility settings as a specific example where PHP is used, to keep the module's purpose from being too vague and cryptic :)
  • Removed the php.test file entirely (rather than leaving it as an empty file with only a header and no actual tests in it).
  • Removed a couple of other references to the PHP filter in the codebase.

The one remaining reference I found which I did not remove in this patch (and which may need a little discussion) is the reference to the PHP filter in the text_summary() function. For example, this code:

  // We check for the presence of the PHP evaluator filter in the current
  // format. If the body contains PHP code, we do not split it up to prevent
  // parse errors.
  if (isset($format)) {
    $filters = filter_list_format($format);
    if (isset($filters['php_code']) && $filters['php_code']->status && strpos($text, '<?') !== FALSE) {
      return $text;
    }
  }

It would be great to remove it and just leave it up to the new PHP filter module (in contrib) to alter that behavior back in, but offhand I'm not sure how that would be possible...

pillarsdotnet’s picture

@#33 by David_Rothstein on November 20, 2011 at 12:05am :

It would be great to remove it and just leave it up to the new PHP filter module (in contrib) to alter that behavior back in, but offhand I'm not sure how that would be possible...

See #221257-169: text_summary() should output valid HTML and Unicode text, and not count markup characters as part of the text length

David_Rothstein’s picture

Hm, so I actually think it's even simpler than that. I looked into this issue a bit, and it seems to me like the check for the PHP filter here serves no purpose whatsoever. That is because check_markup() has already been called (and therefore, any PHP code has already been evaluated) before the text that's being trimmed is ever passed to text_summary().

That means for our purposes we should just be able to go ahead and remove it. (I've also created an issue at #1347920: text_summary() checks for specific filters, but it should be agnostic to filters for the more far-reaching fix, since there are more things we can remove also.)

The attached reroll therefore removes that last reference to $filters['php_code'] from the codebase. It also removes the PHP module from the list of dependencies for one of the tests I fixed above (since the test is no longer using the PHP filter, it no longer requires the module either).

Status: Needs review » Needs work

The last submitted patch, remove-php-filter-1203886-35.patch, failed testing.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
19.24 KB

Oops, I got carried away and deleted a little too much from text_summary(). This should work better.

AaronMcHale’s picture

Have I got this right, or am I reading this from somewhere else, because I remember seeing this somewhere, that this new contributing module would remove the separate PHP Filter created by default when enabling this module?

Thanks
AaronMcHale

sun’s picture

#37: remove-php-filter-1203886-37.patch queued for re-testing.

wizonesolutions’s picture

Since this is actually a core issue now, I thought I'd name-drop PHP filter. It was a probably-good idea that I ran out of time (and arguably interest) to develop further.

David_Rothstein’s picture

#37: remove-php-filter-1203886-37.patch queued for re-testing.

David_Rothstein’s picture

Issue tags: +Security improvements

I think this issue actually counts as a security improvement, because if the PHP module really were to become nothing more than a wrapper around the php_eval() function and the "use PHP for settings" permission (as proposed here), we can consider making it a really hard requirement that no Drupal module is allowed to execute user-provided PHP code without checking for this module being enabled and this permission being granted.

This will make it easier to isolate PHP execution to one part of the codebase, and for people who don't want to allow it on their servers to more easily block it.

I think this is sort of recommended behavior already, but because turning on the PHP module has these other side effects (creation of the text format and filter, etc) some module maintainers don't really want to tell their users to turn it on in order to unlock "import" functionality or other functionality that requires PHP code to be executed.

coderintherye’s picture

Thank you David for summing up one of the reasons why this is an important and needed change. Looking forward to what the test results say here.

RobLoach’s picture

Issue tags: +Needs issue summary update
FileSize
19.26 KB

Attached is a re-roll. Was getting a couple patch warnings. Although I'd opt to removing the module in its entirety and moving it to http://drupal.org/project/php , I'd say this is a good first step. Where in the configuration interface to we ask people to write PHP code? Seems like a major security flaw to me. If Drupal's or PHP's API changes, it could bring down sites.

cosmicdreams’s picture

like how a server's lack of php 5.3 brings down sites? These are deployment issues mostly. I'm all for removing the php filter into a module but I rather like David's take of built in protections against user supplied php in the config.

As for php's use in config, I've mainly seen in present when deciding where a block should be placed or some kind of logic that the module is looking for. A site builder typically gets the option to specify a white list, a black list, or custom php logic.

timmillwood’s picture

People new to drupal seem to find the PHP Filter and use it instead of creating custom modules.

It needs to be in contrib with a big warning saying "Are you really sure you want to use this module? why not just write a little custom module, here's the documentation on that."

corbacho’s picture

A typical use case of a CMS newbie: "I want to add the current year in the footer". He searches in Google and found many answers telling about, you just need this Copyright © echo date('Y');

* In wordpress: Just copy/paste that in footer.php of your theme (inside Wordpress admin panel)
* In Drupal, it requires 3 or 4 steps now (enable module, add block, select input filter, etc). If we remove PHP filter from core, add couple of steps more and 5 minutes?

This was just a silly example. I understand the huge security improvement as a Drupal developer and I'm totally in. But we have to be aware that we are making Drupal even more frustrating for not drupal-savy users

EDIT
@pillarsdonet. (you can't edit page.tpl.php from the admin UI, that's what I meant)

pillarsdotnet’s picture

In Drupal, edit your page template and add the same text.

Still not convinced we need to store PHP code in the database.

pillarsdotnet’s picture

you can't edit page.tpl.php from the admin UI, that's what I meant

So? Use elFinder then.

timmillwood’s picture

Yes this module has it's uses, but does it need be in core? No.

webchick’s picture

I think people grossly underestimate how difficult it is for people new to Drupal when we punt on things with, "Oh, there's a contributed module for that." There are sixteen thousand contributed modules. The chances of finding the one that does what you want when you don't even know what modules are yet are astronomically low. And "Oh, you should really just make a module for your footer text?" Please. :)

I tend to agree that this is basic functionality that I would expect to be included out of the box, and I don't understand why kicking it to contrib is viewed as making anything more secure. It merely makes Drupal core more limited and less useful to various "real world" scenarios that inevitably pop up.

catch’s picture

@webchick, have you seen the attachments in post #4 and #5 of this thread? Putting this functionality in contrib is designed to make it harder to do that.

Footer text is entirely customizable in core at the moment, what we'd be dropping support for is putting arbitrary PHP in there which is considerably more obscure.

And if things like putting the current date in the footer are so important it'd be better handled by something like token support.

webchick’s picture

(Summary of ensuing IRC argument :D)

It only makes it harder to do that because it makes this incredibly useful for newbies functionality impossible to find. Seriously, my first few Drupal sites I couldn't have built without the PHP input format, and I know of several people who eventually found their way to module development through using snippets from the handbook in PHP input format in blocks/nodes. Tokens are nice for things like date, but don't cover numerous other things people might want to put in there.

#4 and #5 simply say to me that we probably need some kind of a security threshold mechanism in core for text formats, similar to our janky permission descriptions but more forceful, that warns the crap out of people if they enable dangerous filters for non-privileged roles. You can pwn a site with Full HTML, too. (Granted, not quite as easily, but enabling Full HTML for anon is way more common than PHP.) Punting this capability to contrib doesn't in any way make that problem go away.

I really strongly feel that if this capability is removed, many new site builders will get completely stuck their first time or two out in building their sites. I'm totally fine with updating the module description to warn people that this is bad juju, but removing it from core altogether I feel would be a huge mistake. We don't babysit people as a general rule; we give them tools they need to solve their problems.

timmillwood’s picture

I agree that 'php filter' is a great tool for noobs. It's the way I learnt Drupal, it's the way I still sometimes test code snippets before putting them into a module, but in my day job I see so many huge enterprise level sites that are using it for large sections of the site. It was only a few days ago I saw a site with nearly 1000 nodes with php in. I understand that the education needs to be there to stop people doing this, but as I am often dealing with legacy sites who are just getting around to resolving these sort of issues, it's a little late for them. I am trying to think about new sites and how we can prevent them from making the same mistake.

corbacho’s picture

Moving the PHP filter to contrib doesn't solve the problem of #4 or #5 (only makes it a little harder). After placing it as a contrib module, I'm sure it will be anyway one of the most popular modules.

People will still install the "PHP filter" contrib module and produce anyway all kind of awful sites from Engineers point of view.. that you can't stop it. People, non-coders, want to put custom code everywhere specially 3rd part plugins and stuff like that. Unstoppable.

What can be wiser is to modify the input filter, with something like this in core: #1228748: New security check: PHP filter available to untrusted roles that checks / forbids dangerous configurations (PHP input in comments) and educate Drupal admins of the dangers of PHP as input filter.

------------
I took a little time to research about other CMS:

Joomla: If you order by "popular extensions" the 5th one is "Jumi", that lets you put PHP, Javascript, etc into your content.

Wordpress: core lets you to put php in the templates files via admin interface, but not in widgets and posts. So you need a plugin. One of the most popular ones is http://wordpress.org/extend/plugins/php-code-widget/

Conclusion: Adding custom code is a really popular thing to do, you have it in core or not

PHP filter module is really useful as block filter, damn bad idea as comments input filter (or node input filter)

Maybe is a Drupal chance to rethink the PHP input filter with extra permissions/ checkings, or "pass the potato" to a contribute module as it is. Both solutions have trade offs.

Jody Lynn’s picture

I second timmillwood in #54. We're making it too easy for people to destroy their sites, and practically putting a seal of approval on it by having it in core. I also am perpetually dealing with enterprise sites completely trashed by php in the nodes.

Using PHP in blocks is not quite as bad to me, though, and the use cases for it from noobs are more understandable. Because nodes are meant to be content though, using it there tends to be a part of much worse hacks. Perhaps one compromise would be to change it from a filter to an option for block content only.

RobLoach’s picture

We could move PHP Text Filter to contrib, and have it consumed by Spark. Distributions allow us to re-think the way Drupal is packaged, and Spark is a perfect opportunity to use it.

If people use the PHP text filter, it means that when they upgrade to the next version of Drupal, their site will go down. If their host upgrades their version of PHP and they're using an obsolete function, their site will give a fatal error and break. It's a huge security flaw, and ends up being extremely detrimental to new users rather than helping them.

mitchell’s picture

> Seriously, my first few Drupal sites I couldn't have built without the PHP input format
Module Builder is worth mentioning here. webchick, do you think you would have been able to get started integrating new code almost as easily if that helper module was recommended in place of php.module? Both methods aim to provide a shortcut in the module development process, but MB strikes me as a solution that reinforces more sustainable use of Drupal.

webchick’s picture

Hey, I wrote that module. ;)

And, no, absolutely not.

PHP input format is a matter of: 1) search for problem you're having + "php", 2) copy/paste snippet, 3) save.

Building a module, even with a wizard, is a matter of understanding what a hook is, understanding what hook(s) you need, understanding what files need to go where in your directory tree, and a whole pile of other Drupal-specific things.

And no, moving this to Spark is not an answer, either. That distribution is focused on content authoring experience, so that doesn't make any sense.

wizonesolutions’s picture

So can we just say in big red letters something like (I haven't checked this for pinpoint accuracy):

WARNING: YOU ARE SUBJECTING YOURSELF TO THE FOLLOWING:

  1. Your site will run a bit slower every time you use PHP like this instead of using a module.
  2. If another editor or developer edits this and messes up, it could break some or all pages on your site until it is fixed.
  3. Fixing problems may require you to edit the database.
  4. Pasting PHP you don't understand here may be dangerous and harm your site.
  5. With a modest investment of time, you could use [insert alternative solution here based on where the PHP is being used] instead and face none of these drawbacks.

At least let them know there may be some foot-shooting going on.

Maaaaaaaan I (almost) wish I had the time to work on PHP handler. The project page brings back fond memories. Co-maintainers welcome :D *ahem*

RobLoach’s picture

Thought I'd tally up the benefits and cons of having PHP module in Drupal Core:

Benefits of shipping PHP module in Drupal Core

1. webchick at #10
One really nice thing about PHP module being in core is that as of D7, all modules can use it to hinge their "Use PHP to control X aspect of this module"
2. corbacho at #47
Typical use case of a CMS newbie
"I want to add the current year in the footer". He searches in Google and found echo date('Y');. In Drupal, it requires 3 or 4 steps now (enable module, add block, select input filter, etc). If we remove PHP filter from core, add couple of steps more and 5 minutes?
3. webchick at #51
I tend to agree that this is basic functionality that I would expect to be included out of the box, and I don't understand why kicking it to contrib is viewed as making anything more secure. It merely makes Drupal core more limited and less useful to various "real world" scenarios that inevitably pop up.
4. corbacho at #55
Joomla
If you order by "popular extensions" the 5th one is "Jumi", that lets you put PHP, Javascript, etc into your content. Wordpress
core lets you to put php in the templates files via admin interface, but not in widgets and posts. So you need a plugin. One of the most popular ones is http://wordpress.org/extend/plugins/php-code-widget/ Conclusion
Adding custom code is a really popular thing to do, you have it in core or not
5. webchick at #59
PHP input format is a matter of
1) search for problem you're having + "php", 2) copy/paste snippet, 3) save.

Cons of shipping PHP module in Drupal Core

1. Garrett Albright at #3
With PHP module in core, people actually use it. And make their sites slow and/or insecure in the process.
2. coderintherye at #12
We removed PHP from core when creating a D6 distribution for our university. The reason was because we wanted to deploy thousands of sites, and give site administrators full control of the site, but without allowing them to execute arbitrary PHP code on the server.
3. timmmillwood at #46
People new to drupal seem to find the PHP Filter and use it instead of creating custom modules. It needs to be in contrib with a big warning saying "Are you really sure you want to use this module? why not just write a little custom module, here's the documentation on that."
4. catch at #52
If things like putting the current date in the footer are so important it'd be better handled by something like token support.
5. Jody Lynn at #56
We're making it too easy for people to destroy their sites, and practically putting a seal of approval on it by having it in core. I also am perpetually dealing with enterprise sites completely trashed by php in the nodes.
6. Rob Loach at #57
If people use the PHP text filter, it means that when they upgrade to the next version of Drupal, their site will go down. If their host upgrades their version of PHP and they're using an obsolete function, their site will give a fatal error and break. It's a huge security flaw, and ends up being extremely detrimental to new users rather than helping them.
7. webchick at #59
PHP input format is a matter of
1) search for problem you're having + "php", 2) copy/paste snippet, 3) save.
RobLoach’s picture

droplet’s picture

@#62 is a big problem but it doesn't work properly ?

http://www.armchairarcade.com/neo/node/1618#comment-26184

EDIT:
im being bad guy and test PHP there, but it prints <?php echo 'abc'; ?> directly without executed

corbacho’s picture

#61 It seems webchick and me are playing the devil's advocate role

Drupal 5 -> Drupal 6 we moved PHP Filter to its own module and to be disabled by default.
If people enables it, it's because they need it, nobody is forcing them.
Placing the module in contrib, only it will rise frustration, but still people will do it.

So...
Could be that the key point here I think is to separate "PHP filter" from to "use PHP for settings" (called "use PHP for block visibility" in D6)

That setting is important, for block visibility and for other contrib modules. For example #870938: Add new permission for controlling imports. merlin wouldn't like to force Views users to enable PHP Input filter for that functionality. That dilemma wouldn't be solve after moving the module "as it is" to contrib space.

Is it an option to split both functionalities and move PHP filter to contrib ?. Also, if we could make that PHP Filter is *always* disabled to use for anonymous role, wouldn't solve #62 ?

RobLoach’s picture

@#62 is a big problem but it doesn't work properly ?

It works on some sites, but I'm going to tell you which ones. The scary thing about PHP access is that once you have it, you can manipulate global $user accordingly (not to mention many other scary things).

This is the result of the PHP module being in core. If something is in Core, then people are encouraged to use it. If the module was shipped via a contributed module, then the people installing it at least already have file access to the system. And I assume that means they'd kind of know what they're getting themselves into.

Placing the module in contrib, only it will rise frustration, but still people will do it.

I think potentially having more than 7,130,000 insecure Drupal sites is somewhat more frustrating :-) . You bring up a good point about where do the two places PHP is used go. I'd consider running any PHP code through the browser a blatantly large security hole. We should move it all out to /project/php (to ease the upgrade process). I'm okay with splitting up the functionality too though.

With work going on in #1591686: Add Twig itself and #1499460: [meta] New theme system, we could potentially have "use Twig for settings". Maybe even a Twig Filter too, which would be infinitely more secure than letting users write PHP code in the browser. That'll have to be discussed once that work progresses though.

catch’s picture

One really nice thing about PHP module being in core is that as of D7, all modules can use it to hinge their "Use PHP to control X aspect of this module"

That's not relevant to this issue at all, we're only talking about removing the filter here, not drupal_eval() or the permission, so it can be struck off the benefits list IMO.

David_Rothstein’s picture

Exactly, and what we're saying here also is that it's even the other way around (a benefit of the change), since as described above maintainers will be much more likely to require this permission if turning on the core PHP module doesn't have other strange side effects such as creation of a new text format.

But if we can't all agree on actually removing the filter from core, maybe we can at least agree on splitting the module in two (and leaving both halves in core as separate modules) and repurpose this issue to do that for starters? That would still have some of the major security benefits we've discussed here. And also, if we ever do agree to remove it from core, having it already split into a separate module would obviously make that process extremely simple as a followup...

catch’s picture

I'd have no objection to a patch that splits the module in two for now.

wizonesolutions’s picture

+1 on splitting in two if nothing else.

RobLoach’s picture

Status: Needs review » Needs work
FileSize
17.28 KB

http://drupal.org/project/php ... The "use PHP for settings" permission is moved to system module. We probably need a note in CHANGELOG.txt too.

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
16.56 KB

http://drupal.org/project/php ... Updated the issue summary.

Status: Needs review » Needs work

The last submitted patch, 1203886.patch, failed testing.

RobLoach’s picture

Looks like the Paranoia module disables PHP module. This still needs a re-roll.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
31.18 KB

Status: Needs review » Needs work
Issue tags: -Security improvements, -Needs issue summary update, -Platform Initiative

The last submitted patch, 1203886.patch, failed testing.

RobLoach’s picture

Status: Needs work » Needs review

#74: 1203886.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Security improvements, +Needs issue summary update, +Platform Initiative

The last submitted patch, 1203886.patch, failed testing.

timmillwood’s picture

I heavily support this, every day I have calls with customer pleading them to disable the PHP filter module listing security and performance impacts.

timmillwood’s picture

Issue summary: View changes

Updated issue summary.

RobLoach’s picture

Issue summary: View changes

dsffds

mgifford’s picture

I hadn't heard of the performance problems with PHP filter.

I definitely agree with it being yanked from Core though. Just too easy to slip into bad practices and have more PHP in the database rather than in a repository.

This patch applies nicely locally.

timmillwood’s picture

Status: Needs work » Needs review

It'd still be great to get this into D8.

On a daily basis I am advising people to rework their sites not to use PHP Filter, and it seems to be assumed it's ok because it's in core.

mgifford’s picture

Someone has to re-roll the patch with updated tests in it unfortunately. Agreed that it would be important to put in to D8 if we can!

RobLoach’s picture

Requires a re-roll... On the plus side, new users can post PHP. On the negative side, new users can post PHP.

worldlinemine’s picture

In general, I believe that the PHP filter supports bad practices and encourages new users to adopt these poor behaviors. This leads to a high frequency of unstable and insecure sites each one of which can lead to a negative perception of the greater project. Easing a new users approach to implementing custom PHP should be achieved via alternative paths and is important, but the use of the PHP Filter to support the ease of introduction to the Drupal project is "penny wise and pound foolish".

Common related issue:

Discussed in greater detail and a mention of poorly written PHP code as breaking cron:

tim.plunkett’s picture

FileSize
31.29 KB

Attempted reroll.

RobLoach’s picture

Status: Needs review » Reviewed & tested by the community

Updated to the latest in the PHP module.

webchick’s picture

Assigned: Unassigned » Dries

This is another one of these.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs work
  1. With this patch applied there are still calls to module_exists('php') and php_eval() left in the codebase, despite the module and function being removed (guess the test coverage is lacking)... There is also Views which calls eval() directly, but that's partially a separate issue.
  2. diff --git a/core/modules/system/system.module b/core/modules/system/system.module
    index cdf7b90..2d21dad 100644
    --- a/core/modules/system/system.module
    +++ b/core/modules/system/system.module
    ....
    +    'use PHP for settings' => array(
    +      'title' => t('Use PHP for settings'),
    +      'restrict access' => TRUE,
    +    ),
    

    Having that in system.module looks wrong on so many levels.

David_Rothstein’s picture

Also not clear where this issue is going:

  1. From #13 onward this issue has only been about removing the PHP text filter from core, not the whole module. The issue title still reflects that.
  2. In #67, #68, and #69 there was even some agreement on merely splitting the module in two for now (without removing anything), since that would be a required first step anyway, and it would also be uncontroversial and have huge security benefits in and of itself.

The latest patches, however, remove the whole module... Why did the issue change gears again?

dawehner’s picture

There is also Views which calls eval() directly, but that's partially a separate issue.

Let's open an issue for that: #1889718: Drop php argument validation and default argument

Dries’s picture

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

I'm not actually convinced we want to remove this module from core. I'd prefer to keep it in core.

David_Rothstein’s picture

Title: Remove PHP text filter from core » Split the PHP text filter out of the main PHP module (and consider removing it from core later)
Status: Closed (won't fix) » Needs work

See #88.......

David_Rothstein’s picture

Title: Split the PHP text filter out of the main PHP module (and consider removing it from core later) » Split the PHP text filter out of the main PHP module (and consider removing the filter from core later)

Rewording the title to be even more explicit...

killes@www.drop.org’s picture

Priority: Normal » Major

I say it dies.

cweagans’s picture

+1. Seriously, code does not belong in the database. If you want to write code, write it in a module. IMO, the optimal solution is this:

git rm -rf core/modules/php

but I'd settle for just removing the php text filter.

mgifford’s picture

It's such a powerful tool. We've inherited so many sites where someone has just threw in PHP willy-nilly into a node. It's like a spoke wrench. In the right hands it will make your wheel true, but for the vast majority your more likely to break a spoke or more.

It would just be nice to make it not so accessible. If folks had to install a module before being able to access the filter they might think twice and just writing the PHP properly in code rather than taking the obvious short cut and adding it to the node/view.

cweagans’s picture

No. The first problem is that people keep calling this a tool. Code in the database is a bug. Period.

This is easily solved by a bit of documentation:

Rather than putting code in your node body, copy this function into a new module (then provide an example hook_node_view implementation). Rather than putting code in your block body, copy this function into a new module (then provide example hook_block_view implementation). This documentation page would take 30 minutes to write, and if that's all that's needed to remove php module from core, I'll volunteer to write it. This problem is easily solved with documentation, and we should not make it easy for code to be put back into the database.

DamienMcKenna’s picture

FileSize
524 bytes

I'd also recommend adding a warning to the description of the PHP module to indicate it is a security hazard.

DamienMcKenna’s picture

Status: Needs work » Needs review
mitchell’s picture

#96: Perhaps a new tool could build on the possibility to write PHP files to appropriately solve the use case of coding within the site builder UI.

cweagans’s picture

Here's how to "to appropriately solve the use case of coding within the site builder UI": don't do it. I will never support including that kind of tool in core, and I will never work on one in contrib. If you want to write code, do it the right way: in a code editor.

cweagans’s picture

Mitchell, also, chx already wrote that: https://drupal.org/project/modulate

RobLoach’s picture

Status: Needs review » Needs work

Likely need an update!

Ran into the PHP module last week. Tried to update a Drupal site, and it resulted in a #whitescreenofdeath. The reason was because a node was using the PHP filter, and calling part of the Twitter module to show recent Tweets. Unfortunately, the Twitter module's API has recently changed, so updating the Twitter module made the PHP filter code fail, and caused a white screen of death. You could blame the Twitter module, but no, it's people thinking using the PHP module is a good idea because it's part of Drupal core.

Let me reiterate... This module sucks and should be avoided. Please don't let it live in Drupal core. We should promote good practices, not terrible ones that cause your site to die.

timmillwood’s picture

+1 RobLoach

cweagans’s picture

+1 RobLoach

Garrett Albright’s picture

The Ubuntu forum attack and downtime post mortem.

Once the attacker gained administrator access in the Forums they were able to add a hook through the administrator control panel. Hooks in vBulletin are arbitrary PHP code which can be made to run on every page load. The attacker installed a hook allowing them to execute arbitrary PHP passed in a query string argument. They used this mechanism to explore the environment and also to upload and install two widely available PHP shell kits. The attacker used these shell kits to upload and run some custom PHP code to dump the ‘user’ table to a file on disk which they then downloaded.

Get PHP filter TF out of core, please.

catch’s picture

Assigned: Unassigned » Dries

Back to Dries. #90 was more than six months ago and doesn't have any justification for marking won't fix.

greg.1.anderson’s picture

Without #94 (or paranoia or modulate modules), anyone with sufficient access to a Drupal site can enable the php module and execute arbitrary php code. Of course, it is very bad if someone happens to obtain or guess an admin password on a site, but arbitrary code execution makes this so much worse.

Not only should php module move to contrib, it should also require Bad judgement, as shown by #102, #5, #4, etc. Let's get this out of core.

Crell’s picture

Just throwing my belated voice in here: Yes, we do want to remove PHP module from core. I want to remove it from contrib, too, but one battle at a time. :-) I'm with Rob and cweagans. PHP code in the database is a bug, not a feature.

php.module--

catch’s picture

@greg.1.anderson that's already implemented to an extent: #786208: Support PHP module.

greg.1.anderson’s picture

@catch well done. Now we only need to decide whether we move php module out of core, or bad judgement in. :)

timmillwood’s picture

Guess it's now up to Dries...

greggles’s picture

I also read the Ubuntuforums post-mortem and thought of this issue.

I also would like to see the module removed from core by leaving the permission.

To say that it needs to be in core to ease the onramp ignores the fact that Drupal's users are comfortable downloading modules.

Attached is a patch that I think gets part way there, though tests need work.

mgifford’s picture

Another option would be to bring in something like the Paranoia module in Core. It took a while for that module to get upgraded to D7.

greggles’s picture

Issue tags: +Security improvements

I assume at least this tag removal was unintentional.

I don't see paranoia module in core as a good answer. It actually removes the "use php for settings" permission which I think needs to be in core (so it can be used by contrib, so that paranoia can block it). It's also a much more extreme solution than the proposal from comment #13 on this issue so if #13 is controversial then paranoia is more controversial.

mgifford’s picture

Issue tags: +Platform Initiative

Yes. No idea why it was gone again when I started editing this reply. Added back in the Platform Initiative too which I had removed (without doing anything intentionally or randomly hitting the keyboard).

Mind you I'm in Rangoon at the moment. The Internet isn't so reliable here.

Dries’s picture

Assigned: Dries » Unassigned

Thought about it more and I think it makes sense to delete this module in its entirely, and offer it as a contributed module (either in full or partially).

greggles’s picture

I'm hopeful you mean "remove everything except the permission" and not "remove it entirely". We're slowly coalescing around contrib modules using this permission to control access to executing php and it would be a shame to lose momentum on that front.

webchick’s picture

The permission could probably just move into system module.

catch’s picture

I have a vague memory of the permission being in the module so that it was easier for paranoia etc. to prevent people ever getting the permission, but I'm not sure if there's any use cases that couldn't be solved if it's just in system module too.

DamienMcKenna’s picture

Devils advocate: what's the purpose of retaining a permission for which core has no use? Is there precedent for this?

greggles’s picture

It doesn't matter much to me whether the permission is in php.module or system.module.

Paranoia blocks enabling php.module and it blocks granting the "use PHP for settings" permission to any roles.

By keeping the permission in php.module, someone could get some of the protections of paranoia just be removing the php.module from their site. If we move it to system that idea goes away. So, I guess I have a slight preference for leaving it in php.module out of respect to people who like deleting php.module from their site.

Regardless of where it is, uid 1 can still bypass it (unless #540008: Remove the special behavior of UID#1 gets in).

greggles’s picture

Issue summary: View changes

fdsa

greggles’s picture

@DamienMcKenna - I've updated the issue summary to give some ideas. If you need more basis search the issue comments for the word "permission."

webchick’s picture

The purpose of the permission being added in D7 is to provide a single "killswitch" for removing PHP access from all users (except UID1 per #121). If this permission isn't provided by core, then every contrib module providing this functionality starts doing separate "Use PHP for Block Visibility", "Use PHP for Views", "Use PHP for Ponies" permissions (as it was in D6) which are much harder to audit.

webchick’s picture

Oops. Cross-posted with an issue summary update. :D Thanks, greggles!

effulgentsia’s picture

I have no objection to retaining the permission in core, either in system.module, or in php.module. However:

If this permission isn't provided by core, then every contrib module providing this functionality starts doing separate "Use PHP for Block Visibility", "Use PHP for Views", "Use PHP for Ponies" permissions (as it was in D6) which are much harder to audit.

Is that the only option? Why can't there be a canonical php.module in contrib that offers the permission and the eval wrapper, and all other contrib modules use that?

webchick’s picture

True enough, I guess. But since one of the arguments against this feature was "if it's in core, people will use it," it seems advantageous to keep such a killswitch in core so that people will use it. :)

DamienMcKenna’s picture

Just to seek clarification again, what is the value of considering something (a permission) a global killswitch when a) user 1 is unaffected, b) most of the dangerous, security-threatening, worst-case-used-of-a-technology-ever happens through user 1 anyway?

ezra-g’s picture

This was acknowledged in comment #121 with the reference to #540008: Remove the special behavior of UID#1.

greggles’s picture

Element b of the hypothetical in #127 hypothetical is false.

Edited to make it clear what hypothetical I was referring to

DamienMcKenna’s picture

Just to mention it, the only times I've seen the PHP module used it was for user 1, hence my reservations.

Resolving #540008: Remove the special behavior of UID#1 would relieve most of my concerns, and I'm sorry for overlooking its gravity when it was first mentioned.

Back to lurking :)

RobLoach’s picture

Status: Needs work » Needs review
FileSize
18.11 KB
  1. Re-rolled on my Github fork
  2. Feel free to submit pull requests from there
  3. Updated the PHP module

Status: Needs review » Needs work

The last submitted patch, drupal 8.x...1203886.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

You're not removing the actual filter (\Drupal\php\Plugin\Filter\Php)

RobLoach’s picture

Status: Needs review » Needs work

The last submitted patch, drupal 8.x...1203886.diff, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
43.65 KB

Used a seperate remote as the mirror wasn't quite up to date:
https://github.com/RobLoach/drupalmirror/compare/1203886

Status: Needs review » Needs work

The last submitted patch, 1203886.diff, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
44.24 KB
  1. Rebased to a new branch
  2. Security test fix: Interdiff
RobLoach’s picture

#138: 1203886-2.diff queued for re-testing.

RobLoach’s picture

Issue summary: View changes

updated to focus on the current proposal as put forth and endorsed by catch, David_Rothstein, etc.

RobLoach’s picture

Title: Split the PHP text filter out of the main PHP module (and consider removing the filter from core later) » Remove the PHP module from Drupal core

Updated the issue summary, and ensured the PHP module is functional.

RobLoach’s picture

Issue summary: View changes

fff

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

RobLoach’s picture

Issue summary: View changes

Updated issue summary.

mparker17’s picture

You updated the issue summary, so we can remove that tag. :)

mparker17’s picture

Status: Needs review » Reviewed & tested by the community

I don't see the PHP module in the list of available modules anymore. I don't see options to write custom PHP code in the block visibility settings. I don't see a PHP input filter option when editing text formats.

The tests pass!

The code looks good!

I'd say this is RTBC!

David_Rothstein’s picture

Title: Remove the PHP module from Drupal core » Remove the PHP module from Drupal core, and split the text filter and Views integration out of the main module

I'm not changing the status from RTBC because I think the module could be removed first and these other issues dealt with later, but are the concerns raised by @greggles, @webchick, and others answered? And I think we still need to make sure the module is split up at some point; simply dumping it in contrib as is won't provide the security benefits.

To clarify that, the only "safe" way for a module to execute user-provided PHP code (in such a way that even user 1 cannot directly bypass it) is to:

  • Check module_exists('php') && user_access('use PHP for settings') before allowing the code to be entered.
  • Check module_exists('php') before allowing the code to be executed.

This is best practice currently, but I think we should aim to enforce it (at the security team level) in Drupal 8. The path to that is:

  1. In Drupal 7, module maintainers have intentionally (and justifiably, I think) resisted the module_exists('php') part of the above, because they don't want to force their users to endure the unwanted side effects (the extra text format, etc) of turning that module on (see #870938: Add new permission for controlling imports for the Ctools issue around this). In Drupal 8, last time I checked it's even worse because some Views code had moved into the module too; the module has gotten bigger when it should be getting smaller. To force module maintainers who want to execute PHP to rely on the PHP module, we have to strip it down and split all the side effects and features out of the main module.
  2. Although the above two code checks aren't that complicated, they are just subtle enough that I think people will manage to get them wrong by accident too. So there could be some benefit to having easy-to-remember API functions as wrappers around the above, something like php_can_be_entered() and php_can_be_executed(). By definition, these API functions would have to live in core. However, I don't think that necessarily means anything else would need to live in core; although it would be unusual for code in Drupal core to check for the existence of a module and permission that live in contrib, it's not completely unprecedented for core code to assume something about contrib modules (at one point I believe there were a few things in core that were specific to the Devel module). I think this is one of the rare cases where it would make sense. (And for extra fun, we could even consider making core use these functions in #932110: On some servers, the Update Manager allows administrators to directly execute arbitrary code even without the PHP module, but that's its whole own discussion.)

Does that sound like a path forward to address the concerns above?

catch’s picture

Assigned: Unassigned » Dries

I'd be OK with adding two helpers like that to core. Assigning this to Dries, I opened this issue in 2011 so it's clear what my view is.

Dries’s picture

Status: Reviewed & tested by the community » Needs work

Per #143, I think we can deal with these issues later and commit this now. Patch needs to be re-rolled though.

webchick’s picture

Issue tags: +Needs followup

Tagging for a follow-up issue in #143.

xjm’s picture

Small suggestion: the patch will reroll more easily and be smaller to review with git config --global diff.renames copies.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
19.37 KB
  1. Rebased and merged the conflict
  2. @xjm There aren't any renames in this diff so that does not affect the size of the patch. You might have meant to suggest -D however, which will only print the header of the files rather than the diff to nothing. I did that in this patch for ya.

Status: Needs review » Needs work

The last submitted patch, 1203886.patch, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
43.49 KB

It appears that results in a diff that applying seems to not enjoy. This is it without -D. For those following at home:
https://github.com/RobLoach/drupalmirror/pull/1
https://github.com/RobLoach/drupalmirror/pull/1.diff

Status: Needs review » Needs work

The last submitted patch, 1203886.patch, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
44.17 KB
ZenDoodles’s picture

Title: Remove the PHP module from Drupal core, and split the text filter and Views integration out of the main module » Remove the PHP module from Drupal core

Just changing the title back. Looks like it got reset accidentally.

(Also... zomg +1)

David_Rothstein’s picture

The title change was on purpose, but it seems like we've agreed to deal with that in followups.

Hopefully the maintainer of https://drupal.org/project/php will agree with a "Remove most of the PHP module from the PHP module" issue, though :)

webchick’s picture

Status: Needs review » Reviewed & tested by the community

Since that looks like it was a straight-up re-roll, moving back to RTBC.

alexpott’s picture

FileSize
44.07 KB

Patch no longer applied. :( core/modules/php/lib/Drupal/php/Tests/PhpFilterTest.php had changed in #2083811: Remove langcode nesting for fields in entity $form structures and this patch deletes it

alexpott’s picture

FileSize
20.05 KB

Actually let me upload the nice to review patch generated with git diff -D 8.x HEAD

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1203886-157.patch, failed testing.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
44.07 KB

Well who know -D is only for reviewing should have read more of the comments :D uploading the patch from #156 again

RobLoach’s picture

Rebased and it resulted in the same was what Alex posted. +1

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Live from NYC. Finally. Let their be rejoicing. :)

jbrown’s picture

Hooray!

RobLoach’s picture

Title: Remove the PHP module from Drupal core » Follow up: Remove the PHP module from Drupal core
Status: Fixed » Needs review
Issue tags: +CHANGELOG.txt, +Needs change record
FileSize
313 bytes

Three things:

  1. Change notification needs review: PHP Filter module removed from Drupal core
  2. Attached patch for CHANGELOG.txt needs a review
  3. How do we proceed with the points brought up in #143?
ZenDoodles’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -CHANGELOG.txt, -Needs change record, -Needs followup

Patch only removes the php filter from CHANGELOG.txt.

Opened follow up issue #2088851: Handle remaining security/permissions issues related to removing the php module from core.

ZenDoodles’s picture

Issue tags: +CHANGELOG.txt

Oops

webchick’s picture

Title: Follow up: Remove the PHP module from Drupal core » Remove the PHP module from Drupal core
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

ami7878’s picture

This is nonsense.

We cannot have this in the core because "Having the PHP module in Drupal Core encourages new users of Drupal to enable and use it ..."

This is like saying lets stop making cars because they encourage new and inexperienced drivers to use it and cause accidents.

Come on, please ...

DamienMcKenna’s picture

@ami7878: I don't know how many Drupal websites you've worked on, but the experiences of probably everyone else in this issue is that the PHP module, specifically the PHP text filter, is used by people who shouldn't have used it in the first place, who leave massive security holes in their sites through their inexperience. IMHO I'd go so far as to say that Drupal never should have had the PHP module in core in the first place, but I'm happy it's finally gone.

Lets be clear about something: solving a problem by using PHP code in a custom block or a node form field is always the wrong way of doing it.

Lets move on.

Garrett Albright’s picture

New drivers must go through training and licensing programs to make sure that they know how to operate a car properly.

This is more like, instead of shipping cars with a stereo system that, when turned on, disable the airbags and anti-lock brakes and turn the car's body into a high-powered magnet, let's stop shipping cars with a stereo system and instead make the user install their own stereo system with those effects if they wish.

I'd go so far as to say that Drupal never should have had the PHP module in core in the first place

Me too. I've felt that way pretty much ever since I discovered this module existed.

danillonunes’s picture

PHP module is more like selling a car with a small fridge in front of the driver's seat filled with 12 cans of beer.

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.

cilefen’s picture

Here are a few words on finding and eliminating content that was using the PHP module.

Garrett Albright’s picture

cliefen's post shows how to find nodes and blocks, but there's still Views fields, Rules or Context selectors, etc… lots of other dark dirty corners where eval() may be lurking.

cilefen’s picture

@Garrett Albright: Good point - I updated and linked back to your comment.

MakeOnlineShop’s picture

I find it very funny that you could think that a newbie will be able to use Drupal...

I am trying Drupal 8 now and find its usability exactly as before if you know what I mean...

It really seem that coders are doing it for themselves, not for users...

Don't worry, nobody but experts will use Drupal 8 !

wizonesolutions’s picture

@MakeOnlineShop This is probably not the best thread to get feedback on your comment. A newbie wouldn't be migrating D7 -> D8 and trying to remove eval()'d PHP.

If you have encountered problems and solved them, update the documentation. If you have problems you have not been able to solve, ask on (not an exhaustive list) Drupal Answers, Twitter #Drupal, or the forums.

Drupal is a community project, so if you want to improve its UX, contribute something back. It doesn't have to be code. It could just be more information about the problems you faced (but not here, not the right thread).

Your comment is too vague to be of any use to anyone who might want to improve Drupal's usability.

liquidcms’s picture

Yes, i guess security issues being solved here.. but i have a quick issue with a client needing to replicate a term field over to a text field so we can finish content import. Sure would be easy being able to create a simple php action and run through VBO.. sad this feature has been dropped.

greggles’s picture

This feature is not dropped. It's still available for Drupal 8 or 9 at https://www.drupal.org/project/php

liquidcms’s picture

a faint glimmer of what was there in D7.. i think PHP module only adds the PHP input filter? No PHP actions, no PHP in custom tokens, etc. but maybe these were all added individually by the contrib modules.