Solved

enctype="multipart/form-data" causing validation to fire

Posted on 2016-11-05
16
29 Views
Last Modified: 2016-11-06
This is strange. If I do this:

<form method="post">

Open in new window

my form validation works fine. If I don't select a file to upload I get an error when I try submit the form. If I do select a file then I get no error. Perfect. However, for file uploads I believe I need:

<form method="post" enctype="multipart/form-data">

Open in new window


The issue is now that whether I select a file or not the error message appears. Why is this?

		if(empty($_POST['fileToUpload'])) {
			
			$message .= "Featured image required";
		}

Open in new window


I have a custom bootstrap file field:

<div class="form-group">
<label>Upload Featured Image</label>
<div class="input-group">
<label class="input-group-btn"> <span class="btn btn-danger">
Browse&hellip; <input type="file" name="fileToUpload" style="display: none;" multiple>
</span> </label>
<input type="text" class="form-control" readonly> </div>
</div>

Open in new window

0
Comment
Question by:Black Sulfur
  • 7
  • 5
  • 4
16 Comments
 
LVL 51

Accepted Solution

by:
Julian Hansen earned 300 total points
ID: 41875415
Uploaded files are in the $_FILES array so you would need to look in
$_FILES['filetoupload']

As an exercise point your script to a reflect script - something like that show below - to see what is actually being sent to PHP - it will help get a handle on what is going on behind the scenes
<?php
 header('Access-Control-Allow-Origin: *');  
echo dirname(__FILE__) . "<br/>";
echo __DIR__. "<br/>";
echo "POST\n";
echo "<pre>" . print_r($_POST, true) . "</pre>";
echo "GET\n";
echo "<pre>" . print_r($_GET, true) . "</pre>";
if ($_FILES) {
echo "<pre>" . print_r($_FILES, true) . "</pre>";

  echo "FILES\n";
  echo <<< TABLE
  <table class="table">
    <tr>
      <th>ID</th><th>Name</th><th>Type</th><th>Size</th>
TABLE;
  foreach($_FILES as $id => $file) {
  if (is_array($file)) {
    foreach($file['name'] as $k => $f) {
    echo <<< ROW
    <tr>
      <td>{$id}[{$k}]</td><td>{$f}</td><td>{$file['type'][$k]}</td><td>{$file['size'][$k]}</td>
    </tr>

ROW;
    }
  }
  else {
    echo <<< ROW
    <tr>
      <td>{$id}</td><td>{$file['name']}</td><td>{$file['type']}</td><td>{$file['size']}</td>
    </tr>

ROW;
    unlink($file['tmp_name']);
  }
  }
}
echo <<< TABLE
  </table>
TABLE;

echo file_get_contents('php://input');
if (!empty($_GET['server'])) {
  echo "<pre>" . print_r($_SERVER,true). "</pre>";
}

Open in new window

0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41875418
Erm... Since the input control fileToUpload is not displayed, how does it get populated?  I think we need to see this example in situ  (instead of in fragments).

Have you tried using  var_dump() to visualize $_POST and $_FILES?
0
 

Author Comment

by:Black Sulfur
ID: 41875420
@ Ray, I updated the question just before you posted I think.. If I understand correctly what you were looking for.
0
 

Author Comment

by:Black Sulfur
ID: 41875423
@ Julian, I think I understand. I changed it to:

if(empty($_FILES['fileToUpload']['name'])){
			
			$message .= "Featured image required";
		}

Open in new window


and it seems to work now. Is that what you meant?
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41875424
0
 

Author Comment

by:Black Sulfur
ID: 41875430
I think I got it now.

This is the check:
	if(empty($_FILES['fileToUpload']['name'])){
			
			$message .= "Featured image required";
		}

Open in new window


And this is how to set the variable which I was also struggling with:

$featured_image = $_FILES['fileToUpload']['name'];

Open in new window


Using that seems to insert the record into the database as expected.
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41875457
and it seems to work now. Is that what you meant?
Yes - the reflect script will help you to see the data that is sent up - always helps to be able to visualise the structure - good practice to use it on your forms to see what is actually being sent to the server.
1
 
LVL 108

Assisted Solution

by:Ray Paseur
Ray Paseur earned 200 total points
ID: 41875471
A note about code like this:
$featured_image = $_FILES['fileToUpload']['name'];

Open in new window

What does that do?  Almost nothing, except that it propagates a value in a variable with global scope ($_FILES['fileToUpload']['name']) into another variable with local scope ($featured_image).  Code like that makes the computer science part of my brain say "WTF?"  

First of all, since the $_FILES array is already global, you can just use it directly; you don't need to copy it!

Second, now that you have a separate copy in a local variable, you step closer to the edge of the cliff that has the question, "Which of these variables is the right one?"  In procedural PHP, the assignment of a new variable actually copied the contents of the original variable.  It did not merely create a new pointer to the old variable.  So now you have two variables with the same data, but different locations and different scopes.  We don't do that because it leads to confusion and programming errors.

Third, there are some very well understood design patterns that use filters to sanitize the contents of external variables like $_FILES.  We know that $_FILES is tainted data, and we assign a new name after we have run the data through the filters to be sure it's safe to use.  But if the data is not filtered, merely copied, we have produced the illusion that (perhaps) we filtered the information somewhere else.  This is especially troublesome when you start working with others on a project.  If you see code that refers to $_FILES you know you're dealing with tainted data.  But what about $featured_image?  Is that safe to use in a database query?  Or to send to a client browser?

Sorry if this seems off-topic, but it's pretty basic to the way professional programmers work.  There is no extra credit given for propagating variables.  Fewer variables means fewer chances for design flaws and errors!
0
Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

 

Author Comment

by:Black Sulfur
ID: 41875679
@ Ray,

Thanks for the input. Just to explain a bit, this is the first time I am really working with file fields so it is easier for me to do this:

$featured_image = $_FILES['fileToUpload']['name'];

Open in new window


especially since I am not used to having two together i.e.: ['fileToUpload']['name']

Also, I have 11 fields that need to be entered into the database and I find it much easier to do this:

$stmt->bind_param("siissddsiss", $prod_name, $prod_cat_id, $prod_location_id..............

Open in new window


as opposed to:

$stmt->bind_param("siissddsiss", $_POST['prod_name'],  $_POST['prod_cat_id'],  $_POST['location_id']...........

Open in new window


In the back of my mind I realize that there could be real trouble if file upload functions are not sanitized etc. but I was just so excited that I was finally going to upload a file and insert a record into the database at once. It might sound trivial to you guys but that is totally awesome for me and I just wanted to do it! :)

Now that I have, I will certainly take a look at how to add some security measures to it.

Lastly, I like to do something like this where I can which I think perhaps justifies not just using $_POST['whatever'] but rather declaring a variable. Maybe it isn't necessary, but while I am learning I just find it easier to work with and less confusing when I have to for example, enter 11 fields into the database.
$email = $_POST['email'];
$safe_email = filter_var($email, FILTER_SANITIZE_EMAIL);

Open in new window

0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41875734
Suit yourself, just don't put that code snippet on your résumé!  

Here's a "dictionary picture" of what's wrong:
$email = $_POST['email'];
$safe_email = filter_var($email, FILTER_SANITIZE_EMAIL);

Open in new window

After you run that code, you have not two, but THREE variables, one with global scope and two with local scope.  Only one of them has been filtered, and unless you test the return value in $safe_email, you don't know whether that variable contains an email address or FALSE.  And there is no reason you would you want to put $email into the symbol table at all.

The problem with code like this is that it contains unnecessary parts and loose ends.  One day, an unexpected or uninitialized variable is going to turn up in an expression with the potential for causing a run-time error.  When you've studied the process and practice of programming, you learn that the earliest part of planning your code takes the form of knowing what your inputs are and what your outputs must be.  Getting from inputs to outputs with a minimal amount of variable proliferation reduces the "footprint" of your code in the symbol table and makes it possible to use automated test cases.  This is a large part of the reason for object-oriented design and programming.  It lets us encapsulate our code and data in ways that isolate the parts and prevent unwanted cross-interference.

Anyway, best of luck with the project! ~Ray
0
 

Author Comment

by:Black Sulfur
ID: 41875893
Darn, I didn't know it was that bad!

It just seemed to make more sense to me. Could you possibly show me the right way for these 2 examples and then I will close this out? If you have time.

And if I look at one of your examples from you article, aren't you doing a similar thing by doing this?

$email_address = trim($_POST["e"]);
$email_address = strtolower($email_address);
$safe_email_address = $mysqli->real_escape_string($email_address);

Open in new window


You have used $email_address multiple times so I am just trying to understand what is actually the correct way to do it.
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41875949
You are needing to do a couple of things here
1. You need to make sure there is something in $_POST['email'] - it may or may not exists so you cannot use it on the assumption it does
2. If it does exist you need to check if it is valid.

You can do it in one line
$email = isset($_POST['email']) ? filter_var($_POST['email'], FILTER_SANITIZE_EMAIL): false;

Open in new window

Which is the same as
if (isset($_POST['email'])) {
  $email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL);
}
else {
  $email = false;
}

Open in new window

I prefer the first because essentially what we are doing here is an assignment - which in simple situations is a one line operation. To have assignments spread over 6 lines - especially when you have to do a lot of these - can make for very lengthy and difficult to read code.

After running either of the above $email will either have a valid email address or it will be false.

You can now test for validity by doing an simple if
if ($email) {
  // email is present and valid so we can use it
}

Open in new window

This can be expanded to other post variables where you could end up with
if ($post1 && $post2 && $post3)  {
  // valid input
}
else {
   // at least one field was invalid so deal with it
}

Open in new window


When uploading files things are bit more complex because you have to
1. Check the file is valid
  a) Size is within limit
  b) Mime type is in set of acceptable types
2. Move file to permanent location
3. Potentially rename file using generated name

There could be issues at at either of the first 2 of those which could result in a failure.

A good practice is to have a class or function that handles image uploads that gives a status - which can then be used in the same if statement above.
0
 

Author Comment

by:Black Sulfur
ID: 41875962
Thanks Julian,

So, is my whole approach incorrect? If I give you a simple example, I just want to know why it's wrong so I can understand. For simplicity's sake, lets say I just want to check that a name field is not empty. What I normally do is, check that is isn't empty and if it's not, I put it into a variable so that when it comes time to insert it into a database, it just seems less complicated to use it like that. For example:

if(empty($_POST['name'])) {
			
			$message = "Please enter your name";
		}

if(!$message) {

$name = $_POST['name'];
$stmt = $link->prepare("INSERT INTO `users` (`name`) VALUES (?)");
$stmt->bind_param("s", $name);

Open in new window


So, I would never be able to execute the SQL unless $name had a value. Is this not a good enough way to determine if there is a value in $name?

But it sounds like Ray is saying that I should not do:

 $name = $_POST['name']. 

Open in new window


It just seems less messy when trying to insert into a database to use $name, $email, $whatever instead of

 $_POST['name'], $_POST['email'], $_POST['whatever']

Open in new window


But if I am wrong then I am wrong and I need to correct it. But if I am going to correct it I need to understand it and nip it in the bud now.
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41876048
... seems less messy ...
This is why we use frameworks and Active Record design patterns.  The messy parts get abstracted away and we don't have to look at the validation code because it becomes part of the woodwork.  And if it's all wrapped inside a function (aka class method) it can't pollute other scopes with unwanted variables.  You can just write something like filter_post_input() and your custom filter_post_input() function will make all your troubles go away, leaving you with a sanitized data set and no side-effects in the global scope.

The notion of scope exists in all programming languages, and it's a little bit different in each of them.  Here is PHP's take on scope.
http://php.net/manual/en/language.variables.scope.php
0
 

Author Comment

by:Black Sulfur
ID: 41876088
Thank you both. I will just have to keep practicing and make an effort to get it right.
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41876167
The pattern I use when processing forms is to name all my controls as an array
<input type="text" name="data[firstname]" />
<input type="text" name="data[surname]" />
<input type="text" name="data[email]" />

Open in new window

That way when the data arrives I do this
$data = isset($_POST['data']) ? $_POST['data'] : false;

Open in new window

Now if $data is false - then we did not arrive here by a form submit - otherwise I validate it.

Validation can now be processed with a loop and a rules array

$errors = validate($data, $rules);

Open in new window


Now if $errors is non-empty I know validation failed - in addition the validate() function returns an array of error messages with the same index as the data in the $data array - so now I can do two things

1. If I am regenerating the form to display the form with errors - I simply use the data index to pull the current value and the message
2. If it is an ajax request I json_encode the array and send it back to the form.

Validate rules can be regular expressions passed in an array with error messages for each data field
Example:
$rules = array(
   'firstname' => array('regx' => '/^[a-zA-Z]{3,50}$/', 'msg' =>  'A firstname > 2 characters and < 50 is required'),
   'surname' => array('regx' => '/.+/',  'msg' => 'A surname is required')
);

Open in new window

Validation would then be something like this
function validate($data, $rules)
{
   $error = array();
   foreach($rules as $id => $r) {
      // If there is no element in the data array for this rule
      // or the data element fails the RegEx then we have an error
      if (!isset($data[$id]) ||!preg_match($r['regx'], $data[$id])) {
        $error[$id] = $r['msg'];
      }
   }

   return $error;
}

Open in new window

1

Featured Post

How to improve team productivity

Quip adds documents, spreadsheets, and tasklists to your Slack experience
- Elevate ideas to Quip docs
- Share Quip docs in Slack
- Get notified of changes to your docs
- Available on iOS/Android/Desktop/Web
- Online/Offline

Join & Write a Comment

Suggested Solutions

Title # Comments Views Activity
how can I dected if URL has variable? 7 23
Creating a slider 12 34
Not needed 13 55
How Can I Use otf Custom Font with TCPDF 7 6
Part of the Global Positioning System A geocode (https://developers.google.com/maps/documentation/geocoding/) is the major subset of a GPS coordinate (http://en.wikipedia.org/wiki/Global_Positioning_System), the other parts being the altitude and t…
Nothing in an HTTP request can be trusted, including HTTP headers and form data.  A form token is a tool that can be used to guard against request forgeries (CSRF).  This article shows an improved approach to form tokens, making it more difficult to…
The viewer will learn how to look for a specific file type in a local or remote server directory using PHP.
This tutorial will teach you the core code needed to finalize the addition of a watermark to your image. The viewer will use a small PHP class to learn and create a watermark.

757 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

19 Experts available now in Live!

Get 1:1 Help Now