[wp-trac] [WordPress Trac] #32482: Fix TinyMCE js include

[WordPress Trac]

#32482: Fix TinyMCE js include
-------------------------------------+------------------------------
 Reporter:  yoni y                   |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  Awaiting Review
Component:  TinyMCE                  |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  close reporter-feedback  |     Focuses:
-------------------------------------+------------------------------

Comment (by yoni y):

 Replying to [comment:1 azaozz]:
 > I'm not that sure what exactly is the purpose here. The TinyMCE JS is
 concatenated and pre-compressed when building, then outputted from a small
 PHP file. It checks some basic support and adds the needed headers. This
 is by far the fastest way to load it. Nothing is being generated on the
 fly, etc.
 >
 > > Generating a static file on the file seems like a waste of resources.
 >
 > Do you mean concatenating and pre-compressing all the JS parts of
 TinyMCE at build time is a waste?
 >
 No I mean not doing so is a waste of time. this file serves precompressed
 file if a gzip one is requested, but concats two files if gzip is not
 enabled on the client.
 You might gain a bit of performance from not gzip each time but then why
 just this file ?
 also I'm pretty sure nginx (and other web servers) static file serving
 will have better performance then this little hack (and without the
 security implications)

 > > This script mimic a web server changing include headers and decided
 weather to serve a compressed file or a plain text one. this seems like
 something that should be left to the handling web server.
 >
 > Unfortunately most shared hosting servers do not add cache expiration,
 content type, etc. headers.
 I find it weird that shared hosting wont have proper caching and headers
 since all there users will be much more happy, but the fact that some
 users have crappy providers doesn't mean everyone else has to suffer.

 > > Having a php files in the wp-include that runs directly by the web
 server seems like it might have some security implications.
 >
 > Not sure what you mean by that. We concatenate and load all admin JS and
 CSS this way.
 All the files ? cause this was the only files that have been blocked under
 my wp-includes path.
 If so this is way worse than I thought.
 Its considered best practice for PHP app to have a single point of entry.
 that way managing authentication and authorization can be much more simple
 and handled globally to the entire APP. Wordpress seems to be doing that
 for 99% of the files and then you have some rouge files like this one (and
 few more legacy ones I found in wp-includes) that now need to be managed
 separately. so having multiple points of entry is harder to secure than
 one.
 Other then that it is again best practice when you have PHP files being
 included it is best to prevent them from being accessed directly think
 about this fake logic -
 <?php if ($user->isAdmin()) { require('clearDb.php') } ?>
 what happens if some one will just access the clearDb.php url ? if it is
 available for public access the security check has been overridden.
 This is of course crappy code and you should never write code like that,
 But if you have files that you know should always be included from other
 files why should you allow anyone to try and access them directly ? are
 you sure every included file in wordpress wont mess things up if it runs
 outside its normal context ?
 Just a few weeks ago it was discovered that the “example.html” in an icon
 font package exposes wordpress sites for XSS. did this file needed to be
 included with wordpress ?
 Having unnecessary access can only lead to security issue.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/32482#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform