[wp-trac] [WordPress Trac] #39875: PDF previews overwrite existing images with the same name.

[WordPress Trac]

#39875: PDF previews overwrite existing images with the same name.
-------------------------------------+--------------------
 Reporter:  gitlost                  |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  4.7.3
Component:  Media                    |     Version:  4.7
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+--------------------

Comment (by gitlost):

 Although I can see an argument for doing the unique file name outside the
 class, and also for adding a PDF marker, I don't see a reason for using a
 random name. What's the benefit over `wp_unique_filename()`?

 An argument against a random name is that it makes linked thumbnails
 (especially relevant when #39618 gets fixed) of the PDF brittle - for
 instance if one had to regenerate thumbnails. Another argument is that it
 loses all association with the original PDF on a file level. Another
 argument is that it's non-semantic. Another argument is that it's a pretty
 violent departure from what's there now and is not very WordPressy, not
 matching practice elsewhere, even in the instance of "wp-admin/includes
 /image-edit.php" from which it's culled, which only uses it as a suffix on
 the file name and has its own reasons and context (eg it knows there will
 be clashes and needs a pregable "unique" marker) for using such a scheme,
 not present here. Another argument is that non-deterministic outcomes make
 writing tests harder.

 An argument for I can see is that it busts caching, which is nice, but I'd
 suggest outside the remit of the ticket.

 My preference (if
 [https://core.trac.wordpress.org/attachment/ticket/39875/39875.patch
 39875.patch] is out) would be something like the following (which uses
 `_pdf` rather than `-pdf` so as not to step on the toes of the
 [https://wordpress.org/plugins/pdf-image-generator/ PDF Image Generator]
 plugin):

 {{{#!php
 $dirname = dirname( $file ) . '/';
 $ext = '.' . pathinfo( $file, PATHINFO_EXTENSION );
 $preview_file = $dirname . wp_unique_filename( $dirname, wp_basename(
 $file, $ext ) . '_pdf.jpg' );
 $uploaded = $editor->save( $preview_file, 'image/jpeg' );
 }}}

 Failing that, if a random string is really wanted, maybe put the
 attachment id in there somewhere...

 (As an aside, I'll upload a PDF which could be used instead of "wordpress-
 gsoc-flyer.pdf" in tests to speed them up a bit. It's from
 [https://brendanzagaeski.appspot.com/0004.html Minimal PDF], modified to
 be US Letter size, and is [https://brendanzagaeski.appspot.com/0001.html
 public domain].)

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