Media browser plus attempts to move files into the created media root directory when it is enabled, but it does this by creating a new path using the file_managed "filename" value. This value is not guaranteed to be unique, so this method causes duplicate file URIs to be created; as this is enforced as unique with a DB constraint, enabling the module fails and the filesystem and file_managed table are both left in an inconsistent state.

Flagged as major due to the potential damage to file_managed data and to the files themselves.

CommentFileSizeAuthor
#4 media_browser_plus-filename_handling-1564678-4.patch586 bytesjhnbrnn
#2 media_browser_plus-filename_handling-1564678-2.patch545 bytesjhnbrnn

Comments

elyobo’s picture

elyobo commented

Simply calling file_move() with the destination directory, rather than directory + a file name, avoids the problem.

jhnbrnn’s picture

jhnbrnn commented
Status: Active » Needs review
StatusFileSize
new media_browser_plus-filename_handling-1564678-2.patch545 bytes

Here's a one-line fix as per #1's recommendation.

camdarley’s picture

camdarley commented

If you remove this line, $destination is not declared anymore...
If think it should be better like this:

-   $destination = $path . '/' . $media->filename;
-  file_move($media, $destination, $replace);
+  file_move($media, $path, $replace);

Isn't it?

jhnbrnn’s picture

jhnbrnn commented
StatusFileSize
new media_browser_plus-filename_handling-1564678-4.patch586 bytes

Ah, you're totally right. Re-rolled with changes.

das-peter’s picture

das-peter commented
Status: Needs review » Fixed

Sounds sane :)
Comitted: http://drupalcode.org/project/media_browser_plus.git/commit/938e74a

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.