WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 5 months ago

#18563 new defect (bug)

Disallowing editing of .php files in the plugin editor blocks access to allowable extensions

Reported by: trepmal Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 2.8
Component: Plugins Keywords: needs-patch
Focuses: administration Cc:

Description

By using the editable_extensions filter, a user can disallow the editing of php files (more accurately: files with a php extension...)

add_filter( 'editable_extensions', 'disallow_php_file_editing' );
function disallow_php_file_editing( $editable_extensions ) {
	unset( $editable_extensions[0] );	
	return $editable_extensions;
}

However, the file selected when first clicking on the Editor link in the menu is always a php file, so the user is given the "Files of this type are not editable" wp_die() message without being presented with a chance to select a file with a different/allowable extension.

Attachments (2)

18563.patch (3.5 KB) - added by ocean90 3 years ago.
18563.diff (3.6 KB) - added by trepmal 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 ocean903 years ago

You mean the plugin editor? Yes, it always shows the main file first. The theme editor should show the style.css first.

comment:2 trepmal3 years ago

Yes - the plugin editor. Sorry, I should have been clearer.

If 'php' is removed from the list of allowable extensions, you can't use the plugin editor to edit files with the allowed extensions unless you know the URL (e.g. /wp-admin/plugin-editor.php?file=akismet%2Freadme.txt&plugin=akismet%2Fakismet.php)

comment:3 trepmal3 years ago

  • Summary changed from Disallowing editing of .php plugin files blocks access to allowable extensions to Disallowing editing of .php files in the plugin editor blocks access to allowable extensions

ocean903 years ago

comment:4 ocean903 years ago

  • Keywords has-patch added

18563.patch is an idea to solve the issue. Just check the plugin files a bit earlier.

comment:5 dd323 years ago

wp_die( sprintf( '<p>%s</p>', __( 'This file is not allowed to edit! Double check the name and try again.' ) ) );

many people don't realise this, but wp_die() wraps it in a p anyway. Also, Think you Engrished that string :) - I like the approach though, it seems the filter was designed for Adding types, rather than removing them.

trepmal3 years ago

comment:6 trepmal3 years ago

Fixed nested <p>s and added some handling for plugins that are a single php file (such as Hello Dolly).

If php is not an editable extension and Hello Dolly is selected, it previously gave:

Notice: Undefined offset: 0 in /path/to/wp-admin/plugin-editor.php on line 61
No such file exists! Double check the name and try again.

I changed it so it instead gives an informative error message. Not sure if it would be better to remove plugins with no editable files from the 'select a plugin to edit' list. Would that be worth the additional work?

comment:7 ocean903 years ago

  • Milestone changed from Awaiting Review to 3.3

Trepal, the problem is, that if you use wp_die() here, you can't select another plugin anymore.

comment:8 trepmal3 years ago

Okay, I see. If Hello Dolly is the first plugin in the list, you'd be unable to select an editable plugin.

comment:9 gburtini3 years ago

A reasonable workaround, if we want to do this for a quick release, would be to simply dump the list of available files for that plugin (with links) in to the wp_die call. This would allow users to move between files even if they're blocked.

Of course, the ideal solution would simply change the <textarea> on the edit plugin page to an error message (grayed out text perhaps), leaving the sidebar with the links to the other files in tact.

I think this brings up another minor issue with the plugin editor though -- the list of files is inelegant as can be. Plugins with lots of files are difficult to navigate within it, and files more than one directory deep don't display until you select a file within that directory. There's probably something that could be done to improve this.

comment:10 olleicua3 years ago

  • Cc olleicua@… added
  • Keywords needs-patch added

Should this be pushed to the next release? I'm adding needs patch.

comment:11 nacin3 years ago

  • Keywords has-patch removed
  • Milestone 3.3 deleted

Not a regression, time to punt this.

comment:12 nacin3 years ago

  • Milestone set to Future Release

comment:13 nacin5 months ago

  • Component changed from Administration to Plugins
  • Focuses administration added
Note: See TracTickets for help on using tickets.