[wp-trac] [WordPress Trac] #60835: Fix and improve handling of uploading of font files

[WordPress Trac]

#60835: Fix and improve handling of uploading of font files
----------------------------------------------------+---------------------
 Reporter:  azaozz                                  |       Owner:  (none)
     Type:  defect (bug)                            |      Status:  new
 Priority:  high                                    |   Milestone:  6.6
Component:  Upload                                  |     Version:  6.5
 Severity:  normal                                  |  Resolution:
 Keywords:  needs-testing has-patch has-unit-tests  |     Focuses:
----------------------------------------------------+---------------------

Comment (by azaozz):

 > Again, could you please specify exactly what the bug is

 Sure. I think you're right, there's been so many debates/comments/delays
 about this that it is easy to loose sight of the actual problems. Here's a
 tl;fr:

 - [57868] was committed as a fix for #60652 that was supposed to fix the
 possibility of infinite loop. Instead of fixing the loop the code there
 only hides it from developers.
 - The [https://github.com/WordPress/wordpress-develop/pull/6211 PR
 introducing the changes] in [57868] was reviewed by five developers:
 @swissspidy, @mmaattiiaass, @grantmkin, @youknowriad, and myself. All
 reviewers found the code unsatisfactory, and thought there is a better way
 to do this. I even went ahead and tried to explain how this part of the
 code should work which was very different from the code in that PR. Please
 note: all reviews on the PR are about fixing the infinite loop as
 described in the Trac ticket. There are no reviews of that PR as an API
 change.
 - At the end the PR was accepted as a fix for #60652 with reservations,
 simply because it was very near the WP 6.5 release date to come up with a
 better patch. As part of that acceptance there wass the promise to revisit
 this and fix it.
 - After WP 6.5 was released I followed up on that promise and made the
 changes that fully fix the original #60652 bug (the infinite loop is
 prevented, not just hidden), and also the bug in the Uploads API that was
 introduced in [57868]: possibility of a part of the API to be disabled by
 a plugin.
 - The [https://github.com/WordPress/wordpress-develop/pull/6407 current
 PR] went through a very thorough review by two developers. It's been more
 than two months of questions and comments, and I believe it is a good
 solution for this part of the codebase.

 To answer the question directly: The biggest bug here is that a part of
 the WP Uploads API can be disabled by a plugin. This may have been an
 intentional change that was committed without a review from an unrelated
 Trac ticket. However that improper commit doesn't make it less of a bug.

 As a result from this API change a plugin can remove the `font_dir` filter
 and the `wp-content/uploads/fonts` directory completely (fonts will be
 saved mixed with all other files in the uploads directory). This hinders
 future development in WP and is unacceptable imho. For example, having a
 /fonts directory is needed to implement one of the most requested
 features: be able to drop fonts in the /fonts directory and WP recognizes
 them.

 Another bug is that the solution to #60652 in not-good-enough as found by
 the reviews on #60652. This ticket was started as a fix for that.

 Yet another bug here is that [57868] introduces an anti-pattern to WP, a
 filter is run in the callback to another filter. This causes several
 problems including a possibility for infinite loops and possibility for
 plugins to disable the "nested" filter. It is more of a "code
 architecture" bug but I think it has major drawbacks and such code should
 never exist in WP.

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