Plugin Editor lists all files in plugin - should limit to text only

[jbsil]

The plugin editor lists and allows users to edit files that are not text only, including images. Simplest solution would be to limit to certain file extensions that are always text only.

[jbsil]

Limits file extensions to php, txt, js, css, html, xml, inc and include

[jbsil]

Replying to jbsil:
Yeah... I didn't understand what worksforme meant when I added it. Whoops?

[Denis-de-Bernardy]

Maybe add .text and .htm to the list, and lowercase the extension before checking?

Also, it won't work with a weird plugin name like foo.bar/foo.bar.php

Aside, and for what it's worth... testing reveals that using an anchored regex such as /\.([^{.]+)$/ is much faster than using php's pathinfo().}

[Denis-de-Bernardy]

silly wiki formatting. the regex was:

/\.([^.]+)$/

[jbsil]

Excellent suggestions. Thanks! New patch file reflects the additional extensions, strtolower before check, and regex to find the extension.

[jhodgdon]

Can I make a suggestion? Maybe allow a plugin to modify the list of allowed editable extensions. Something like

$include = apply_filters( 'editable_extensions', array("php", "txt", "js", "css", "html", "xml", "inc", "include"));

Just a thought...

[bingorabbit]

Just a fix for calling files that don't need to be editted using a GET request in the URL

[bingorabbit]

URL Fix Patch

[bingorabbit]

I have added a URL Fix Patch as anyone can access such files in the editors using a GET Request in the URL..

[jbsil]

Includes regex to find extensions, apply_filter call for 'editable_extensions', and check for current file as well as the list

[jbsil]

allow a plugin to modify the list of allowed editable extensions.

Included in 9452.1.patch

I have added a URL Fix Patch as anyone can access such files in the editors using a GET Request in the URL.

Good catch! Your updated code used the first patch shown here, so I changed it to use the regex, the filter, and to exit before the <form> is displayed, with clean html and a moderately useful error message.

[bingorabbit]

Replying to jbsil:

allow a plugin to modify the list of allowed editable extensions.

Included in 9452.1.patch

I have added a URL Fix Patch as anyone can access such files in the editors using a GET Request in the URL.

Good catch! Your updated code used the first patch shown here, so I changed it to use the regex, the filter, and to exit before the <form> is displayed, with clean html and a moderately useful error message.

Excellent, this works greatly here.

[strider72]

If putting in a hook to allow plugins to modify the list, you should probably just allow them to *add* to the list, rather than overwrite it completely.

Something like I did in this patch: ​http://trac.wordpress.org/ticket/8964

[jbsil]

Only allows filter to add to existing list. All other changes also in this patch.

[jbsil]

Replying to strider72:

If putting in a hook to allow plugins to modify the list, you should probably just allow them to *add* to the list, rather than overwrite it completely.

Something like I did in this patch: ​http://trac.wordpress.org/ticket/8964

That's probably safer, yes. I took a different approach to it than the patch you linked to.

[ryan]

(In [10879]) Don't allow editing of binary files. Props jbsil. fixes #9452

[hakre]

Just FYI: generated links still lack of using &amp; instead of &. see #9432 and ​patch (patch is in conflict with [10879])

[bingorabbit]

Error layout CSS Fix

[bingorabbit]

The layout of the error was nasty, the red box expanded to the end of the page, so I added a simple CSS inline fix. I think it may considered reopened till a code developer close.

I really dunnu in such "tiny" situations what should be the action done as I'm a bit new to Trac.

[hakre]

The last attachment should fix the output problems (you're right bingorabbit, that did not work out). I have chosen a wp_die() to display the two new messages. This does not need any css modifications.

If you want an error message box, then I suggest to place it on top by adding a callback instead:

add_action('admin_notices', create_function('', sprintf('print \'<div class="%s"><p>%s</p></div>\';', 'error', $error)));

[Denis-de-Bernardy]

+1 to the latest patch

@hakre: the error class, applied to a div, places the thingy to the top automatically.

[Denis-de-Bernardy]

err... nm that. the point raised by bingorabbit is invalid -- that it extends the full width is actually expected, no?

[bingorabbit]

Replying to Denis-de-Bernardy:

err... nm that. the point raised by bingorabbit is invalid -- that it extends the full width is actually expected, no?

Actually I couldn't get this one, can you please explain?!

[hakre]

@denis: 100%/.error/top: well, not with that editor page.

@bingorabbit: denis assumed that 100% is ok here. but it is not this time because of the wrong placed error message.

I still think what wp_die() is best here. the error message now pops mostyl because of better file sanitization and the new check for editable textfiles. because the action is invalid, wp_die() is perfectly fitting here.

[Denis-de-Bernardy]

oh, in that case, might it be missing a div class=wrap, rather?

[Denis-de-Bernardy]

(I agree the die call is better though, let's get dev-feedback before updating anything.

[ryan]

wp_die() sounds fine it me. It will allow us to remove some if error blocks and cleanup the code.

[Denis-de-Bernardy]

11. patch needs a small refresh to remove the unneeded wrapper on if no error then

[hakre]

direct wp_die() vs. later $error / fix of filter

[hakre]

unneeded wrapper if no error then has been removed, wp_die() direct.

while goind through I saw that you are not able to actually filter the list of extensions, you can only add new ones. this has been simplyfied to filter the actual list.

the i asked myself if it makes sense to use a regex to get the file extension. isn't instrrev or similar faster? but this is backend and one call only so maybe not a need to put the finger on.

[Denis-de-Bernardy]

there is still an unneeded if ( !$error ) call in the default case.

else, tested and good to go imo. codepress makes all of this look really neat. :-)

[hakre]

removing overseen if ( !$error) condition

[hakre]

denis, thanks for having a look. that is now removed, should be fine now.

[ryan]

Patch isn't applying cleanly for me.

[hakre]

Patch fixed for current trunk

[hakre]

had conflicts, could fix it. please try again.

[ryan]

(In [11214]) Die on file error. Cleanup. Props hakre. fixes #9452