Textpattern CMS

#1 2008-02-18 13:25:51

guiguibonbon

Member

Registered: 2006-02-20

Posts: 296

Bug : update filename after the file is replaced

When a file is replaced, the name isn’t updated in the database. At line 555, replace

					safe_update('txp_file', 'size = '.$size.', modified = now()', 'id = '.$id);

with

					safe_update('txp_file', "size = '$size', modified = now(), filename = '$name'", 'id = '.$id);

There’s also one snippet I don’t understand, at line 534 :

		if (!$filename) {
			file_list(gTxt('invalid_filename'));
		}

$filename refers to the old file name, while $name refers to the new one. Why would it check to see if the file used to have a name ? Also note that this procedure isn’t done when a new file is uploaded. This is the only place where the “invalid filename” text is called.

And lastly, for the sake of consistency, why was the file_db_add() function created, while it does nothing more than safe_insert() ? It just does part of it twice.

Last edited by guiguibonbon (2008-02-18 13:29:42)

#2 2008-02-18 13:33:28

guiguibonbon

Member

Registered: 2006-02-20

Posts: 296

Re: Bug : update filename after the file is replaced

Oh, ok, starting to get it.

You’re “not supposed” to replace a file with one with a different filename. Weird assumption. In that case, I guess line 534 should read if ($filename != $name).

#3 2008-02-18 13:53:45

guiguibonbon

Member

Registered: 2006-02-20

Posts: 296

Re: Bug : update filename after the file is replaced

Mystery n#3 : why delete the record for an existing file on line 544 when the replace has failed ?

So, here’s the corrected file_replace() function, which allows you to replace a file with one named differently :

	function file_replace()
	{	
		global $txpcfg,$extensions,$txp_user,$file_base_path;
		extract($txpcfg);
		$id = assert_int(gps('id'));

		$rs = safe_row('filename','txp_file',"id = $id");

		if (!$rs) {
			file_list(messenger(gTxt('invalid_id'),$id,''));
			return;
		}

		extract($rs);

		$file = file_get_uploaded();
		$name = file_get_uploaded_name();

		if ($file === false) {
			// could not get uploaded file
			file_list(gTxt('file_upload_failed') ." $name ".upload_get_errormsg($_FILES['thefile']['error']));
			return;
		}

		if (!$filename) {
			file_list(gTxt('invalid_filename'));
		} else {
			$newpath = build_file_path($file_base_path,$name);
			$oldpath = build_file_path($file_base_path,$filename);

			if (is_file($newpath)) {
				rename($newpath,$newpath.'.tmp');
				$oldpath = build_file_path($file_base_path,$name.'.tmp');
			}

			if(!shift_uploaded_file($file, $newpath)) {

				file_list($newpath.sp.gTxt('upload_dir_perms'));

				// rename tmp back
				rename($newpath.'.tmp',$newpath);			
				// remove tmp upload
				unlink($file);				
			} else {
				file_set_perm($newpath);
				if ($size = filesize($newpath))
					safe_update('txp_file', "size = '$size', modified = now(), filename = '$name'", 'id = '.$id);

				$message = gTxt('file_uploaded', array('{name}' => htmlspecialchars($name)));

				file_edit($message, $id);
				// clean up old
				if (is_file($oldpath))
					unlink($oldpath);
			}
		}
	}

#4 2008-02-18 15:34:43

iblastoff

Plugin Author

From: Toronto

Registered: 2006-06-11

Posts: 1,197

Website

Re: Bug : update filename after the file is replaced

yes i’ve always found ‘replacing a file’ to be a nuisance. especially if i upload a plugin for download, i pretty much HAVE to leave the filename as something like lam_image_uploader (and not lam_image_uploader_v0.6c or whatever which i’d rather do) or else every time i’d replace it with an updated version, the filename just stays the same.

but thanks gg for that new snippet of code!

---

my plugins || iblastoff.ca

#5 2008-02-20 08:05:48

Mary

Sock Enthusiast

Registered: 2004-06-27

Posts: 6,236

Re: Bug : update filename after the file is replaced

You’re “not supposed” to replace a file with one with a different filename. Weird assumption.

Not entirely. The purpose of the function, originally, was to allow you to update the file’s size and modified date. But yes, that should be corrected.

why delete the record for an existing file on line 544 when the replace has failed ?

Good question. Whether there’s an answer to be found… :)