Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#6294 closed defect (bug) (fixed)

Uploading media file with same sanitized filename as existing file will overwrite existing file.

Reported by: markjaquith Owned by: neodude
Milestone: 2.5 Priority: highest omg bbq
Severity: blocker Version: 2.5
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:


  1. Upload "test.png" with dimensions X1xY1
  2. Upload a different file that is also named "test.png" with dimensions X2xY2

The second test.png will overwrite the first, although the first will keep its dimensions of X1xY1, so it will be squished or stretched out.

Blocker, because someone your original file is GONE without any warning.

Desired outcome: test-N.jpg where N is an integer that increments as high as it needs to go to avoid overwriting an existing file.

Attachments (2)

wptests@r174-for-#6294.diff (4.3 KB) - added by neodude 10 years ago.
unit tests for wp-includes/functions.php's wp_unique_filename()
functions.php-r7414.diff (1.4 KB) - added by neodude 10 years ago.
patch for wp_unique_functions, fixed issues described in comment 2

Download all attachments as: .zip

Change History (5)

#1 @neodude
10 years ago

  • Owner changed from anonymous to neodude
  • Status changed from new to assigned

I think I narrowed it down. I'm writing the test cases now, will put that and patch up asap.

10 years ago

unit tests for wp-includes/functions.php's wp_unique_filename()

10 years ago

patch for wp_unique_functions, fixed issues described in comment 2

#2 @neodude
10 years ago

  • Cc neodude added
  • Keywords has-patch needs-testing added

functions.php-r7414.diff fixes:

  • edge case: failed to handle a file named '.ext'
  • sanitize_title_with_dashes called too late, which is, AFAIK, the cause of this bug
  • with s_t_w_d moved up, the # and \ checks are redundant, since s_t_w_d does s/[%a-z0-9 _-] anyway

I tested two different files named test%test.png and test%%test.png, which correctly gives testtest.png and testtest1.png respectively.

The unit tests cover pretty much all of wp_unique_filename, but only vaguely common bits of sanitize_title_with_dashes. The latter function is very complex to write test cases completely for, so I thought I'd save my time.

#3 @markjaquith
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [7440]) Prevent uploads from overwriting each other. props neodude. fixes #6294

Note: See TracTickets for help on using tickets.