Jacques1

JSON data? I see no JSON data in your script.

There's an example for using external SMTP servers on the PHPMailer page. Download the library, include it as explained in the README and then enter your Gmail credentials like in the example.

It's difficult to make sense of this all. After reading it again, it seems your problem is that you're not allowed to use the mail() function and now look for an alternative way of sending mails. Is this true? In that case, you need access to an external mail relay (e. g. a Gmail account). Do you have that?

Don't use random code you found somewhere on the Internet. Most of it is crap, and that one is no exception. The part about zero security applies here as well. Again: What makes you think that your server doesn't support PHPMailer? This is by far the best solution, and it supports many different PHP versions and many different scenarios. It makes much more sense to figure out this problem rather than jump to some nonsense script from 2009.

I don't see how that answers the question. What I do see, though, is that the script has no security whatsoever and basically allows anybody to use your server as an open mail relay for arbitrary messages. If the spammers find this, your server will be blacklisted in no time, and hosters usually don't find that very funny. The mail() function is generally the worst possible choice, because it's a low-level feature for experts. If I understand you correctly, you used PHPMailer before (which is a much better idea). What's wrong with that?

You can answer that yourself – in fact, you're the only one who can answer it, because we don't know your site. What happens when you request the scripts?

What's the goal of hiding the files? As deathbeam already said, the PHP scripts shouldn't output anything at all, which means there's nothing to hide. And the form is already public, isn't it? The only reason for why you would hide the files is if you insist on a clean API: You don't want “dead” resources which can be accessed with a URL but have no content. In that case, simply store the internal scripts somewhere outside of the document root and adjust the file paths of your include statements. So instead of assuming that all scripts are in the same directory, you'd have to move some directories up.

Using md5_file() as the filename means that the user cannot have two files with the same content. What's the point if that? There are no such restriction in a “real” filesystem. If you want to prevent accidental duplicates, there are much smarter solutions. Also, what happens if the user does upload the same content with a different name? Do you just overwrite the file and change the name? Does the script crash? Do you ask for permission? Last but not least, use pathinfo() to get the extension. No need for any regex magic.

I think you're still confusing collision attacks and preimage attacks. To find a password, you need a preimage for a particular hash. It doesn't help you that somewhere there are two preimages which both map to some other hash. You need this hash. And that's is a preimage attack, not a collision attack. Or maybe you think that collisions are so common that an attacker can choose from a large pool of different strings which are all accepted as a valid password? No. Collisions don't just happen by accident, you need to actually find them. So again: Collision resistance is not relevant for password hashing. This claim is just wrong and leads to the wrong conclusions. You do need collisions resistance for things like digital signatures, but that's an entirely different topic. Right now, we're talking about passwords.

Hardly. Storing an upload with the user-provided filename in some personal directory is the worst possible option: You need to be very, very careful that you don't end up with people uploading malicious scripts. If they can freely choose the filename, nothing prevents them from using a “.php” or “.html” extension. You have to go through complicated uniqueness checks, or you may uncontrolledly overwrite existing files. You force the user to give the files unique names, which is completely unnecessary. Storing the images in BLOBs means that you massively bloat the database and keep it busy with swapping image data back and forth. So as much as I appreciate alternative solutions, I fear those are not good.

No, no, no. Please stop spreading this nonsense. Collision resistance has absolutely nothing to do with password hashing, and I have no idea why this myth keeps popping up in discussions. A collision attack means finding two arbitrary preimages which map to the same hash. That doesn't help us find passwords at all. What we need is a preimage which maps to a particular hash. And that's an antirely different scenario aptly called preimage attack. This may seem like an unimportant detail, but many people actually believe this collision stuff and choose the algorithm based on collision resistance (e. g. SHA-2). That's a huge mistake, because SHA-2 is just as unsuitable for password hashing as MD5. The problem of MD5 is its efficiency, not its issues with collisions. In fact, you may very well use MD5 for high-entropy input like random tokens.

First of all, this is hashing, not encryption. Those are two entirely different concepts, and it's very important to not confuse them. General-purpose hash algorithms like MD5, SHA-1, SHA-2 etc. are inacceptable for password hashing. They're designed for an entirely different purpose (like data integrity), and they're supposed to be very fast. When dealing with passwords, this property makes them extremely vulnerable to brute force attacks. For example, an old gamer GPU can calculate billions(!) of MD5 or SHA hashes per second. If the attacker is willing to pay some extra money for specialized hardware or a cloud computing service, they can have trillions of hashes per second. It's easy to see that even strong passwords won't survive such an attack. This home-made salting scheme also doesn't help at all. In fact, you should never invent your own security algorithms. Leave that to the experts. The crypt() function can theoretically be used to hash passwords. However, there are two big problems: Most algorithms it offers are long obsolete. Most people have no idea how to use it correctly (and appearently don't bother to read the manual). Unfortunately, this applies to you as well: When you just pass some gibberish to the second parameter, you get the extremely weak DES-based algorithm. Even worse, this algorithm only processes the first 8 bytes/characters of the password and ignores the rest. So crypt() is out of question as well. That leaves you with password_hash(), and this is indeed the best solution. It uses very strong algorithms (currently bcrypt), and it's specifically designed for non-experts.

I understand this feeling. I also used to be very sceptical towards new tools: Why use jQuery when I might as well write plain JavaScript? Why use a PHP framework when my oldschool PHP scripts seem work just fine? But it's just a feeling. As you've already pointed out, a lot of your revervations against template engines are purely subjective. If you approach this question rationally and go by hard facts rather than personal opinions, I think you'll come to a different results. I already listed the objective advantages of template engines, so what are the concrete drawbacks? Performance? Show me the benchmarks. Also note that a lot of PHP applications run on the slowest possible Apache-CGI setup on the slowest possible shared host, so maxing out the template performance is simply not a concern for them. Added complexity? Sure, there are some extra scripts in my lib directory. But in practice, that doesn't affect me in any way. The actual API is very simple with minimal overhead. Of course you're free to say: I just don't like template engines. Fine, don't use them. Nobody said you should. But I think the OP wanted to read an objective comparison rather than our personal preferences.

Why is learning new tools such a big problem? A developer should be able to learn and use all kinds of tools, especially in web programming. If they can't, then, frankly, this job is not for them. You're worried about performance? Do you have concrete figures at hand, or is this just a vague feeling? All modern engines cache the templates as plain PHP scripts, which means there's no technical difference between a handwritten PHP “template” and a Twig template after the initial parsing stage. I very much doubt that there's any significant performance loss. But feel free to prove your point. No, no, you don't understand. Template inheritance means that I can create a base template and then override specific parts of it in subtemplates – similar to inheritance in OOP. This is incredibly useful and leads to very clean templates. And that tells us what exactly? That you can throw together some lame tag replacer in 2 minutes? No doubt about that. But I'm talking about a professional template engine. How do you know what I need?

It's definititely recommended to use a proper template engine. (Ab)using PHP for templates comes with a lot of problems: In theory, it may be possible to just separate the PHP logic from the HTML markup, but in reality, it rarely happens. If programmers get the chance to write spaghetti code, they often do, be it because of sloppiness, time pressure, lack of knowledge or whatever. PHP “templates” violate the principle of least privilege. The sole purpose of a template is to generate output, so there's absolutely no reason why it should have the full power of a web programming language. There's no well-defined data interface between the application and the “template”: By default, the “templates” simply have unrestricted access to all global variables and all constants. This can partially be fixed by wrapping the include statements in functions. But then there's still unrestricted access to all superglobals and all constants. PHP is a template engine from the 90s. It doesn't have any modern features whatsoever: no template inheritance, no auto-escaping, no custom tags. The PHP syntax is way too verbose. Compared to Twig, it feels like having to write twice as much. Sure, pure PHP is good enough for small applications and quick hacks. But for anything bigger, use Twig, Smarty or one of the other engines.

The question is: What are you trying to achieve? Why are you afraid of people accessing the scripts? If bad things happen when people execute your scripts directly, then that's the problem. You cannot solve this by hiding them. You need to actually fix the scripts so that they only do what they're supposed to do. For example, a configuration script shouldn't do anything but define configuration values. Calling it directly should have no absolutely no effect. So why worry about it? In fact, it's nonsense to tell people that access to some script is forbidden. What's the point of that? If you don't want anybody to access a resource, then this resource shouldn't exist at all. In other words, the scripts don't belong into the document root. If that's not an option, make your webserver yield a 404 response for all scripts except the public ones. But again: Direct access to a script should never be a problem. The only valid reason for hiding a script is to create a clean API. And this requires a 404 response, not this “forbidden” stuff.

So there are two stages? One image upload and then a separate page for other data? Can't you make that a single page? Then you don't have to mess with temporary data.

Escaping doesn't work like this. The mysqli_real_escape_string() function only works for quoted values. It does not work for identifiers or unquoted values. In that case, it simply doesn't have any effect at all.

Indeed. Storing complex data like lists, JSON objects or whatever in an SQL database is generally wrong, because it violates the First normal form (values must be atomic). I mean, just look at how much code you need for a trivial task like adding a new upload. That should be a single INSERT query, not 20 lines of PHP! And now try to actually do something with the data in MySQL: It's almost impossible. You cannot use a normal query, because all information is mixed into this one string. On top of that, you'll run into big problems when people try to upload multiple files in quick succession. Imagine a situation like this: Request 1 Request 2 ----------------------------------------------------- user uploads file A user uploads file B you read the JSON object and append file A you read the JSON object and append file B you save the new JSON object with file A you save the new JSON object with file B, overwriting the previously updated object -> The new JSON object only contains file B So, no, this is not a good idea. A couple of other things: Why do you re-create all images? You're still relying on the working directory. To get the extension of a file, just use pathinfo($file_path, PATHINFO_EXTENSION). No need for this home-made explode()/end() stuff. You really need to start using prepared statements. Manual escaping is fragile and obsolete. Do not use SET CHARACTER SET or SET NAMES to change the character encoding of the database connection! This doesn't inform the MySQL client about the new encoding, so it thinks you're still using the old one. As a result, critical functions like mysqli_real_escape_string() may break entirely.

WTF? You tried to use FTP to move a local file to a local destination? I think you're very, very confused, MarcinUK.

"The script doesn't upload files" is a bit vague, don't you think? Have you turned on error reporting? What error messages do you get then? Also, do you realize that this script has no protection whatsoever? It uploads any file to any location, which means you might as well print your FTP credentials on the screen.

The thing is that you should only have short, simple control structures within your HTML markup (like loops and if statements). You're doing the exact opposite: You have a big block of PHP code, and then you embed HTML snippets as needed. Of course that's possible. But it goes against the underlying principle, and it's very tempting to put all kinds of application logic into that PHP block. The PHP syntax noise (echo, quotes, semicolons) also makes the HTML markup somewhat difficult to read. But the real problem is that PHP simply is a lousy template engine: The syntax is verbose and fugly, there's no separation of logic and layout, there's no built-in scoping, and there are no modern features like template inheritance. With regard to templating, PHP is basically stuck somewhere in the 90s. Sure, it's good enough for small scripts and demonstrational purposes. But for anything more complex we really should be using a proper template engine like Twig or Smarty.

Yes. Who says the timestamp is unique? Nothing prevents two users from uploading an image within the same second. That's a good thing. This way the user doesn't have to worry about unique filenames, and you don't have to go through complicated uniqueness checks. What about a simple equality check like you do in your second query? It's just not necessary. When you only have one row, you only need one fetch() call.

So you've replaced a bunch of broken crap code with a different bunch of broken crap code. And that helps you how exactly? As far as I can tell, we're back to square one: The stuff doesn't work, and you need help to fix it. Look, you can spend the rest of your life debugging nonsense code you found somewhere on the Internet. But what's the point of that? Wouldn't it make a lot more sense to learn PHP and write your own code? Isn't that the whole point of programming? PHP is no rocket science. If you're willing to learn, you should be able to understand database queries, sessions and password hashing very quickly. I think you should give it a try. It's also much more satisfying to create something yourself rather than end up with “I found some random code on the Internet, and then some random guy fixed it for me”.