PHP Upload of file broken after server move?

Chris1701 · Jul 15, 2013

Several years ago we moved our web server from a shared server to a virtual private server, mainly so that we could install a better search engine that we couldn't install on the shared server. The move went fine and the search engine works like a charm. Unfortunately I'm still dealing with a few issues leftover from the move and I could use a little advise. Just FYI, my background is mainly as a Delphi / C developer and I'm the only computer technician left after layoffs and furloughs.

The problem is that there are a number of content management pages used to add / edit / delete data from the MySql database. The pages still work but several of them have an option to upload images to the website i.e. the board of directors editor includes an option to upload a photo and that photo is displayed on the directors page. What happens is that any edits or a new record updates just fine but the upload of the image fails. I found a good tutorial for PHP uploads here: W3Schools PHP Upload and I've been following it trying to see where there could be a problem and after several days and numerous hours I haven't spotted the problem but the example is coded somewhat differently than the upload script on the site.

I've tried adding some echo statements to try and see what the error message is but another problem is that when I hit the "Submit" button on the edit form the page clears and returns to the content management window and I'm never able to see the actual error message. I'm sorry to say that I'm probably spoiled from years of using an IDE with an integrated debugger so any debugging tips would be appreciated.

I'm going to include the code of the upload script that is being used below, I'd appreciate any help to try and resolve this issue.

Thanks,

- Chris

In the edit form this is the form statement:

PHP:

<form action="index.html" method="post" enctype="multipart/form-data" >
<input type="hidden" name="table" value="<? echo "njhs_".$tablename ?>">
<input type="hidden" name="recid" value="<? echo $_GET["recid"] ?>">
<input type="hidden" name="action" value="updatedb">
<input type="hidden" name=assetPath value = "../assets">

PHP:

function upload($key){

    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	
	$ulPath = "$assetPath/$table/$key";
	$extension = substr($_FILES['upfile']['name'][$key], -3); // grabs the extension from the original file
	$extension = strtolower($extension);
	$orig_filename = $_FILES['upfile']['name'][$key];
	echo "<p>Orig Name: ".$orig_filename."</p>";
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	if ($extension == "gif" || $extension == "jpg") {
		$new_name = "$timestamp.$extension";
	} else {
		$new_name = $orig_filename;
	}		
	$the_file = "$ulPath/$new_name";
	#echo "\$the_file = $the_file<br>\n";
	if ($error) {
		echo ($error);
		return("FAILED upload");
	} else { # cool, we can continue
		if (!@copy($tmp_name, $the_file)) {
			echo("\n<b>FAILED copying temp file. ($tmp_name to $the_file)</b><br>");
			return("FAILED copy");
		}
		return $new_name;
	}
}

jpadie · Jul 16, 2013

after the upload run this in your script. the $_FILES superglobal should contain any errors that relate to the upload itself.

Code:

echo '<pre>' . print_r($_FILES, true) . "\n" . print_r($_POST, true) . '</pre>';

after that the most likely errors that arrive after a move are these

1. different php.ini files between servers. Does your new server have file uploads turned on? are the POST max etc sizes ok? does the temp dir exist?

Code:

file_uploads 1
# leave temp dir blank to force use of system temp
upload_max_filesize 20M  
max_file_uploads 10
post_max_size 22M

2. permissions issues
make sure that the php process has write and read privileges on the temp directory specified for file uploads in php.ini and in the target directory for saving the image.

3. wrong php.ini
there is normally a system wide php.ini, but this can be 'superseded' by other php.ini files, httpd.conf, .htaccess and other files of these types that are host or directory specific. putting a phpinfo() in your code can sometimes help as it will spit out the current used configuration files.

on your code, this line seems to do nothing

Code:

if ($error) {

as the $error variable is not set by your code.

don't use this code

Code:

if (!@copy($tmp_name, $the_file)) { //etc

instead do this

Code:

switch ($_FILES['upfile']['error'][$key]):
 case 0 :
   //no error, so process
   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
   if(!is_dir($ulPath)) die('upload path is not a directory');
   if(!is_writable($ulPath)) die('cannot write to upload path');
   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
    return $new_name;
   else:
    die ("Possible file upload attack!");
   endif;
   break;
 case 1: die('uploaded file is too big');
 case 2: die('uploaded file exceeds max_file_size directive in the html form');
 case 3: die('uploaded file only partially uploaded');
 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
 case 6: die('cannot locate a temporary folder');
 case 7: die('cannot write upload to disk');
 case 8: die('a php extension prevented the file upload');
endswitch;

Chris1701 · Jul 16, 2013

First, thank you for taking the time to help me.

I'm working my way through your suggestions one at a time:

I added the code you indicated into the script "echo '<pre>' . print_r($_FILES, true) . "\n" . print_r($_POST, true) . '</pre>';" and when I try to edit a record and upload two image files this is the part of what is returned that seems relevant to the upload of the images:

Code:

Array
(
    [small_image_file] => Array
        (
            [name] => 1022869850.jpg
            [type] => image/jpeg
            [tmp_name] => /tmp/php65ZsNy
            [error] => 0
            [size] => 2612
        )

    [large_image_file] => Array
        (
            [name] => 1022869850.jpg
            [type] => image/jpeg
            [tmp_name] => /tmp/phpSobN53
            [error] => 0
            [size] => 8136
        )

)

I gather that a 0 error code means that the upload was successful but the images never appear in the location they are supposed to. I searched the entire directory structure for the images that I uploaded but they don't appear to be anywhere that I looked.

I searched the server for "php.ini" file and in the structure of the web server our site is located in "/home/webadmin/<ourdomain>/html" and the "php.ini" files located are in these locations:

/etc/php.ini
/proc/32701/root/etc/php.ini
/proc/32701/root/proc/32701/root/etc/php.ini
/proc/32701/root/proc/32701/task/32701/root/proc/32701/root/etc/php.ini
/proc/32701/root/proc/32701/task/32701/root/proc/32701/root/proc/32701/root/etc/php.ini

There are several directory's like this where the ftp file finder just seems to recurse the same directory's deeper and deeper as if a lower level directory points back to /proc/3270 and the path ends up just going deeper and never finishes recursing

/proc/self/root/etc/php.ini
/proc/self/root/proc/1388/task/1388/root/etc/php.ini

and no where under the /home directory is there a copy of the "php.ini" file, also every copy of the "php.ini" file has an identical date and time stamp. I downloaded a copy and looked at the file, I'm not going to post the whole thing as it's 1055 lines but this is the section regarding uploads:

Code:

;;;;;;;;;;;;;;;;
; File Uploads ;
;;;;;;;;;;;;;;;;

; Whether to allow HTTP file uploads.
file_uploads = On

; Temporary directory for HTTP uploaded files (will use system default if not
; specified).
;upload_tmp_dir =

; Maximum allowed size for uploaded files.
upload_max_filesize = 2M

As per your suggestion I checked phpinfo() and the ini file that is being used is /etc/php.ini

I see that no upload temp directory is specified, in general where should that be located in the overall directory structure? That may end up being the problem, once I hear back from you as to the correct location (should it just be called "temp") and I'll create the directory using the server control panels file manager. One thing that I'm unsure of is how to make sure that the Php process has read and write privileges on the temp directory?

- Chris

jpadie · Jul 16, 2013

ok.

in the absence of a specified folder for temp, php will use the system temp. which is fine.
the file upload max is a bit low. but other than that, you're good to go.

the debug information at the top suggests that the file is getting uploaded just fine.

so now try the file handling code that I posted (the last code snip) and see what happens.

Chris1701 · Jul 16, 2013

I tried replacing the file handling code as you suggested, the updated upload code is below. I uploaded the new db.inc.php file to the server and tried again to edit a record and upload a couple files. I browsed for the files and selected them and hit the "Submit" button and no error appeared on the screen. When I look at the MySql database the two fields "small_image_file" and "large_image_file" which should hold the name of the files I'm uploading are blank and I can't find the files anywhere on the server. I've searched the server for both the name of the files I'm uploading "1022869850.jpg" as well as the tmp name returned in the $FILES array "phpMGIuhK" and "php8iOUsk" (see below) and I can't find them and the system tmp directory /tmp appears to be empty.

Code:

Array
(
    [small_image_file] => Array
        (
            [name] => 1022869850.jpg
            [type] => image/jpeg
            [tmp_name] => /tmp/phpMGIuhK
            [error] => 0
            [size] => 2612
        )

    [large_image_file] => Array
        (
            [name] => 1022869850.jpg
            [type] => image/jpeg
            [tmp_name] => /tmp/php8iOUsk
            [error] => 0
            [size] => 2612
        )

)

So I'm at a loss as to why this is still failing, no errors but the files never appear on the server. Any ideas?

PHP:

function upload($key){

    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	
	$ulPath = "$assetPath/$table/$key";
	$extension = substr($_FILES['upfile']['name'][$key], -3); // grabs the extension from the original file
	$extension = strtolower($extension);
	$orig_filename = $_FILES['upfile']['name'][$key];
	echo "<p>Orig Name: ".$orig_filename."</p>";
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	if ($extension == "gif" || $extension == "jpg") {
		$new_name = "$timestamp.$extension";
	} else {
		$new_name = $orig_filename;
	}		
	$the_file = "$ulPath/$new_name";
	echo "\$the_file = $the_file<br>\n";
	if ($error) {
		echo ($error);
		return("FAILED upload");
	} else { # cool, we can continue
		# old Eduweb code
		#if (!@copy($tmp_name, $the_file)) {
		#	echo("\n<b>FAILED copying temp file. ($tmp_name to $the_file)</b><br>");
		#	return("FAILED copy");
		#}
		#return $new_name;
		
		# New Tech-Tips code
		switch ($_FILES['upfile']['error'][$key]):
		 case 0 :
		   //no error, so process
		   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
		   if(!is_dir($ulPath)) die('upload path is not a directory');
		   if(!is_writable($ulPath)) die('cannot write to upload path');
		   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;
		 case 1: die('uploaded file is too big');
		 case 2: die('uploaded file exceeds max_file_size directive in the html form');
		 case 3: die('uploaded file only partially uploaded');
		 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
		 case 6: die('cannot locate a temporary folder');
		 case 7: die('cannot write upload to disk');
		 case 8: die('a php extension prevented the file upload');
		endswitch; 

	}
}

jpadie · Jul 16, 2013

once a script ends, php will delete any files that it has created in the temp directory. so I am not surprised that you are not seeing anything there.

I suggest the next step is to 'footprint' your code a little.

Code:

function upload($key){

    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	jLog('post data', $_POST);
	$ulPath = "$assetPath/$table/$key";
        jLog('ulPath', $ulPath');
	$extension = substr($_FILES['upfile']['name'][$key], -3); // grabs the extension from the original file
	$extension = strtolower($extension);
	$orig_filename = $_FILES['upfile']['name'][$key];
        jLog('original filename', $orig_filename;
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	if ($extension == "gif" || $extension == "jpg") {
		$new_name = "$timestamp.$extension";
	} else {
		$new_name = $orig_filename;
	}		
        jLog('new name', $new_name);
	$the_file = "$ulPath/$new_name";
        jLog('the_file', $the_file);
		switch ($_FILES['upfile']['error'][$key]):
		 case 0 :
                   jLog('no file upload errors');
		   //no error, so process
		   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
		   jLog('upload path found');
                   if(!is_dir($ulPath)) die('upload path is not a directory');
                   jLog('upload path is a directory');		   
                   if(!is_writable($ulPath)) die('cannot write to upload path');
		   jLog('upload path is writable');
                   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			jLog('move successful');
                        jLog('new path', $the_file);
                        return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;
		 case 1: die('uploaded file is too big');
		 case 2: die('uploaded file exceeds max_file_size directive in the html form');
		 case 3: die('uploaded file only partially uploaded');
		 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
		 case 6: die('cannot locate a temporary folder');
		 case 7: die('cannot write upload to disk');
		 case 8: die('a php extension prevented the file upload');
		endswitch; 

	}
}
function jLog($message, $data = NULL){
  echo '<pre>';
  echo '----------';
  echo $message . "\n";
  if(!is_null($data)): var_dump($data); echo "\n"; endif; 
  echo '----------';
  echo '</pre>';
}

Chris1701 · Jul 17, 2013

I added all the jLog code into the upload script (which is part of the db.ini.php file), first time around after I uploaded it I got a parse error because I didn't comment out something and I fixed that and reuploaded the file. I'm still having the exact same issue as before i.e. I edit the record and hit submit and the screen clears and returns me to the index.html page of the content management system and I'm not able to see any of the results of the jLog function.

Is there any way to pause the script execution at the end of the upload script before the screen is cleared and I'm returned to the index page?

jpadie · Jul 17, 2013

i see.

yes there is a way to view the output, come what may. try this for debugging purposes

Code:

function upload($key){
    ob_end_flush();
    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	jLog('post data', $_POST);
	$ulPath = "$assetPath/$table/$key";
        jLog('ulPath', $ulPath');
	$extension = substr($_FILES['upfile']['name'][$key], -3); // grabs the extension from the original file
	$extension = strtolower($extension);
	$orig_filename = $_FILES['upfile']['name'][$key];
        jLog('original filename', $orig_filename;
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	if ($extension == "gif" || $extension == "jpg") {
		$new_name = "$timestamp.$extension";
	} else {
		$new_name = $orig_filename;
	}		
        jLog('new name', $new_name);
	$the_file = "$ulPath/$new_name";
        jLog('the_file', $the_file);
		switch ($_FILES['upfile']['error'][$key]):
		 case 0 :
                   jLog('no file upload errors');
		   //no error, so process
		   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
		   jLog('upload path found');
                   if(!is_dir($ulPath)) die('upload path is not a directory');
                   jLog('upload path is a directory');		   
                   if(!is_writable($ulPath)) die('cannot write to upload path');
		   jLog('upload path is writable');
                   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			jLog('move successful');
                        jLog('new path', $the_file);
exit;
                        return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;
		 case 1: die('uploaded file is too big');
		 case 2: die('uploaded file exceeds max_file_size directive in the html form');
		 case 3: die('uploaded file only partially uploaded');
		 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
		 case 6: die('cannot locate a temporary folder');
		 case 7: die('cannot write upload to disk');
		 case 8: die('a php extension prevented the file upload');
		endswitch; 
	}
}

Chris1701 · Jul 17, 2013

I went over the code you posted and as far as I can see the only thing different is two statements first "ob_end_flush();" at the beginning of the function and then an "exit;" statement in this part of the code:

PHP:

        if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			jLog('move successful');
                        jLog('new path', $the_file);
exit;
                        return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;

Is that correct?

I added those statements and uploaded the db.inc.php file and tried to edit one of the records again, I browsed to the files and selected them and when I hit the submit button I'm still not seeing any errors or output and the screen is cleared and I'm returned to the content management home page?

jpadie · Jul 17, 2013

Hi
That is strongly suggestive of the function never being called at all.
What cms are you using?

Chris1701 · Jul 18, 2013

This web site was written way back circa 2000-2001 by some consultants that are no longer in business, it's their own cms not anything like Wordpress, Joomla or Drupal. It uses a number of Php functions and forms to edit / add / delete items from the MySql databases. I've looked through the code in the index.html and db.inc.php and I believe that this is the function responsible for updating the database after submit on the edit form is pressed:

PHP:

function doUpdate($recid){

    // echo "<P>Table: ".$table."<br></p>";

	global $table, $fields, $values, $sf, $ExhibitDate, $EventData, $cbyn, $CBField, $SetItems,
	  $upfile, $timestamp;
	
	$table = $_POST["table"]; 
	$fields = $_POST["fields"]; 
	$values = $_POST["values"];
	$sf = $_POST["sf"];
	$ExhibitDate = $_POST["ExhibitDate"];
	$EventData = $_POST["EventData"];
	$cbyn = $_POST["cbyn"];
	$CBField = $_POST["CBField"];
	$SetItems = $_POST["SetItems"];
	$upfile = $_POST["upfile"];
	$timestamp = $_POST["timestamp"];
	
	
	if ($upfile) {
	    echo "<p> In UpFile FileName: ".$_FILES['upfile']['name']."</p>";
		$timestamp = time(); // include the same nix timestamp in each file
		foreach ($upfile as $key => $value){
			if ($_FILES['upfile']['name'][$key]){
				if ($value != "none") {
					$filename = upload($key);
					echo "<p> FileName: ".$filename." Table: ".$table." Key: ".$key." RecID: ".$recid."</p>";
					deletePreviousFile($table, $key, $recid);
					$SetItems .= "$key=".$filename.", "; // start query for the update
				}
			}
		}
	}
	if ($sf) { // check for "simple field" input
		$items = buildUpdate_SimpleFields($sf);
	}
	if ($EventData) { // check for event/calendar input
		$dbdate = $EventData["year"] . "-" . $EventData["month"] . "-" . $EventData["day"];
		$stime = calcHour($EventData["shour"], $EventData["sampm"]) . ":" . $EventData["sminute"];
		$etime = calcHour($EventData["ehour"], $EventData["eampm"]) . ":" . $EventData["eminute"];
		$items .= "date='".$dbdate."', start_time='".$stime."', end_time='".$etime."', ";
		// echo "<p>DBInc Items: ".$items."</p>";
	}
	
	if ($ExhibitDate) { // check for Exhibit Date input
		$exdate = $ExhibitDate["year"] . "-" . $ExhibitDate["month"] . "-" . $ExhibitDate["day"];
		$items .= "startdate='$exdate', ";
	}
	if ($CBField) { // check for checkbox input
		/*
		This is a kind of hack to make checkboxes work properly.
		Since checkboxes don't return their names if unchecked, we must remember to use a hidden input
		field in forms to return the name of checkbox fields
		*/
		foreach ($CBField as $key => $value) {
			if ($cbyn[$value]){
				$items .= "$value='y', ";
			} else {
				$items .= "$value='n', ";
			}
		}
	}
	
	
	// echo "<p>DBInc Items: ".$items."</p>";
	
	$items = substr($items, 0, -2);
	$query = "UPDATE $table SET $items WHERE recid=$recid";

	dbConnect();
	$result = mysql_query($query) or die ("Invalid Query");
	mysql_close();
}

and there is a section of code that calls the upload function in db.inc.php:

PHP:

     if ($upfile) {
	        echo "<p> In UpFile FileName: ".$_FILES['upfile']['name']."</p>";
		$timestamp = time(); // include the same nix timestamp in each file
		foreach ($upfile as $key => $value){
			if ($_FILES['upfile']['name'][$key]){
				if ($value != "none") {
					$filename = upload($key);
					echo "<p> FileName: ".$filename." Table: ".$table." Key: ".$key." RecID: ".$recid."</p>";
					deletePreviousFile($table, $key, $recid);
					$SetItems .= "$key=".$filename.", "; // start query for the update
				}
			}
		}
	}

Something in here is failing and probably as you guessed upload is never called. I wonder if for some reason the main condition "if ($upfile) {" is failing / false. Going back and forth over the code one thing that I've noticed and can't reconcile is that the code in the "doInsert" function declares these variables:

PHP:

$table = $_POST["table"]; 
	$fields = $_POST["fields"];
	$values = $_POST["values"];
	$sf = $_POST["sf"];
	$ExhibitDate = $_POST["ExhibitDate"];
	$EventData = $_POST["EventData"];
	$cbyn = $_POST["cbyn"];
	$CBField = $_POST["CBField"];
	$upfile[large_image_file] = $_FILES["large_image_file"];
	$upfile[small_image_file] = $_FILES["small_image_file"];
	$timestamp = $_POST["timestamp"];

but in the "doUpdate" function (which is the one I'm testing) these are the variables declared:

PHP:

$table = $_POST["table"]; 
	$fields = $_POST["fields"]; 
	$values = $_POST["values"];
	$sf = $_POST["sf"];
	$ExhibitDate = $_POST["ExhibitDate"];
	$EventData = $_POST["EventData"];
	$cbyn = $_POST["cbyn"];
	$CBField = $_POST["CBField"];
	$SetItems = $_POST["SetItems"];
	$upfile = $_POST["upfile"];
	$timestamp = $_POST["timestamp"];

I notice that in the insert function "$upfile" is declared with fields for "large_image_file" and "small_image_file" but not in the "doUpdate" function where there's only one declaration "$upfile = $_POST["upfile"];". Do you think that could be the problem and replacing that line of code with the two from the insert function may fix it?

jpadie · Jul 18, 2013

bit short of time but try changing that condition for

Code:

if(isset($_FILES['upfile'])):
...
endif;

Chris1701 · Jul 18, 2013

Take your time, this has been broken for more than 4 years, nobody's going to flip if it takes a few weeks to get it fixed now that I'm getting a chance to work on it.

Sorry to say it didn't work, this is now the code for that block in the doupdate function:

PHP:

    if(isset($_FILES['upfile'])):
        echo "<p> In UpFile FileName: ".$_FILES['upfile']['name']."</p>";
		$timestamp = time(); // include the same nix timestamp in each file
		foreach ($upfile as $key => $value){
			if ($_FILES['upfile']['name'][$key]){
				if ($value != "none") {
					$filename = upload($key);
					echo "<p> FileName: ".$filename." Table: ".$table." Key: ".$key." RecID: ".$recid."</p>";
					deletePreviousFile($table, $key, $recid);
					$SetItems .= "$key=".$filename.", "; // start query for the update
				}
			}
		}
	endif;

and when I try and update a record I still don't see any of the diagnostic messages that I've put in for example in this block I'm not seeing anything from "echo " In UpFile FileName: ".$_FILES['upfile']['name']."";" so it's not even making it to there. Just as a test outside this block right before the "if(isset($_FILES['upfile'])):", I added this:

PHP:

 echo "<p> In UpFile FileName: ".$upfile["large_image_file"]."</p>";

and in this case I am actually seeing the result of that which is "In UpFile FileName: Array".

I then changed that statement to echo " In UpFile FileName: ".$_FILES['upfile']['name']."";" and " In UpFile FileName: ".$_FILES['upfile']."";" and the result of both of those was apparently blank as it said "In UpFile FileName:" after an edit. I know that updates are being applied because there's a contact phone number field and every edit I alternately add then delete my extension from that and that change is being applied.

After this I took a different approach and I tried using the option to add a new record rather then edit a record and I created a dummy record and this time when I hit "submit" I got some results from the logging code that you gave me:

Code:

----------post data
array(6) {
  ["table"]=>
  string(13) "njhs_exhibits"
  ["recid"]=>
  string(0) ""
  ["action"]=>
  string(8) "updatedb"
  ["assetPath"]=>
  string(44) "/home/webadmin/<My Domain>/html/assets"
  ["sf"]=>
  array(8) {
    ["title"]=>
    string(20) "Chris\'s Test Record"
    ["short_description"]=>
    string(19) "Test exhibit create"
    ["long_description"]=>
    string(19) "Test exhibit create"
    ["detail_html"]=>
    string(0) ""
    ["status"]=>
    string(4) "past"
    ["contact_name"]=>
    string(5) "Chris"
    ["contact_phone"]=>
    string(4) "x256"
    ["contact_email"]=>
    string(23) "Chris@ABCXYZ.org"
  }
  ["ExhibitDate"]=>
  array(3) {
    ["month"]=>
    string(1) "7"
    ["day"]=>
    string(2) "18"
    ["year"]=>
    string(4) "2010"
  }
}

----------

----------ulPath
string(75) "/home/webadmin/<My Domain>/html/assets/njhs_exhibits/large_image_file"

----------

----------original filename
----------

----------new name
----------

----------the_file
string(76) "/home/webadmin/<My Domain>/html/assets/njhs_exhibits/large_image_file/"

----------

----------no file upload errors
----------

----------upload path found
----------

----------upload path is a directory
----------

----------upload path is writable
----------

Possible file upload attack!

So looking at the code in the upload function these statement are returning blank:

PHP:

jLog('original filename', $orig_filename );
jLog('new name', $new_name);
jLog('the_file', $the_file);

So there's no filename in those variables and the "$the_file" variable is a path without a file name. The latest version of the upload function code with all the modification that you've suggested is below:

Code:

function upload($key){
    ob_end_flush();
    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	jLog('post data', $_POST);
	$ulPath = "$assetPath/$table/$key";
    jLog('ulPath', $ulPath);
	$extension = substr($_FILES['upfile']['name'][$key], -3); // grabs the extension from the original file
	$extension = strtolower($extension);
	$orig_filename = $_FILES['upfile']['name'][$key];
    jLog('original filename', $orig_filename );
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	if ($extension == "gif" || $extension == "jpg") {
		$new_name = "$timestamp.$extension";
	} else {
		$new_name = $orig_filename;
	}		
        jLog('new name', $new_name);
	$the_file = "$ulPath/$new_name";
        jLog('the_file', $the_file);
		switch ($_FILES['upfile']['error'][$key]):
		 case 0 :
                   jLog('no file upload errors');
		   //no error, so process
		   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
		   jLog('upload path found');
                   if(!is_dir($ulPath)) die('upload path is not a directory');
                   jLog('upload path is a directory');		   
                   if(!is_writable($ulPath)) die('cannot write to upload path');
		   jLog('upload path is writable');
                   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			jLog('move successful');
                        jLog('new path', $the_file);
			exit;
                        return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;
		 case 1: die('uploaded file is too big');
		 case 2: die('uploaded file exceeds max_file_size directive in the html form');
		 case 3: die('uploaded file only partially uploaded');
		 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
		 case 6: die('cannot locate a temporary folder');
		 case 7: die('cannot write upload to disk');
		 case 8: die('a php extension prevented the file upload');
		endswitch; 
	}

Any thoughts on this?

Chris1701 · Jul 18, 2013

One other thing, I went to delete the dummy record that I had supposedly added and it appears as if that no new record was created since I can't see it in either the content management editor for that table nor browsing through the database in PHPMyAdmin.

jpadie · Jul 18, 2013

that dump from jLog is very useful. it shows that the $key variable that is being passed is essentially a null or empty string.

so can we add a couple more lines to the script and retest the output?

Code:

//after this line
jLog('post data', $_POST);
jLog('files data', $_FILES);
jLog('$key', $key);

the dump of the post data also shows that there is no post field called upfile in the html form you are using. so the original conditional never gets fired. and my quick fix won't work either because inside the conditional the foreach loop runs over $_POST['upfile']

so here is a longer fix. one other thing to note is that the doUpload() function refers to a _POST['timestamp'], which does not exist in the field data the jLog provides. I suggest you change that to $timestamp = time(); or better still use a unique id

Code:

$new_name = uniqid('uploadFile', true) . '.' . $extension;

Code:

<?php

function getSet($field, $value){
	return sprintf("`%s`='%s'", $field, mysql_real_escape_string($value));
}

function doUpdate($recid){

	// echo "<P>Table: ".$table."<br></p>";

	global $table, $fields, $values, $sf, $ExhibitDate, $EventData, $cbyn, $CBField, $SetItems,
	$upfile, $timestamp;
	
	dbConnect();
	
	$table = $_POST["table"];
	$fields = $_POST["fields"];
	$values = $_POST["values"];
	$sf = $_POST["sf"];
	$ExhibitDate = $_POST["ExhibitDate"];
	$EventData = $_POST["EventData"];
	$cbyn = $_POST["cbyn"];
	$CBField = $_POST["CBField"];
	$SetItems = $_POST["SetItems"];
	
	$timestamp = $_POST["timestamp"];

	$setItems = array();
	
	$timestamp = time(); // include the same nix timestamp in each file
	
	if (isset($_FILES['upfile'])):
		
		foreach ($_FILES['upfile']['name'] as $key=>$originalName):
			$filename = upload($key);
			jLog('filename returned from upload function for key = ' . $key, $filename);
			jLog("FileName: ".$filename." Table: ".$table." Key: ".$key." RecID: ".$recid, '');
			deletePreviousFile($table, $key, $recid);
			$setItems[] = getSet($key, $filename);
		endforeach;
	endif;

	if ($sf) : // check for "simple field" input
		//$items = buildUpdate_SimpleFields($sf);
		foreach($_POST['sf'] as $field=>$value):
			$setItems[] = getSet($field, $value);
		endforeach;
	endif;
	
	if ($EventData) : // check for event/calendar input
		$dbdate = $EventData["year"] . "-" . $EventData["month"] . "-" . $EventData["day"];
		$stime = calcHour($EventData["shour"], $EventData["sampm"]) . ":" . $EventData["sminute"];
		$etime = calcHour($EventData["ehour"], $EventData["eampm"]) . ":" . $EventData["eminute"];
		$setItems[] = getSet('date',$dbdate);
		$setItems[] = getSet('start_time', $stime);
		$setItems[] = getSet('end_time', $etime);
		
	endif;
	

	if ($ExhibitDate) : // check for Exhibit Date input
		$exdate = $ExhibitDate["year"] . "-" . $ExhibitDate["month"] . "-" . $ExhibitDate["day"];
		$setItems[] = getSet("startdate", $exdate);
	endif;
	if ($CBField) : // check for checkbox input
		/*
		This is a kind of hack to make checkboxes work properly.
		Since checkboxes don't return their names if unchecked, we must remember to use a hidden input
		field in forms to return the name of checkbox fields
		*/
		foreach ($CBField as $key => $value) :
			$setItems[] = getSet($value, $cbyn[$value] ? 'y' : 'n');
		endforeach;
	endif;


	// echo "<p>DBInc Items: ".$items."</p>";

	$items = substr($items, 0, -2);
	$query = 'UPDATE $table SET %s WHERE recid="%s"';
	$query = sprintf($query, implode(',', $setItems), mysql_real_escape_string($recid));

	
	$result = mysql_query($query) or die ("Invalid Query: " . mysql_error() . " \n Query was: $query");
	mysql_close();
}

?>

Chris1701 · Jul 22, 2013

Ok, as I remember from when the hosting company migrated our site there were a slew of problems mostly do to the fact that the original authors had written the site assuming that all variable were global. The much newer version of PHP on the VPS server had that option turned off by default for security reasons and the hosting company discouraged me from changing that option back. So most likely the variables that are blank are ones that not being passed by post and were assumed to be global, which may have been an accepted practice at the time but which is not now.

In this problem there are basically two files involved: index.html (the index for the content management system) and the edit_exhibits.php which is the page which is called when either Edit (on a particular record) or Add New to add a new record is clicked. The index.html includes two PHP files db.inc.php and forms.inc.php, while the edit_exhibits.php only includes db.inc.php but oddly there's a condition at the beginning that only includes db.inc.php if a recid is being passed:

PHP:

if ($_GET["recid"]){
	include("../includes/db.inc.php");
	$RowDat = getRowData("njhs_".$tablename, $_GET["recid"]);
	$pdate = explode("-", $RowDat["startdate"]);
	$year = $pdate[0];
	$month = $pdate[1];
	$mday = $pdate[2];
}
$fdate = getdate();
?>

This seems odd to me since to add a new record functions from the db.inc.php are going to be required, I'm going to test putting that include outside that condition to see if that makes any difference.

I'm going to try and track down where $key and $upfile are being declared and they either have to be made global or passed through post.

I searched through all php files looking for where upfile was declared and I found a backup copy of the edit_exhibits.php file from 2007 and oddly enough the current code for the input fields for the file upload are this:

PHP:

<input type="file" name="small_image_file" size = "80">
<input type="file" name="large_image_file" size = "80">

and the old code was:

PHP:

<input type="file" name="upfile[small_image_file]" size = "80">
<input type="file" name="upfile[large_image_file]" size = "80">

I'm not sure who changed this but the hosting company's migration team was supposed to fix these problems and never did and they may have made some changes that were supposed to fix these problems but in actuality they did not.

I added the lines you requested to the jlog and I changed the lines of code for those two input file fields back to the old code "<input type="file" name="upfile[small_image_file]" size = "80">". It also seems to me that edit_exhibits.php is where upfile is declared but not as global so I created upfile as a post variable and then in the doinsert function (in db.inc.php) I added this line with the rest of the variable declarations: "$upfile = $_POST["upfile"];" and then retried inserting a dummy exhibits record. The results of the jlog are below and as with my previous attempt there must be something wrong with the doinsert function since no new record was created.

Code:

----------post data
array(6) {
  ["table"]=>
  string(13) "njhs_exhibits"
  ["recid"]=>
  string(0) ""
  ["action"]=>
  string(8) "updatedb"
  ["assetPath"]=>
  string(44) "/home/webadmin/<my domain>/html/assets"
  ["sf"]=>
  array(8) {
    ["title"]=>
    string(20) "Chris\'s Test Record"
    ["short_description"]=>
    string(19) "Test exhibit create"
    ["long_description"]=>
    string(30) "Test exhibit create new record"
    ["detail_html"]=>
    string(0) ""
    ["status"]=>
    string(4) "past"
    ["contact_name"]=>
    string(5) "Chris"
    ["contact_phone"]=>
    string(12) "xxx-xxx-xxxx"
    ["contact_email"]=>
    string(23) "Chris@<my domain>"
  }
  ["ExhibitDate"]=>
  array(3) {
    ["month"]=>
    string(1) "7"
    ["day"]=>
    string(2) "22"
    ["year"]=>
    string(4) "2012"
  }
}

----------

----------files data
array(2) {
  ["small_image_file"]=>
  array(5) {
    ["name"]=>
    string(15) "1022869850s.jpg"
    ["type"]=>
    string(10) "image/jpeg"
    ["tmp_name"]=>
    string(14) "/tmp/phpuvzJMJ"
    ["error"]=>
    int(0)
    ["size"]=>
    int(2612)
  }
  ["large_image_file"]=>
  array(5) {
    ["name"]=>
    string(15) "1022869850l.jpg"
    ["type"]=>
    string(10) "image/jpeg"
    ["tmp_name"]=>
    string(14) "/tmp/phpemmgoj"
    ["error"]=>
    int(0)
    ["size"]=>
    int(8136)
  }
}

----------

----------$key
string(16) "large_image_file"
----------

----------ulPath
string(75) "/home/webadmin/jerseyhistory.org/html/assets/njhs_exhibits/large_image_file"
----------

----------original filename 
----------

----------new name
----------

----------the_file
string(76) "/home/webadmin/jerseyhistory.org/html/assets/njhs_exhibits/large_image_file/"

----------

----------no file upload errors
----------

----------upload path found
----------

----------upload path is a directory
----------

----------upload path is writable
----------

Possible file upload attack!

You mentioned "one other thing to note is that the doUpload() function refers to a _POST['timestamp'], which does not exist in the field data the jLog provides. I suggest you change that to $timestamp = time(); or better still use a unique id"; I see where in the doUpdate and doInsert functions have conflicting information where they declare global $timestamp and yet there's a statement "$timestamp = $_POST["timestamp"];". This is wrong for sure and in the doUpdate and doInsert I will remove $timestamp as global as well as the "$timestamp = $_POST["timestamp"];" since there is later in both functions "$timestamp = time(); // include the same nix timestamp in each file" which is as you say is correct and as far as I can tell neither index.html nor edit_exhibits.php uses or declares a $timestamp variable so it is meant to be local to the doUpdate and doInsert functions.

I commented out the old doUpdate function and pasted in your rewritten doUpdate, based on what I've written above I removed $upfile as global and added "$upfile = $_POST["upfile"];" to the variable declarations so upfile is being pasted from edit_exhibits.php; I also removed $timestamp as global since I believe it's meant to be local only. I tried updating a record where the files are blank and unfortunately the files failed to upload, the results of the jlog are below and I see some signs of progress. "upfile" now contains the correct file names that I'm aiming to upload; I'm unsure of what the purpose of the $key variable is as it's value seems meaningless; something is wrong with the code that create the $new_name and $the_file variables since $new_name appears to be an extension without a file name and $the_file appears to be a path and extension without a file name.

Code:

----------post data
array(7) {
  ["table"]=>
  string(13) "njhs_exhibits"
  ["recid"]=>
  string(2) "77"
  ["action"]=>
  string(8) "updatedb"
  ["upfile"]=>
  string(0) ""
  ["assetPath"]=>
  string(44) "/home/webadmin/<My Domain>/html/assets"
  ["sf"]=>
  array(8) {
    ["title"]=>
    string(94) "<Exhibit description bla bla bla>"
    ["short_description"]=>
    string(76) "<More Exhibit description bla bla bla>"
    ["long_description"]=>
    string(835) "<Long Exhibit description bla bla bla>"
    ["detail_html"]=>
    string(0) ""
    ["status"]=>
    string(4) "past"
    ["contact_name"]=>
    string(0) ""
    ["contact_phone"]=>
    string(17) "xxx-xxx-xxx x256"
    ["contact_email"]=>
    string(23) "Steve@<My Domain>"
  }
  ["ExhibitDate"]=>
  array(3) {
    ["month"]=>
    string(2) "12"
    ["day"]=>
    string(2) "20"
    ["year"]=>
    string(4) "2011"
  }
}

----------

----------files data
array(1) {
  ["upfile"]=>
  array(5) {
    ["name"]=>
    array(2) {
      ["small_image_file"]=>
      string(15) "1022869850s.jpg"
      ["large_image_file"]=>
      string(15) "1022869850l.jpg"
    }
    ["type"]=>
    array(2) {
      ["small_image_file"]=>
      string(10) "image/jpeg"
      ["large_image_file"]=>
      string(10) "image/jpeg"
    }
    ["tmp_name"]=>
    array(2) {
      ["small_image_file"]=>
      string(14) "/tmp/phpEtH0en"
      ["large_image_file"]=>
      string(14) "/tmp/phpeQajnA"
    }
    ["error"]=>
    array(2) {
      ["small_image_file"]=>
      int(0)
      ["large_image_file"]=>
      int(0)
    }
    ["size"]=>
    array(2) {
      ["small_image_file"]=>
      int(2612)
      ["large_image_file"]=>
      int(8136)
    }
  }
}

----------

----------$key
string(16) "small_image_file"

----------

----------ulPath
string(75) "/home/webadmin/jerseyhistory.org/html/assets/njhs_exhibits/small_image_file"

----------

----------original filename
string(15) "1022869850s.jpg"

----------

----------new name
string(4) ".jpg"

----------

----------the_file
string(80) "/home/webadmin/jerseyhistory.org/html/assets/njhs_exhibits/small_image_file/.jpg"

----------

----------no file upload errors
----------

----------upload path found
----------

----------upload path is a directory
----------

----------upload path is writable
----------

----------move successful
----------

----------new path
string(80) "/home/webadmin/jerseyhistory.org/html/assets/njhs_exhibits/small_image_file/.jpg"

----------

jpadie · Jul 23, 2013

can you post the raw HTML of the upload form?

jpadie · Jul 23, 2013

on the upload function (rather than doUpload() the issue now boils down to this line:

Code:

$new_name = "$timestamp.$extension";

this is working (technically), but because $timestamp is set to the value of $_POST['timestamp'], which does not exist the result is just '.jpg'; which gets overwritten each time as it's the same.

since the name of the file does not really have to be meaningful, I would do this instead:

Code:

<?php 
function upload($key){
    ob_end_flush();
    $table = $_POST["table"]; 
	$assetPath = $_POST["assetPath"];
	$timestamp = $_POST["timestamp"];
	jLog('post data', $_POST);
	$ulPath = "$assetPath/$table/$key";
    jLog('ulPath', $ulPath);
	
    
    /** deal with existing file names **/
    $orig_filename = $_FILES['upfile']['name'][$key];
    jLog('original filename', $orig_filename );
	$tmp_name = $_FILES['upfile']['tmp_name'][$key];
	jLog('tmpFile name filename', $tmp_name );
	/**construct new unique file name **/
	$ibits = getimagesize($tmp_name);
	if($ibits === false) die("file is not an image");
	list($w, $h, $type) = $ibits;
	$new_name = uniqid('image_file',true) . image_type_to_extension($type, true);
	jLog('new name', $new_name);
	$the_file = "$ulPath/$new_name";
    jLog('the_file', $the_file);

    /** end file name code **/
    
    switch ($_FILES['upfile']['error'][$key]):
		 case 0 :
           jLog('no file upload errors');
		   //no error, so process
		   if(!file_exists($ulPath)) die('cannot find the upload path') ; //or maybe create it
		   jLog('upload path found');
                   if(!is_dir($ulPath)) die('upload path is not a directory');
                   jLog('upload path is a directory');		   
                   if(!is_writable($ulPath)) die('cannot write to upload path');
		   jLog('upload path is writable');
                   if (move_uploaded_file($_FILES['upfile']['tmp_name'][$key], $the_file)):
			jLog('move successful');
                        jLog('new path', $the_file);
			exit;
                        return $new_name;
		   else:
			die ("Possible file upload attack!");
		   endif;
		   break;
		 case 1: die('uploaded file is too big');
		 case 2: die('uploaded file exceeds max_file_size directive in the html form');
		 case 3: die('uploaded file only partially uploaded');
		 case 4: die('no file was uploaded'); //may be better to handle this more gracefully
		 case 6: die('cannot locate a temporary folder');
		 case 7: die('cannot write upload to disk');
		 case 8: die('a php extension prevented the file upload');
		endswitch; 
	} 

?>

I have also been meaning to point out that the idea of including the file save path etc in the POST data does not sound great. for example, if your perms were set in a certain way, a malicious user could upload an image and have it overwrite files - potentially even those like .htaccess (although with the latest fix above, I can't immediately see how).

It would be better practice to assess the upload path and table server side at the point that they are used, rather than at the point that the form is served. this is because a user could just change that information on resubmission and your receiving code does not validate the input. If there is some reason why it can only be assessed when the form is served, then store the data in a session variable; rather than client-side.

this is all more complicated/time-consuming because you're dealing with cr*ppy code to start with. It would take much less time (for you) to refactor this aspect of the code from the ground up. Should you want to do but feel you cannot share the code publicly, feel free to reach out via the contact details on my web-site (rathercurious.net) and then, when done, we can post back a solution with any sensitive bits left out; so that others might benefit. I believe I have some pre-built classes that will get you to a solution in 10-15 mins; but would need to see the code first.

I'd need the code from the point that the page with the form is generated to the point that the submission page is generated. or just send it all. I don't need passwords etc.

Chris1701 · Jul 23, 2013

I'm going to email you the files that are involved in the problem, so you should have them shortly.

In regards to the $timestamp variable and the filename problem, I had looked through the four files involved and it appears that it was declared in doInsert and doUpdate but not otherwise used there. Then $timestamp was used in upload without being passed in or initialized. So I removed any reference to $timestamp in doInsert and doUpdate and then in the upload function commented out "$timestamp = $_POST["timestamp"];" and added "$timestamp = time(); // include the same nix timestamp in each file". This seems to have fixed (at least partially) the upload problem, on retest the small image file did get uploaded and moved to the correct location but for some reason there's still a problem with the large image file and I'm going to try and locate that, maybe add some more jlog lines specifically for the the large image.

Also for some reason, despite that the file upload works (for the small image) the fields in the MySql database "small_image_file" and " large_image_file" that hold the image file names are not being updated with the names of the files being uploaded.

Code:

----------post data
array(7) {
  ["table"]=>
  string(13) "njhs_exhibits"
  ["recid"]=>
  string(2) "77"
  ["action"]=>
  string(8) "updatedb"
  ["upfile"]=>
  string(0) ""
  ["assetPath"]=>
  string(44) "/home/webadmin/<My Domain>/html/assets"
  ["sf"]=>
  array(8) {
    ["title"]=>
    string(94) "Short description"
    ["short_description"]=>
    string(76) "More Short description"
    ["long_description"]=>
    string(835) "Long exhibit description bla bla bla"
    ["detail_html"]=>
    string(0) ""
    ["status"]=>
    string(4) "past"
    ["contact_name"]=>
    string(0) ""
    ["contact_phone"]=>
    string(12) "xxx-xxx-xxxx"
    ["contact_email"]=>
    string(23) "Steve@<My Domain>"
  }
  ["ExhibitDate"]=>
  array(3) {
    ["month"]=>
    string(2) "12"
    ["day"]=>
    string(2) "20"
    ["year"]=>
    string(4) "2011"
  }
}

----------

----------files data
array(1) {
  ["upfile"]=>
  array(5) {
    ["name"]=>
    array(2) {
      ["small_image_file"]=>
      string(15) "1022869850s.jpg"
      ["large_image_file"]=>
      string(15) "1022869850l.jpg"
    }
    ["type"]=>
    array(2) {
      ["small_image_file"]=>
      string(10) "image/jpeg"
      ["large_image_file"]=>
      string(10) "image/jpeg"
    }
    ["tmp_name"]=>
    array(2) {
      ["small_image_file"]=>
      string(14) "/tmp/phpXKG53E"
      ["large_image_file"]=>
      string(14) "/tmp/php2c4mX9"
    }
    ["error"]=>
    array(2) {
      ["small_image_file"]=>
      int(0)
      ["large_image_file"]=>
      int(0)
    }
    ["size"]=>
    array(2) {
      ["small_image_file"]=>
      int(2612)
      ["large_image_file"]=>
      int(8136)
    }
  }
}

----------

----------$key
string(16) "small_image_file"

----------

----------ulPath
string(75) "/home/webadmin/<My Domain>/html/assets/njhs_exhibits/small_image_file"

----------

----------original filename
string(15) "1022869850s.jpg"

----------

----------new name
string(14) "1374583960.jpg"

----------

----------the_file
string(90) "/home/webadmin/<My Domain>/html/assets/njhs_exhibits/small_image_file/1374583960.jpg"

----------

----------no file upload errors
----------

----------upload path found
----------

----------upload path is a directory
----------

----------upload path is writable
----------

----------move successful
----------

----------new path
string(90) "/home/webadmin/<My Domain>/html/assets/njhs_exhibits/small_image_file/1374583960.jpg"

----------

Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

PHP Upload of file broken after server move?

MIS

Technical User

MIS

Technical User

MIS

Technical User

MIS

Technical User

MIS

Technical User

MIS

Technical User

MIS

MIS

Technical User

MIS

Technical User

Technical User

MIS

Similar threads

Log in

Part and Inventory Search

Sponsor