View Issue Details

IDProjectCategoryView StatusLast Update
0007176mantisbtadministrationpublic2013-05-06 15:01
Reporteratomoid Assigned Todregad  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionduplicate 
Product Version1.0.1 
Summary0007176: Move Attachments to Disk: data loss due to filename overwrite
Description

When using the admin tool "Move attachments to Disk" (script: (the ), any files in the database that have the same filename get overwritten in the destination directory.

Because it is common for bugs to contain attachments of the same filename (as with screenshot files, ie, "picture1.png"), the bugs that have this name for their attachemnt will end up all sharing a common file, which i guess is the most recent one that was written to disk by the script.

Suggestions:

  • The filenames should be hashed in the same way the uploaded ones are.

  • Until this can be fixed, a warning about this problem should appear in the "system_utils.php" page nect to the script description.

  • The hashed filenames should include some resemblance to their original names so they can be identified outside of Mantis (ie., "c368f2e128c883981f150515bc12a38c_picture1.png")

Steps To Reproduce

1) Set up your mantis config to allow the files to go into a directory on disk instead of the db sql. Log at least two bugs that share different files that happen to have the same filenames.

2) set in config_inc.php (in my case):
$g_file_upload_method = DISK;
$g_absolute_path_default_upload_folder = '/home/httpd/mantis/mantis-files/';

3) go to mantis/admin/system_utils.php and click the "Move Attachments to Disk" button (the "move_db2disk.php" script).

Bug >> There should probably be a confirmation dialog before doing something of this magnitude.

Bug >> The script reports errors and fails unless you first go into each of the projects and set the Upload File Path to your "$g_absolute_path_default_upload_folder" setting. For some reason, the script wont use that default setting.

Bug >> Look for the reports you wrote that have the attachemnts with the same filename, they will both share the same file. The other report's unique file attachemnt is lost as a result.

Tagspatch
Attached Files
0001-Fix-overwriting-files-and-setting-file-permissions-0.patch (3,237 bytes)   
From ed9abc6cda3501f19a5624ea37de7abcdd3e7afc Mon Sep 17 00:00:00 2001
From: Rolf Kleef <rolf@drostan.org>
Date: Fri, 11 Jun 2010 19:43:04 +0200
Subject: [PATCH] Fix overwriting files and setting file permissions - 0007176 and others

---
 admin/move_db2disk.php |   29 ++++++++++++++++++++++++++---
 1 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/admin/move_db2disk.php b/admin/move_db2disk.php
index 605ec53..a09a36d 100644
--- a/admin/move_db2disk.php
+++ b/admin/move_db2disk.php
@@ -113,15 +113,36 @@ function upgrade_move_att2disk( $p_source ) {
 		if( $p_source == 'attachment' ) {
 			$t_project_id = bug_get_field( $t_row['bug_id'], 'project_id' );
 			$t_bug_id = $t_row['bug_id'];
+			// taken from file_api.php file_add(): TRUE value of
+			// $t_file_hash = ( 'bug' == $p_table ) ? $t_bug_id : config_get( 'document_files_prefix' ) . '-' . $t_project_id;
+			$t_file_hash = $t_bug_id;
 		} else {
 			$t_project_id = (int) $t_row['project_id'];
 			$t_bug_id = $t_project_id;
+			// taken from file_api.php file_add(): FALSE value of
+			// $t_file_hash = ( 'bug' == $p_table ) ? $t_bug_id : config_get( 'document_files_prefix' ) . '-' . $t_project_id;
+			$t_file_hash = config_get( 'document_files_prefix' ) . '-' . $t_project_id;
 		}
 
+		$t_file_name = $t_row['filename'];
+
+		// create unique name: taken from file_api.php file_add():
 		$t_file_path = project_get_field( $t_project_id, 'file_path' );
+		if( $t_file_path == '' ) {
+			$t_file_path = config_get( 'absolute_path_default_upload_folder' );
+		}
+
+		$t_unique_name = file_generate_unique_name( $t_file_hash . '-' . $t_file_name, $t_file_path );
+		// $t_disk_file_name seems not needed?
+		//$t_disk_file_name = $t_file_path . $t_unique_name;
+		$c_unique_name = db_prepare_string( $t_unique_name );
+		// end part from file_add()
+
+		// file_add() seems to not use this way to get the absolute prefix?
 		$prefix = get_prefix( $t_file_path );
 		$t_real_file_path = $prefix . $t_file_path;
-		$c_filename = file_clean_name( $t_row['filename'] );
+		// changed $c_filename into $c_unique_name in rest of function
+		// $c_filename = file_clean_name( $t_row['filename'] );
 
 		printf( "\n<tr %s><td>%8d</td><td>%s</td><td>", helper_alternate_class(), $t_bug_id, $t_row['filename'] );
 
@@ -129,13 +150,15 @@ function upgrade_move_att2disk( $p_source ) {
 			echo 'Destination ' . $t_real_file_path . ' not writable';
 			$t_failures++;
 		} else {
-			$t_file_name = $t_real_file_path . $c_filename;
+			$t_file_name = $t_real_file_path . $c_unique_name;
 
 			// write file to disk store after adjusting the path
 			if( file_put_contents( $t_file_name, $t_row['content'] ) ) {
+				chmod( $t_file_name, config_get( 'attachments_file_permissions' ) );
+
 				// successful, update database
 				/** @todo do we want to check the size of data transfer matches here? */
-				$c_new_file_name = $t_file_path . $c_filename;
+				$c_new_file_name = $t_file_path . $c_unique_name;
 				$query2 = "UPDATE $t_file_table SET diskfile = " . db_param() . ",
 						folder = " . db_param() . ", content = '' WHERE id = " . db_param();
 				$update = @db_query_bound( $query2, Array( $c_new_file_name, $t_file_path, $t_row['id'] ) );
-- 
1.7.0.4

Relationships

duplicate of 0015572 closeddregad diskfile_is_name_unique() can return non-unique filename 
related to 0015496 closeddregad Script to move attachments from db to disk not working 
has duplicate 0005836 closedrolfkleef move_db2disk should prefix filename with bug id to prevent filename collisions 
has duplicate 0009495 closedrolfkleef Attachment Tool overwrites files with the same name 
has duplicate 0011591 closedrolfkleef admin/move_db2disk.php: data loss due to filename overwrite, $g_attachments_file_permissions not used 

Activities

vboctor

vboctor

2006-09-29 02:03

manager   ~0013546

This tool should create hashes for file names the same way new attachments are handled. It is important for the hashes not to include file extensions, otherwise they can be executed by the server.

If the hashes are to include the file name, then the dots are to be replaced by underscores. Although I don't see a big advantage for that.

emm

emm

2006-10-20 10:07

reporter   ~0013631

Hi,
and sorry if I am saying something stupid, but why couldn't we had the bug_id as the name for a folder in the path?
In the previous example it would be:
'/home/httpd/mantis/mantis-files/7176/picture1.png';
So that files with same name couldn't be overwrited.
(P.S. sorry for my bad english)

emm

emm

2007-09-17 05:38

reporter   ~0015648

Hi,
is there a way to move all files from database to disk but from the database view? because the database seems to be so huge that the execution of the script always stops when executed from the mantis view.
although I've put in php.ini a large memory limit.
or maybe I need to increase parameters in MySQL configuration.

vboctor

vboctor

2007-09-17 10:08

manager   ~0015650

Reminder sent to: thraxisp

Looping in thraxisp since he implemented this feature.

omeslo

omeslo

2007-12-17 05:30

reporter   ~0016454

I extracted 5000+ attachments from the db to disk using the following workaround:

in move_db2disk.php around line 15, insert the line

set_time_limit ( 1200 ); // this is the max execution time of the script in seconds (20 minutes was enough for me)

and replace @ line 103
$c_filename = file_clean_name($v_filename);
with
$c_filename = md5(file_clean_name($v_filename) . $v_date_added . $v_bug_id);

this will generate an md5 hash of the concatenated filename, the date added and the bug-id, thus making the filename fairly unique.

swellnuts

swellnuts

2008-12-06 05:30

reporter   ~0020260

Works for me.
I had to fiddle with path names a bit.
Make a backup of your mantis_bug_file_table first, in case you mess it up.
Perhaps making the backup should be part of the script?

ngombe

ngombe

2010-04-15 09:36

reporter   ~0025129

I recently upgrade from 1.1.6 to 1.2.0 a +420Mo database ...

I need a single path for all the files from all projects, had to first do a

mysql> update mantis_project_table set file_path=’/path/to/my/files/’ WHERE id ;

then modify the script move_db2disk.php line 125 did the trick:

/ $c_filename = file_clean_name( $t_row['filename'] ); /
$c_filename = md5( file_clean_name($t_row['filename']) . $t_row['date_submitted'] . $t_row['bug_id']);

rolfkleef

rolfkleef

2010-06-11 13:50

reporter   ~0025813

As part of our own upgrade process, I've taken parts from the file_add() function in core/file_api.php to replace the filename creation code in move_db2disk.php

It also sets the file permissions using the same line of code as file_add()

I think it should also fix the problem with not using the default upload path configuration.

No confirmation dialogue yet :-)

Patch attached based on the current master-1.2.x

underdog

underdog

2011-02-03 08:29

reporter   ~0028149

Hi everyone,

I have made the process to move to the hard disk attachment of the bugs stored in the database, I am trying to repeat the process from the beginning, but when I run the process again tells me that no files attached to move.

I searched the process generates a record, if so, I can not find it.

Wanted to know what exactly does the process of moving the attachment so that you know have already switched back again and if possible to let me re-run the process to move the files again, thank you all in advance and I apologize for my English.

abma

abma

2012-12-04 19:14

reporter   ~0034471

i've written my own script to do the same because i couldn't increase the time limit on the server:

https://gist.github.com/4210521

maybe it helps others! use at your own risk!

you only need to remove the line "include('../../springpw.php');"
and set some valid values here:
mysql_connect($spring_dbhost, $spring_dbuser, $spring_dbpass);

then it should work fine, it also uses paths set in project settings.

my script can be run from console, this is why i wrote it... (found no easy way to run the move_db2disk.php on the console).

pbourjac

pbourjac

2012-12-17 09:03

reporter   ~0034568

I tested the patch provided a few weeks ago, and it looks like it nicely did the job. I had to run the script several times since identical file names were generated on the same run for duplicate file names, but the script handled it correctly, refusing to overwrite the first file, and keeping the duplicates in the database. Each run did a number of extractions, so I ended up with no files kept in the database, as it should. So for me, it is fine.

rombert

rombert

2013-04-23 16:49

reporter   ~0036652

@dregad - I recall you fixed something similar recently, related to name clashes between attachments. Does that fix this issue as well?

dregad

dregad

2013-04-24 04:37

developer   ~0036658

Should be fixed in 1.2.16 with the fix of the db2disk utility.