I think this implementation is still lacking improvement vs the previous version.
Image size
First of all, the maximum allowed size for pictures is too large. But the biggest problem is that the pictures are stored in raw form without any optimization. So if a user uploads a picture of 10 Mb straight from their smartphone, then the picture will consume 10 Mb on the server. Since it is a profile picture the ultimate size should be closer to 50 Kb than 50 Mb. Because it should be compressed further and resized. There are massive gains to be made. Standard practice actually. Bonus: resizing the image is a good way to ensure that the uploaded file is actually an image in a proper format.
Speed
The other problem is that pages will be slow to load. Constraining pictures within a frame of 200px by 300px for example will not change anything - the browser will load the picture in full size. As long as you are developing in local mode you won't notice. It's when you go live that the slowness will become apparent.
Even in this day and age, visitors expect pages that load fast, lightning fast. They get impatient with sites that are slow and irresponsive. Bottom line, you are wasting storage space, bandwidth and people's time.
Think about your carbon footprint too. The Internet is largely running on coal. Definitely not green at all.
So, I strongly recommend that you not only resize and optimize but convert all pictures to PNG format on your end to streamline things.
Unique identifier and privacy
My advice is to ditch uniqid
because as already explained it goes not guarantee uniqueness and you are not even requesting additional entropy. The odds of a collision are slim but still. The more data you have, the more there is a chance the "unthinkable" will happen.
If you are using Mysql you can use the UUID() function instead. I don't think PHP has an equivalent built-in function.
I find myself in disagreement with @KIKO Software over one point.
Using an incremented ID for the profile picture is more convenient, but using an incremented or predictable identifier also makes scraping and enumeration of users easier. Probably not something you really want due to privacy considerations - remember how content was scraped massively from Facebook, Linkedin etc. I don't like it either, but this is defensive coding vs convenient coding.
OK, maybe this is not the next Facebook but you have to think ahead and be aware of the future consequences of design choices you are making today.
Constants
Regarding the picture directory, it's hard-coded twice in your piece of code. It is a parameter that should go in a configuration file. So that when you need to migrate files, you won't have to rewrite the whole table in your database and change your code on top of that.
You should use a full path, not a relative path. If you restructure your code, move files to directories etc, the path could become invalid as a result and you will have to debug and rewrite your code. It can be more flexible.
Security
Security-wise the function looks sound. Since the file name is not under the control of the user (potential attacker) it is difficult to interfere with it.
Question: how is $userLoggedIn
set ? You are checking that the variable is not empty, is your assumption well-tested ?
SQL performance
I might change this statement a bit:
$stmt = $con->prepare("UPDATE users SET profile_pic = ? WHERE username = ?");
and use the ID instead of the username as criterion. The ID can be loaded as a session variable along with other data. The assumption is that the username field may not be covered by an index unlike the ID, thus updates will be slower especially on a large table. Check your table structure and run an execution plan to find out more. It goes without saying that there should be a unique constraint on the username.
Type confusion
At the top of your code $errors
is defined as an array:
$errors = [];
The intention being to be able to report multiple errors. But in practice you are only handling one error at a time eg:
$errors = "Only jpeg, jpg, png and gif files are allowed.";
and $errors
becomes a string.
To remain consistent you could do:
$errors[] = "Only jpeg, jpg, png and gif files are allowed.";
And then you can report more than one error at a time, for example "File too large" and "Wrong image format". Currently the previous error is overwritten, if any. As long as the array length is zero it means there is no error.
if(!$errors) {
will work as expected, it could also be expressed with the empty
function.