[wp-trac] [WordPress Trac] #60652: font_dir filter enters an infinite loop if wp_get_upload_dir() is used in the filter callback

[WordPress Trac]

#60652: font_dir filter enters an infinite loop if wp_get_upload_dir() is used in
the filter callback
-------------------------------------------------+-------------------------
 Reporter:  mmaattiiaass                         |       Owner:
                                                 |  peterwilsoncc
     Type:  defect (bug)                         |      Status:  closed
 Priority:  normal                               |   Milestone:  6.5.1
Component:  General                              |     Version:  trunk
 Severity:  normal                               |  Resolution:  fixed
 Keywords:  has-unit-tests has-patch fixed-      |     Focuses:
  major dev-reviewed                             |
-------------------------------------------------+-------------------------

Comment (by peterwilsoncc):

 Replying to [comment:45 azaozz]:
 > Sorry but seems I'm missing something.
 >
 > The current docblock encourages plugin authors to use a WordPress
 private function. This is wrong in principle and should never happen imho.
 >
 > In addition it seems that this private function is part of an
 implementation that is not the best. It was added to WP with the excuse
 that there is not enough time to improve the code, and an improvement
 would most likely be made in WP 6.6.

 I'm surprised I need to but I will explain how the Fonts PHP API works:

 Nothing substantial has changed since r57619 was committed on the day of
 Beta 1.

 In order to upload or sideload files, it's required to add a filter to the
 `upload_dir` filter. Without applying the filter files will upload to the
 default `uploads/yyyy/mm` directory.

 I agree that it would probably be better if an upload context was added to
 the upload handlers and related functions but a ticket for the enhancement
 was not opened during the development of the Fonts API so by the time it
 was committed to beta 1 it was too late to do so.


 > > the only way extenders can upload or sideload to the font directory
 >
 > You mean the existing `wp_font_dir()` and the `'font_dir'` filter are
 inadequate and the extenders shouldn't or couldn't use them? Then why was
 this committed in the first place?

 The Fonts API can be extended by Core developers, theme or plugin
 developers. As the API will be primarily maintained by contributors to
 wordpress-develop, it's important to note how it works.

 `wp_font_dir()` is for getting the upload directory. It can be for
 deleting files while removing font-face post types.

 The API committed on the day of beta 1 attempted to use it during
 filtering of uploads too but that introduced an infinite loop bug. A
 suggestion was made in Slack on how to fix this but the suggestion was
 ignored and r57740 used a workaround introducing a maintenance burden for
 future contributors.

 The only way to avoid the infinite loop was to use a separate function for
 filtering the directory during uploads.

 The pull request was open for several weeks and reviewed by multiple core
 contributors. Once I had followed up their suggestions I pinged them again
 to remind them the ticket needed a follow up review so it could be
 committed.

 ----

 Documentation is both for Core contributors and extenders. I think the
 documentation had been made inferior in the patch that has been committed
 and it would have been better practice to wait for further comment before
 committing it.

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