PHP upload problem in FireFox

[corianton]

Hi, new to the forum.  Thanks in advance.

 

The problem I am having is that in IE7, users can upload any of the file types I have specified (see code below), but in Fire Fox, I can't upload files of "application/msword" type.  Again IE7 works fine.  I can't figure out what I have done wrong.

 

I have a contact form for people to contact us on othe front page of my site.  In the form, I have the standard

 

   <label for="attachment" class="fullLabel">Attach File:</label>
    <input name="attachment" type="file" id="attachment" tabindex="9" size="19" />

 

And then the .php file that does the work looks like this:

 

<?php
// move the file from the temp folder
if (!empty($_FILES['attachment'])) {
if (($_FILES["attachment"]["type"] == "application/msword")
|| ($_FILES["attachment"]["type"] == "application/pdf")
|| ($_FILES["attachment"]["type"] == "text/plain")
|| ($_FILES["attachment"]["type"] == "text/rtf")
       && ($_FILES["attachment"]["size"] < 20000)) {
define('UPLOAD_DIR', '/path/to/my/uploads/');
//$theFile = $_FILES['attachment']['name'];
$theFile = str_replace(' ', '_', 'tls_file_'.date('Y-m-d').'_'.$_FILES['attachment']['name']);
move_uploaded_file($_FILES['attachment']['tmp_name'], UPLOAD_DIR.$theFile);
  }	
}


// send email
if (!empty($_POST['remail'])) {
// process the email
//if (!empty($_POST['send'])) {
//if (array_key_exists('send', $_POST)) {
$to = $_POST['remail']; // use your own email address
$subject = 'New Quote Request from TLS site';
// process the $_POST variables
$name = $_POST['f_name'].' '.$_POST['l_name'];
$phone = $_POST['phone'];
$email = $_POST['email'];
$translate = $_POST['sourceLanguage'].' -> '.$_POST['targetLanguage'];
    $desc = $_POST['description'];
$instruct = $_POST['instructions'];	
//$filename = $_FILES['attachment']['name'];
// build the message
$message = "Name: $name\n\n";
$message .= "Email: $email\n\n";
$message .= "Phone: $phone\n\n";
$message .= "Translation: $translate\n\n";	
$message .= "Description: $desc\n\n";	
$message .= "Instructions: $instruct\n\n";
$message .= "File: http://www.mysite.com/uploads/$theFile\n\n";	
// limit line length to 70 characters
$message = wordwrap($message, 70);
// send it
$mailSent = mail($to, $subject, $message);
//}
}

// insert the lead
if (!function_exists("GetSQLValueString")) {
function GetSQLValueString($theValue, $theType, $theDefinedValue = "", $theNotDefinedValue = "") 
{
  $theValue = get_magic_quotes_gpc() ? stripslashes($theValue) : $theValue;

  $theValue = function_exists("mysql_real_escape_string") ? mysql_real_escape_string($theValue) : mysql_escape_string($theValue);

  switch ($theType) {
    case "text":
      $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
      break;    
    case "long":
    case "int":
      $theValue = ($theValue != "") ? intval($theValue) : "NULL";
      break;
    case "double":
      $theValue = ($theValue != "") ? "'" . doubleval($theValue) . "'" : "NULL";
      break;
    case "date":
      $theValue = ($theValue != "") ? "'" . $theValue . "'" : "NULL";
      break;
    case "defined":
      $theValue = ($theValue != "") ? $theDefinedValue : $theNotDefinedValue;
      break;
  }
  return $theValue;
}
}

$editFormAction = $_SERVER['PHP_SELF'];
if (isset($_SERVER['QUERY_STRING'])) {
  $editFormAction .= "?" . htmlentities($_SERVER['QUERY_STRING']);
}

if ((isset($_POST["MM_insert"])) && ($_POST["MM_insert"] == "selectForm")) {
  $insertSQL = sprintf("INSERT INTO requests (id, l_name, f_name, phone, email, source, target, `description`, instructions , `date`, service_type) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)",
                       GetSQLValueString(md5($_POST['email'].time()), "text"), 
				   GetSQLValueString($_POST['l_name'], "text"),
                       GetSQLValueString($_POST['f_name'], "text"),
                       GetSQLValueString($_POST['phone'], "text"),
                       GetSQLValueString($_POST['email'], "text"),
                       GetSQLValueString($_POST['sourceLanguage'], "text"),
                       GetSQLValueString($_POST['targetLanguage'], "text"),
                       GetSQLValueString($_POST['description'], "text"),
				   GetSQLValueString($_POST['instructions'], "text"),
                       GetSQLValueString($_POST['date'], "date"),
				   GetSQLValueString($_POST['service_type'], "date"));

  mysql_select_db($database_tls_conn, $tls_conn);
  $Result1 = mysql_query($insertSQL, $tls_conn) or die(mysql_error());

  $insertGoTo = "thank_you.php";
  if (isset($_SERVER['QUERY_STRING'])) {
    $insertGoTo .= (strpos($insertGoTo, '?')) ? "&" : "?";
    $insertGoTo .= $_SERVER['QUERY_STRING'];
  }
header(sprintf("Location: %s", $insertGoTo));
}

 

I am really just concerned with the first bith, the email is working fine, as well as the mysql stuff. 

 

I have even changed that first part to look like this:

<?php
// move the file from the temp folder
if (!empty($_FILES['attachment'])) {
//if (($_FILES["attachment"]["type"] == "application/msword")
//|| ($_FILES["attachment"]["type"] == "application/pdf")
//|| ($_FILES["attachment"]["type"] == "text/plain")
//|| ($_FILES["attachment"]["type"] == "text/rtf")
       if ($_FILES["attachment"]["size"] < 20000) {
define('UPLOAD_DIR', '/path/to/my/uploads/');
//$theFile = $_FILES['attachment']['name'];
$theFile = str_replace(' ', '_', 'tls_file_'.date('Y-m-d').'_'.$_FILES['attachment']['name']);
move_uploaded_file($_FILES['attachment']['tmp_name'], UPLOAD_DIR.$theFile);
  }	
}

 

And it still doesn't work, Fire fox can't upload but IE can.

 

I ended up commenting the whole restriction section out like so:

<?php
// move the file from the temp folder
if (!empty($_FILES['attachment'])) {
//if (($_FILES["attachment"]["type"] == "application/msword")
//|| ($_FILES["attachment"]["type"] == "application/pdf")
//|| ($_FILES["attachment"]["type"] == "text/plain")
//|| ($_FILES["attachment"]["type"] == "text/rtf")
       //&& ($_FILES["attachment"]["size"] < 20000)) {
define('UPLOAD_DIR', '/path/to/my/uploads/');
//$theFile = $_FILES['attachment']['name'];
$theFile = str_replace(' ', '_', 'tls_file_'.date('Y-m-d').'_'.$_FILES['attachment']['name']);
move_uploaded_file($_FILES['attachment']['tmp_name'], UPLOAD_DIR.$theFile);
  //}	
}

 

Which makes it work, but now I think I have a security risk in that people can upload just any kind of file to my server.  So my question is, does anyone know why FireFox won't run this script properly?  And also, am I really putting myself at risk by not having ANY restriction on the kind of files uploaded?

[gevans]

Can you show us your whole form??

[corianton]

Sorry, here is the form.  I am sort of inheriting this, so it may be messy:

 

<script src="../SpryAssets/SpryValidationTextField.js" type="text/javascript"></script>
<link href="../SpryAssets/SpryValidationTextField.css" rel="stylesheet" type="text/css" />
<form method="POST" action="<?php echo $editFormAction; ?>" enctype="multipart/form-data" name="selectForm" class="Normal" id="selectForm">
  <div class="formFrame">
    <label for="f_name">First Name:</label>
    <span id="reqFirstName">
    <input name="f_name" type="text" id="f_name" tabindex="1" />
    <span class="textfieldRequiredMsg">*First Name is required.</span></span><br />
    <label for="l_name">Last Name:</label>
    <span id="reqLastName">
    <input name="l_name" type="text" id="label" tabindex="2"  />
    <span class="textfieldRequiredMsg">*First Name is required.</span></span><br />
    <label for="email">Email:</label>
    <span id="reqEmail">
    <input name="email" type="text" id="email" tabindex="3"  />
    <span class="textfieldRequiredMsg">*Email is required.</span><span class="textfieldInvalidFormatMsg">*Invalid email address.</span></span><br />
    <label for="phone">Phone:</label>
    <span id="reqPhone">
    <input name="phone" type="text" id="phone" tabindex="4" />
    <span class="textfieldRequiredMsg">*Phone is required.</span></span><br />
    <label class="fullLabel" for="sourceLanguage">Source Language:</label>
    <br />
    <div id="sourceSelectDiv" spry:region="dsSourceLanguages">
      <select name="sourceLanguage" spry:repeatchildren="dsSourceLanguages" tabindex="5" >
        <!--<option value="{language}">{language}</option>-->
        <option spry:if="{ds_RowNumber} == {ds_CurrentRowNumber}" value="{language}" selected="selected">{language}</option>
        <option spry:if="{ds_RowNumber} != {ds_CurrentRowNumber}" value="{language}">{language}</option>
      </select>
    </div>
    <label for="targetLanguage" class="fullLabel">Target Language:</label>
    <br />
    <div id="targetSelectDiv" spry:region="dsTargetLanguages" >
      <select name="targetLanguage" spry:repeatchildren="dsTargetLanguages" tabindex="6" >
        <!--<option value="{language}">{language}</option>-->
        <option spry:if="{ds_RowNumber} == {ds_CurrentRowNumber}" value="{language}" selected="selected">{language}</option>
        <option spry:if="{ds_RowNumber} != {ds_CurrentRowNumber}" value="{language}">{language}</option>
      </select>
    </div>
    <label for="catSelect" class="fullLabel">Select Category:</label>
    <br />
    <div id="categorySelector" spry:region="dsWebCategories" >
      <select id="catSelect" name="catSelect" spry:repeatchildren="dsWebCategories" tabindex="7" 
      	onchange="document.forms[0].subCatSelect.disabled = true; dsWebCategories.setCurrentRowNumber(this.selectedIndex);"  >
        <!-- <option value="{email}">{name}</option>-->
        <option spry:if="{ds_RowNumber} == {ds_CurrentRowNumber}" value="{email}" selected="selected">{name}</option>
        <option spry:if="{ds_RowNumber} != {ds_CurrentRowNumber}" value="{email}">{name}</option>
      </select>
    </div>
    <label for="catSelect" class="fullLabel">Select Sub Category:</label>
    <br />
    <div id="Web_Categories_Detail" spry:region="dsWebCategoriesDetail dsWebCategories dsWebCategories" >
      <select name="subCatSelect" spry:repeatchildren="dsWebCategoriesDetail" tabindex="8"       	
        onblur="$('remail').value = $('catSelect').getValue(); $('description').value = 'Category: ' + dsWebCategories.getRowByRowNumber($('catSelect').selectedIndex).name + 
            ' - ' + dsWebCategoriesDetail.getRowByRowNumber(this.selectedIndex).name">
        <option spry:if="{dsWebCategories::ds_RowNumber} == {dsWebCategories::ds_CurrentRowNumber}" value="{name}" selected="selected">{name}</option>
        <option spry:if="{dsWebCategories::ds_RowNumber} != {dsWebCategories::ds_CurrentRowNumber}" value="{name}">{name}</option>
        <!-- <option value="{name}">{name}</option>-->
      </select>
    </div>
    <label for="attachment" class="fullLabel">Attach File:</label>
    <input name="attachment" type="file" id="attachment" tabindex="9" size="19" />
    <br />
    <label for="comments" class="fullLabel">Special Instructions:</label>
    <br />
    <textarea name="instructions" cols="27" rows="4" id="instructions" tabindex="10"></textarea>
    <br />
<input type="radio" name="return" id="return" tabindex="11" style="clear:both; width: auto; height: auto; float:left;" onclick="toggleRepBox(this);" />
    <label for="return" style="width: 170px;">I've worked with TLS before</label>
    <br />     <br />    
    <select name="rep" id="rep" style="display:none;" onchange="$('remail').value = this.getValue();">
      	<option value="">Please select your rep</option>
        <option value="email@email.com">Aaron Smith</option>
        <option value="email@email.com">Karen Leyton</option>
        <option value="email@email.com">Mark Johnson</option>
        <option value="email@email.com">Phillip Lechter</option>
        <option value="email@email.com">Shawna Barnett</option>
        <option value="email@email.com">Jay Casper</option>
        <option value="email@email.com">Jim Sterzer</option>
        <option value="email@email.com">Andrea Thompson</option>
        </salesreps>
      </select>
<input type="radio" name="return" id="return" tabindex="12" style="clear:both; width: auto; height: auto; float:left;" onclick="$('remail').value = email@email.com;" />
    <label for="return2" style="width: 170px;">No, I am a new customer</label>
<br />
      <input id="getQuote" name="getQuote" type="image" class="submitButton" src="../Test/GetQuote_Button.gif" alt="Submit" width="75" height="25" tabindex="13" />
      <br />
      <input id="remail" name="remail" type="hidden" class="hidden" value="email@email.com" />
      <input id="service_type" name="service_type" type="hidden" class="hidden" value="Quote Request" />
      <input id="description" name="description" type="hidden" class="hidden" value="" />
      <input id="date" name="date" type="hidden" class="hidden" value="<?php echo date('Y-m-d') ?>" />
      <input id="MM_insert" type="hidden" name="MM_insert" class="hidden" value="selectForm" />
      <br />
  </div>
</form>
<script language="javascript" type="text/javascript">
function toggleRepBox(clicked) {
	if (clicked.checked) {
		$('rep').style.display='block';
	} else {
		$('rep').style.display='none';			
		$('remail').value = $('catSelect').getValue();
	}
}

var sprytextfield1 = new Spry.Widget.ValidationTextField("reqFirstName", "none", {validateOn:["change"]});
var sprytextfield2 = new Spry.Widget.ValidationTextField("reqLastName", "none", {validateOn:["change"]});
var sprytextfield3 = new Spry.Widget.ValidationTextField("reqEmail", "email", {validateOn:["blur"]});
var sprytextfield4 = new Spry.Widget.ValidationTextField("reqPhone", "none", {validateOn:["change"]});
</script>

[PFMaBiSmAd]

When a comparison does not match one of your expected values, echo out the value in an user error message and/or log failed form submission data to a file so that you can see what it actually is. 

[.josh]

When a comparison does not match one of your expected values, echo out the value in an user error message and/or log failed form submission data to a file so that you can see what it actually is. 

 

Lies! That's way too obvious to be true.

[corianton]

Haha, I am really laughing.  I guess I forgot to mention I am a complete novice (well almost).  We don't have an in house guy to do this anymore, as the guy who wrote this (as I am constantly finding) pretty messed up script no longer works here.

 

I think you are telling me to add something to the script to tell me what is going on, why Firefox fails.  If so, what do I need to add to output that kind of message?  Any hints for a complete novice?  Or a link?

 

Sorry to have bothered you superior beings with my apparently laughably basic question.

[.josh]

Job vacancy? Looking for a replacement?

 

just echo the variable in question out to see what's in it.

 

echo $_FILES["attachment"]["type"];

 

 

[PFMaBiSmAd]

 

 

Edit: More verbose version -

 

The problem is that different browsers send different mime types for the same file and your code needs to include all the known possible values and for an unexpected value your code needs to do something to both alert the visitor and make a record for you that something unexpected occurred (logging of errors is beyond the scope of this post and is left up to the programmer to implement as he sees fit.)

 

When dealing with real data in real applications, anything can go wrong and the code should still behave in an expected and controlled manner. In real application code (i.e. bulletproof code that continues to work no matter what type of data or lack of data it receives), almost every if() test should have an else clause to take care of the case where the if() test failed.

 

For the code you inherited, it is lumping together the tests for ["type"] and ["size"] and if either of those conditions fail, the code does nothing to tell you what was wrong or which test failed.

 

For just the portion of the code in question (I'll assume that if a file is not uploaded that you want to skip that portion of the code, same as what it is doing now), the following will at least report the type, size, and move_uploaded_file errors -

 

if (!empty($_FILES['attachment'])){
// create an array of accepted types for easy expansion
$accepted_types = array();
$accepted_types[] = "application/msword";
$accepted_types[] = "application/pdf";
$accepted_types[] = "text/plain";
$accepted_types[] = "text/rtf";
if(!in_array($_FILES["attachment"]["type"],$accepted_types)){
	echo "The supplied mime type: {$_FILES["attachment"]["type"]}, is not supported<br />";
} elseif($_FILES["attachment"]["size"] >= 20000){
	echo "The size of the uploaded file: {$_FILES["attachment"]["size"]}, is greater than permitted<br />";
} else {
	define('UPLOAD_DIR', '/path/to/my/uploads/');
	//$theFile = $_FILES['attachment']['name'];
	$theFile = str_replace(' ', '_', 'tls_file_'.date('Y-m-d').'_'.$_FILES['attachment']['name']);
	if(move_uploaded_file($_FILES['attachment']['tmp_name'], UPLOAD_DIR.$theFile)){
		echo "The uploaded file was processed correctly<br />";
	} else {
		echo "The uploaded file could not be processed<br />";
	}
}
}

 

[corianton]

PFMaBiSmAd , thank you.  That is very helpful.  I'll start there.  Thanks.

 

No, no job vacancy yet.  If I can't fix this there might be!

[corianton]

Ok, so your tip really helped.  I got an error saying a 56 kb msword file was 26739kb.  So for some reason FireFox is reporting that the file I am uploading is way bigger than it really is.  I have no idea why it would do that. Any guesses?

 

Also, the echo's for error reporting you gave me work, but it gives me a "headers already" sent error because later (line 127) it is saying

 

  $insertGoTo = "thank_you.php";
  if (isset($_SERVER['QUERY_STRING'])) {
    $insertGoTo .= (strpos($insertGoTo, '?')) ? "&" : "?";
    $insertGoTo .= $_SERVER['QUERY_STRING'];
  }
header(sprintf("Location: %s", $insertGoTo));
}

 

And it gives an error that the echos above have already started header output.  Is there anyway to fix that?

[glenelkins]

Hi

 

Im not sure why its reporting file size incorrectly...thats wierd!

 

The headers already sent is probably because they were sent when uploading the file... if you place this code at the very top of your document just  after <?php

 

ob_start();

 

that will solve it.

 

PFMaBiSmAd - nice one with the in_array() function, iv never seen that one before and the number of times it would of come in handy is unreal! rather than looping through arrays all the time ...sweet!

 

[corianton]

Ok I'll try that.  Thanks.  Yeah it is EXTREMELY weird, and that i t is FF and not IE that is having issues. . . strange.  I wonder though, if I am limiting the type of files, and most people using my site are just uploading documents, I can probably drop the max size thing.  I wonder if that would do it.  I'll have to try.

[corianton]

I think I just found out what is happening.  FF is reading ">= 20000" as 20000 bytes, and IE is reading it as kb.  I don't know why.  But that is what seems to be happening from my tests.  so I changed it to 21000000 and it works in FF, but now people can upload massive files in IE.  Oh well, I guess as close to a fix as I can hope.

 

And by the way, I tried placing the ob_start(); but it just made it so I don't see the echos at all.

[glenelkins]

ob_start() shouldnt stop anything echoing to the screen. paste your code plz

[premiso]

ob_start() shouldnt stop anything echoing to the screen. paste your code please

 

Umm are you sure?

 

The whole point of ob_start is to not have output echo until you use ob_end_clean type function to paste it to the screen.

[corianton]

What premiso says is what is happening.  It is erasing the echos.

 

http://us.php.net/manual/en/function.ob-get-contents.php 

 

Is this what I need?  And then call it later?  Not sure.

[premiso]

Give it a try and see =) Half the fun of programming is testing new functionality!

[.josh]

ob_start() shouldnt stop anything echoing to the screen. paste your code please

 

Umm are you sure?

 

The whole point of ob_start is to not have output echo until you use ob_end_clean type function to paste it to the screen.

 

no, the whole point of ob_start is to keep the output in a buffer, so that if you have for instance, headers added to the output later on in your script, it will send it in the correct order, so you will not get headers already sent errors.  Unfortunately, people use ob_start as a bandaid to fix those errors, instead of doing the logical thing: reordering their code to begin with. 

 

<?php
ob_start();
echo "blah blah blah";
?>

 

Give it a try and see =) Half the fun of programming is testing new functionality!

[glenelkins]

Unfortunately, people use ob_start as a bandaid to fix those errors, instead of doing the logical thing: reordering their code to begin with.

 

Exaxtly! but it works when you get these errors ha! You should really learn how to manage your headers correctly and you should get the errors in the first place ( headers are annoying! )

[gevans]

Unfortunately, people use ob_start as a bandaid to fix those errors, instead of doing the logical thing: reordering their code to begin with.

 

Exaxtly! but it works when you get these errors ha! You should really learn how to manage your headers correctly and you should get the errors in the first place ( headers are annoying! )

 

Headers are a huge part of programming in php. Used in several different areas for several different purposes. Redirecting, setting output type, altering caching, setting page re validate dates etc...

 

And if used properly they are far from `annoying`