Jacques1

Hi, there's obviously a lot of confusion regarding escaping. Since escaping is also critical for security, you definitely need to understand this before you consider uploading your application. First of all: mysql_real_escape_string() and htmlspecialchars() have absolutely nothing to do with each other and serve two entirely different purposes. The function mysql_real_escape_string() prepares a PHP value in a way that it can be inserted into a MySQL string literal. Let's say you want to insert the following string into the database: The president's daughter Obviously, you can't just take this string and drop it right into a query, because this leads to a syntax error: INSERT INTO whatever (content) VALUES ('The president's daughter') -- this is invalid syntax ; If the PHP value comes from the user, they can even exploit this bug to break out of the string literal and manipulate the query itself (see SQL injection). To prevent this, all special characters like ', " and \ must be escaped by prepending a backslash. This tells MySQL to interpret those characters as literal text rather than MySQL syntax: INSERT INTO whatever (content) VALUES ('The president\'s daughter') -- this is valid syntax; the inner quote is now literal text ; Note that SQL-escaping only works within quoted string literals. If you forget the quotes, you again have a bug which can be used for SQL injection attacks. It's also important to know that manual escaping is extremely error-prone. Programmers have screwed up again and again, and this has lead to a constant stream of security vulnerabilities in web applications. People simply forget the escaping from time to time, or they forget the quotes, or they mess up the character encoding (which can render the function useless). A much better solution is a parameterized statement which separates the data from the actual query. However, this feature is only supported by PDO and MySQLi (I strongly recommend PDO). Whenever you want to insert a PHP value into a query, you must either use a parameterized statement or manually escape the value and wrap it in quotes. Inserting raw values is a bug. You said you're getting literal backslashes in your database when you use mysql_real_escape_string(). This has nothing to do with the function and is caused by some other part of the program or a misconfiguration. If you got some really, really old version with a bad configuration, it could be a problem of “magic quotes”. What does this say:? var_dump(get_magic_quotes_gpc()); The function htmlspecialchars() is an entirely different thing. I guess you should first fix the database issues.

Hi, no offense, but this makes absolutely no sense whatsoever. You're asking for the ID of the last inserted row, but there's no INSERT query in your code. You prepare a SELECT query, but you never execute it. If the method worked, it would return a single number, but then you try to loop over this number as if it was some kind of collection. If the loop worked, it would just overwrite the same variable over and over again. What is $result('id') supposed to be? Pretty much every line is a mystery. Are you sure you've thought this through? Maybe what you actually want is the biggest member ID? That's one line of code: $last_member_id = $this->db->query('SELECT MAX(id) FROM member_data')->fetchColumn();

Hi, I don't really get your question. The activation code already proves that the user has gone through the registration procedure. What more do you need?

What wouldn't work? If you want to map each ID to the title, price etc., just make an associative array: $data = array(); while ($row = mysqli_fetch_assoc($result)) { $data[$row['id']] = array( 'title' => $row['title'], 'pris' => $row['pris'], ); } echo json_encode($data); Now $data looks something like this: array( '42' => array( 'title' => 'some title', 'pris' => '12.90' ), '123' => array( 'title' => 'another title', 'pris' => '32.50' ), ... )

And how exactly are your 100,000 lines of unformatted CSS supposed to help us with the h1 tag? We need the relevant data, not tons of useless stuff. ginerjm specifically asked you about the faulty h1 tag. So where is that?

Hi, stuffing data into a script element is a bad idea, because this easily leads to cross-site scripting vulnerabilities and bugs. Since you're not even trying to prepare the data for the scripting context, it's almost guaranteed to blow up. For example, what if the title contains a double quote? Then obviously the whole JavaScript syntax breaks. Use Ajax to load the data from PHP into JavaScript. As a simple example: data_provider.php <?php // Use the JSON format for the data, because this can easily be parsed by JavaScript. header('Content-Type: application/json;charset=utf-8'); $data = array( 'x' => 'foo', 'y' => 'bar', ); echo json_encode($data); <!DOCTYPE HTML> <html> <head> <meta charset="utf-8"> <title>Ajax test script</title> <script src="http://code.jquery.com/jquery-1.11.1.min.js"></script> </head> <body> <!-- Put this into an external script; inline scripts suck. --> <script> $.getJSON('data_provider.php', function (data) { alert('The value of "x" is: ' + data.x); }); </script> </body> </html>

If you add an ORM, you simply replace the dependency on a DB wrapper with a dependency on a certain ORM interface. Instead of query(), fetch() etc., you now have load(), save() and whatnot. Don't get me wrong, I have nothing against ORMs or ignace's suggestions. ORMs can be a useful abstraction layer in certain scenarios. What I don't like is when they're thrown at “newbies” as the one-and-only solution which absolutely must be used. No, you don't need an ORM for good OOP code. A simple database interface is perfectly fine. You may be flamed for not being cool or enterprisey enough, but those are social matters, not technical ones.

Check your closing braces. You should start using proper indentation and consistent formatting. This will help avoid such errors.

Guys, do you realize that mich2004 is new to OOP and already totally confused? What's the point of insisting on advanced stuff like ORMs? By all means, his code is fine. Yes, there are all kinds of fancy PDO wrappers, ORMs, frameworks and whatnot. But it's not like this would somehow magically improve your code. In fact, I think this has become a fetish where people look down at decent programmers just because plain SQL is no longer considered cool. The only dependency in his code is the dependency on an SQL database. Well, I think he can live with that for now.

No offense, but claiming to aim for decoupling and then going with singletons makes absolutely no sense whatsoever. Singletons are the most extreme form of coupling, because now you have hard-coded references to one specific class everywhere. It's impossible to use your classes with any other database interface – unless you literally go through your entire code and manually change those references. What makes even less sense is that your original approach was actually pretty decent and much better in terms of decoupling. So you're replacing good code with bad code (as to your declared goals and general consensus). What's the point of that? I don't get it.

If you just want to learn PDO, this wiki should help you. Since the old MySQL extension is based on 90s technology, you will have to learn some new concepts like object-oriented programming, the already mentioned parameterized statements or proper error handling with exceptions. But don't worry, it's no rocket science. If you read this very short tutorial and then simply play with PDO a bit, you should understand it very quickly. In fact, PDO is much more intuitive than the old extension. It's just that people have gotten used to copying and pasting the same old code, so anything new looks scary at first.

Hi, do you realize that your script can be used to send arbitrary e-mails to any address? SInce you insert the user input straight into the headers and the body, even the dumbest script kiddie can easily assemble their own message and then use the headers to make your server send it anywhere they want. Once the bots have found the contact form, your server will be blacklisted pretty soon, and then you aren't allowed to send any e-mail addresses. You should keep away from low-level functions like mail(), unless you have in-depth knowledge about SMTP and specific reasons for why you need access to the raw mail content. If you just want to send an e-mail, use a library like PHPMailer.

I just told you that the whole password code is broken. You have two options now: You can keep messing with the broken code and waste some more days in the hopes to somehow get it working – this may or may not happen. Or you can simply replace all the bad stuff with a single function call which will work. It's up to you.

If you find MySQLi easier to learn than PDO, then clearly you're using it the wrong way. You've obviously just added an “i” to each function call in the hopes that this will somehow magically convert everything to MySQLi. It doesn't work like this. If you want to update your code (which is a good idea!), you need to actually rewrite it and get rid of bad habits. For example, values are no longer inserted directly into the query string. This is extremely insecure and has lead to countless of SQL injection vulnerabilities. Instead, you use parameterized statements to securely pass data to queries. Unfortunately, that's when MySQLi turns out to be very complicated and very cumbersome. Take a trivial task like fetching all forum posts from one member in a particular category: <?php /* * Make MySQLi throw an exception in case of an error. Without this, you have to * manually check every single return value to find out if there was a problem. */ $mysqli_driver = new mysqli_driver(); $mysqli_driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; $database = new mysqli('localhost', 'someuser', 'somepassword', 'somedatabase'); $database->set_charset('utf8'); // Use a parameterized statement to securely pass the data to the query. $forum_posts_stmt = $database->prepare(' SELECT forum_post_id, content FROM forum_posts WHERE author = ? AND category = ? '); $forum_posts_stmt->bind_param('ii', $_GET['author'], $_GET['category']); $forum_posts_stmt->execute(); $forum_posts_stmt->bind_result($forum_post_id, $content); header('Content-Type: text/html;charset=utf-8'); while ($forum_posts_stmt->fetch()) { echo '<p>' . htmlspecialchars('Post ' . $forum_post_id . ' says: ' . $content) . '</p>'; } Don't tell me this is easy. The statement alone requires five different methods. PDO is much more straightfoward. You only need prepare(), execute() and a plain foreach loop: <?php $database_options = array( PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, ); $database = new PDO('mysql:host=localhost;dbname=somedatabase;charset=utf8', 'someuser', 'somepassword'); $forum_posts_stmt = $database->prepare(' SELECT forum_post_id, content FROM forum_posts WHERE author = :author AND category = :category '); $forum_posts_stmt->execute(array( 'author' => $_GET['author'], 'category' => $_GET['category'], )); header('Content-Type: text/html;charset=utf-8'); foreach ($forum_posts_stmt as $forum_post) { echo '<p>' . htmlspecialchars('Post ' . $forum_post['forum_post_id'] . ' says: ' . $forum_post['content']) . '</p>'; } In addition to that, PDO isn't limited to MySQL. It's a universal interface for all mainstream SQL database systems. So why use MySQLi? Just because the name sounds familiar?

Why do you dig out a thread from 2009? It might also be helpful to read the manual. Questions like this are exactly why it exists. No, the parameter has nothing to do with cookies. It tells PHP whether or not the old session file should be deleted.

Well, this is one of those famous PHP issues. Always put the application logic to the very top of the script and the HTML to the very bottom. Do not mix logic with markup like you currently do. Once there's output, PHP can no longer send HTTP headers. So your script should look like this: <?php // all the application logic in a single PHP block ?> <!-- Now the HTML, possibly mixed with short PHP blocks -->

I think the lesson should rather be: Turn your friggin' errors on! http://www.php.net/manual/en/pdo.setattribute.php

Well, but that's not what the code does. You check if there are any rows. And if there aren't, you jump to the error handling procedure. In other words, even perfectly valid queries are treated as errors only because they have an empty result set. Yes, exceptions will fix this, because you no longer have to manually check for errors. MySQLi will do this for you. However, if you sticked to your old approach, I guess what you actually wanted is this: <?php $result = $cn->query('...'); // Check the return value: If and only if it's false, there was an error. if ($result === false) { // now the error handling } else { return $result->fetch_all(MYSQLI_ASSOC); } But like I said, this is no longer needed.

The drop-down menu doesn't make much sense, though. You've basically written your own implementation of what the browser would do if this was an actual form. So why not use a form in the first place? I mean, loading another page and passing parameters to the URL is exactly what <form action="graph2.php" method="get"> does. There's absolutely no need to do this yourself with JavaScript. Are you sure this saves you time? It sounds more like you're wasting a lot of time with rewrites, debugging and workarounds. I mean, reloading the entire page whenever the user changes the parameters is hardly the final solution. Isn't the whole point of JavaScript charts that you can reload them dynamically? Well, but this means you'd have to throw away your current code and start over with the Ajax approach.

That's not what I meant. Escaping does not work at all when there are no quotes. The point of mysqli_real_escape_string() is to prevent a value from breaking out of a quoted string. When there is no quoted string, you cannot use this function. Passing around error messages as return values is very messy, cumbersome and unclear. How do you know that there was an error when you call the function? Do you check the data type of the return value? Not exactly a good solution. If you want to convert MySQli errors into your own errors, use exceptions. They allow you to catch errors and throw other errors instead: <?php // make MySQLi throw an exception when there's an error $mysqli_driver = new mysqli_driver(); $mysqli_driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; $database = new mysqli('localhost', 'user', 'password', 'db'); // catch a MySQLi exception and throw your own exception instead try { $database->query('THIS IS NOT VALID'); } catch (mysqli_sql_exception $test_exception) { throw new MyOwnException('This is my own exception. The original error message was: ' . $test_exception->getMessage()); } class MyOwnException extends Exception { } You didn't answer my first question: Why is an empty table an error for you?

On a side note: I find it very refreshing to finally see someone who actually wants to understand the problem and not just patch up a bunch of code. You have a good attitude, keep it up.

If I had a dollar everytime I hear that sentence, I'd be rich by now. I guess I should stop using the word “security”. In the PHP world, this seems to always trigger the same reaction: “Nah, I need no security. Maybe later.” Well, let's call it good code then. Have you considered simply doing it for the sake of correctness and quality, regardless of whether or not this runs in “the entire WWW”? Or is this, again, unthinkable in the PHP world? I'm sorry for the frustration, this is not about you personally. But I see the same pattern over and over again, and I just don't get it. Why do PHP programmers always go with the worst option when given the choice? Why do PHP programmers rather make up excuses for bugs instead of fixing them? Well, I guess that's why the rest of the world laughs about PHP and doesn't take it seriously. You can't really blame them. That might be a good idea.

The code generally doesn't make sense. Why do you regard an empty result set as an error? Empty tables are perfectly normal, there's nothing inherently wrong about them. What's the point of your mysqli_real_escape_string() stuff? Do you realize that escaping only works for quoted values? In your case, it has no effect whatsoever. Why do you use this weird kind of error handling where you return the error message as a string? Why not use real errors as intended by PHP? Frankly, what's the whole point of this function?

Did you run the code through some kind of obfuscator? This is the most incomprehensible thing I've seen in a while.

“Guru” is his user title, not his first name. So I think you can stop calling him “Guru Barand”.