Changing profile picture php

Question

I am creating a social network and I am having a couple of problems .

1. When I click the change profile picture the image goes to the destination and then nothing happens .

2. I think there is a max size for the picture that is uploaded . How do I give a statement that tells the user the image is to big ?

profile.php:

<form id="form2" action="upload.php" method="post" enctype="multipart/form-data">
<p id="p1">Change profile picture:</p> <br />
<input type="file" name="fileToUpload" id="fileToUpload"><br />
<br><input id="sub1" type="submit" value="Change profile picture" name="change"><br />
</form>

<!-- Trigger the Modal -->
<img id="myImg" src="default.png" width="200" height="150">

<!-- The Modal -->
<div id="myModal" class="modal">

<!-- The Close Button -->
<span class="close" onclick="document.getElementById('myModal').style.display='none'">&times;</span>

 <!-- Modal Content (The Image) -->
<img class="modal-content" id="img01">

<!-- Modal Caption (Image Text) -->
 <div id="caption"></div>
</div>
<script>
// Get the modal
var modal = document.getElementById('myModal');

// Get the image and insert it inside the modal - use its "alt" text as a caption
var img = document.getElementById('myImg');
var modalImg = document.getElementById("img01");
var captionText = document.getElementById("caption");
img.onclick = function(){
modal.style.display = "block";
modalImg.src = this.src;
captionText.innerHTML = this.alt;
}

// Get the <span> element that closes the modal
var span = document.getElementsByClassName("close")[0];

// When the user clicks on <span> (x), close the modal
span.onclick = function() { 
modal.style.display = "none";
}
</script>

upload.php:

$target_dir = "images/uploads/";
$target_file = $target_dir . basename($_FILES["fileToUpload"]["name"]);
$uploadOk = 1;
$imageFileType = pathinfo($target_file,PATHINFO_EXTENSION);

// Check if image file is a actual image or fake image

if(isset($_POST["change"])) {

move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file);
$sql = "UPDATE users SET userPic = '".$_FILES['fileToUpload']['name']."' WHERE username = '" . $username . "'";

$check = $conn->query($sql);

if($check !== false) {
echo "<a href = profile.php> Profile pciture has been changed </a>" . $check["mime"] . ".";
$uploadOk = 1;

} else {
echo "File is not an image.";
$uploadOk = 0;
}
} else {

echo"did not change";
}

Answer 1

In your HTML, specifically this line:

<img id="myImg" src="default.png" width="200" height="150">

You just have the image always set to default.png. Unless the posted code is wrong, or I am missing something. It shouldn't be a surprise why that is the only image you see.

You will need to select the image src value from the DB and add it to that tag.

So you will need to:

SELECT userPic FROM users WHERE username = :username

Where :user is the currently logged in users ID. Without more info on what frameworks, or what DB library you are using I can't help you much more than that. For example I have no idea how you save users in the session data after they login, so I have no way to know how you would get the ID for them. I also don't know if you use PDO, MySqli or even what DB you use.

In any case, I would add a few things I think could be improved upon.

• in you user table in the DB for the userPic field. Set the default value to default.png so that when a new user record is added that field will contain the default image.

• Very Important Anywhere you are concatenating input into your SQL, change to use a prepared statement. Otherwise you are asking to get hacked. See Sql Injection attacks.

• Make sure to delete, or overwrite the old images.

• Make sure images uploaded by User A, can't be overwritten if User B uploads an image with the same name.

• I would not use the user provided name for the image. Users can put all kinds of garbage in there. At the very least, replace all special chars and spaces with - and then reduce the -.

Like this:

  //note - remove the extension before doing this.
 $str = 'dirty image name/.*';
 $cleanname = trim(preg_replace(['/[^-\w]/', '/-{2,}/'], '-', $dirtyname),'-');

Outputs:

 dirty-image-name

See it live

I've even had users add quotes to filename, not sure how they did it, but I had a hard time deleting this file on Linux. Spaces can reek havoc on urls. Look at urls, you typically wont see any spaces or other special chars. You should also add the time() to the image, otherwise another user could overwrite an image of another user. So like $imagename = time().'-'.$imagename; Or use the users ID instead, which may be even better.

As I mentioned in the comments, with the right naming convention you could get away without even having to save the imagename in the DB. If you were to save the image like this.

 {$user_id}_profile.png

Then you could check:

 $profileImage = "{$path}/{$user_id}_profile.png";

 if(!file_exists($profileImage)){
      $profileImage = "{$path}/default.png";
 }

 echo "<img src=\"{$profileImage}\" >";

The only difficulty with this system, is changing the images to PNG, But it has the advantage of taking care of the filename, overwriting issues, and saves you from storing it in the DB.

It's not a bad Idea to modify the image and scale it or crop it to the size you need. Here is an image class I wrote, a long time ago. It's not 100% finished, and not really documented. But it does work to the best of my knowledge. I haven't done anything Image heavy in about 5 years. But at the least it should give you some ideas on how to crop, resize, convert images etc..

https://github.com/ArtisticPhoenix/MISC/tree/master/Image

It's a bit outside of the scope of this question to go to deep into that topic, but you're more then welcome to use it, even if just for inspiration.

Happy coding~

UPDATE

You can hash a file many ways,

• You can do hash_file('sha256', $filepath)
• you can hash the name hash('sha256', $filename)

if SHA-256 is to long you can use sha1 instead. Probably even MD5 would work, they are 64,40, and 32 in length. I found SHA-1 to be slightly faster then MD5 (when doing over 10k hashes), and I mean a few hundredths of a second faster.

Hashing the name will be much faster then hashing the contents of the file. File hashing can have some useful applications, but it's probably outside the scope of this question. Basically, you can tell if it's the same file by comparing the hash of it's contents.

In any case, I would add some identifier specific to the user uploading it, like the username:

 $filename = $username.'-'.hash('md5', $filename).$ext;

Outputs something like:

someuser-5d41402abc4b2a76b9719d911017c592.png

Alternatively, you could store all a user's "Stuff" in a folder named after them:

images/someuser

I have even used Zip archives to store a users files, This works extremely well, but adds a bit more complexity to the application, because you have to Zip/Unzip the files when you want to save them and display them. But it keeps everything so much cleaner on the filesystem. Performance wise I haven't noticed any big issues, but I have a very powerful server on Azure ( 54GB of ram )

I mention these for a few reasons:

• User A should never be able to overwrite the content of User B
• When removing a user it's much easier to delete the files if they have a piece of info tied to the user's account. Such as deleting a whole folder or Zip archive named for their account.

There are many other Algo's you can use for hashing, You can get a list with hash_algos() The longer the hash the less chance of having a collision or a duplicate hash for a different plaintext string. You can write a simple loop to get the lengths of them:

   foreach(hash_algos() AS $algo){
       $hash = hash($algo, "hello");
       echo "Algo:{$algo} Hash:{$hash} Len:".strlen($hash)."\n";
   }

You can see this live here

It outputs something like this:

 Algo:md2 Hash:a9046c73e00331af68917d3804f70655 Len:32
 Algo:md4 Hash:866437cb7a794bce2b727acc0362ee27 Len:32
 Algo:md5 Hash:5d41402abc4b2a76b9719d911017c592 Len:32
 Algo:sha1 Hash:aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d Len:40
 Algo:sha224 Hash:ea09ae9cc6768c50fcee903ed054556e5bfc8347907f12598aa24193 Len:56
 Algo:sha256 Hash:2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824 Len:64
 ...
 Algo:fnv1a64 Hash:a430d84680aabd0b Len:16

I've even used fnv1a64 for filename hashing ( length of 16).

You may also wish to store the original name of the file. Typically what I do is setup a table for user_files then I have some fields like this in it

 id - Primary Auto Increment key
 user_id - Foreign key for the user table
 hashname - hashed name of the file ( - the extension )
 filename - original filename
 extension - file extension
 status - status ( deleted | public | private )
 type - type of image ( profile_image, thumb_60, thumb_128 )
 create_date - date the record was created

So I save it on the hard drive using the hashname, but when a user views the name of the file, I give them filename this way it's a name they recognize, without me having to muck with the URL encoding issues associated with raw filenames. A "Psudo" link to a file like this would be:

  <a href="{$imagepath}{$hashname}" >{$filename}</a>

Never store the "Pathname" the full path and filename. Only store the the filename and use code to point to the path. Paths can change over time, and if you store them you may have to update all your DB records when that happens (Or use Symlinks to get your site to work). If you store just the filename, you can just change the code.

It's also important to not put user info into the "path", use the user info to lookup the file, then build the path based off of that. You have to be careful of Directory Transversal attacks.

https://en.wikipedia.org/wiki/Directory_traversal_attack

This is why hashing the filename is a good Idea.

You can also change the collation of the field you save the hash into to be UTF8_bin which is UTF8 binary. What this does is make searching on that field case sensitive. Hashes are case sensitive. This is a little thing that is often overlooked. So, I wouldn't say it's necessary but it's a good practice that shows you know what you are doing. Also the smaller the length of the hash the more important this becomes.

For documentation on this:

https://dev.mysql.com/doc/refman/5.7/en/case-sensitivity.html

If you want a column always to be treated in case-sensitive fashion, declare it with a case-sensitive or binary collation

I know it's a lot to take into account, but really it's worth doing it right.

UPDATE2

As you asked in the comments, you can use uniqid but I would still strongly recommend tying in a piece of info for each user, this can be done by adding a prefix to it.

  $username = 'someuser';
  uniqid($username.'-');

Outputs:

 someuser-5a5a847a96c76

The main reason for this (as I mentioned before) is you need to make 100% sure that one user cannot overwrite another users data. uniqid is not "guaranteed" to make unique values 100% of the time.

http://php.net/manual/en/function.uniqid.php

Warning This function tries to create unique identifier, but it does not guarantee 100% uniqueness of return value.

Cross contamination, from one account to another, should be protected against with the utmost care. Failing to do this, will destroy any "faith" users have in your system's security. It may seem like a miniscule amount of risk, but if it happens would be really bad. I know I would not use a site where my information can be leaking from my account to anther account.