Jacques1

You know how you can avoid those errors? Proper formatting. No offense, but this is a mess even for a Wordpress template. Indentation and visual structuring isn't just nice-to-have. It's a major part of writing correct code. If you you write a mess, then you first confuse other readers, then yourself and finally the parser. You've reached the last stage.

I've already understood the problem the first time, you don't need to explain it twice. But now it seems you're not embedding the link in your HTML document but rather use it in a header() call for a redirect. Then of course you must not HTML-escape it. As I've already tried to explain above, the whole purpose of htmlspecialchars() is to allow you to embed the URL in an HTML document. If that was never your intention, then why on earth would you HTML-escape it?

There seem to be some general misunderstandings about escaping. You're dealing with two layers here: On the one hand, you have the URL itself with its specific syntax. On the other hand, this URL is embedded in an HTML document which has its own syntax. You start with a valid URL. The http_build_query() function is indeed helpful for this, because it automatically takes care of encoding reserved characters, setting the right delimiters etc. Let's say your result is this: https://example.com/page?foo=bar&qux=quux Now you want to embed the URL in an HTML document (as a href attribute, for example). Using the raw URL would be a bad idea, because this might lead to a cross-site scripting attack. The ampersand is also more or less reserved for HTML entities and shouldn't be used as a literal character. So the URL (the entire URL!) needs to be HTML-escaped before it can be embedded into the HTML document. The result will be something like this: https://example.com/page?foo=bar&qux=quux Note that the “&” entity only affects the HTML parser. Once your browser has interpreted the HTML markup, it “sees” the real URL with its literal ampersand: https://example.com/page?foo=bar&qux=quux All parameters are intact. If they aren't, there's some other problem. What does the URL look like in the browser bar after you've clicked on the link? It's very important to understand those two stages (many people don't). The “&” entitiy does not change or even break the URL. It merely helps the browser parse the HTML document. Once this step is done, you have a literal ampersand again. Also note that you're using htmlspecialchars() incorrectly. If you only give it an input string, how is it supposed to know what this strings means? A string by itself is just a bunch of bytes. To map those bytes to actual characters, the function needs to know the character encoding you're using. ASCII? ISO 8859-1? UTF-8? Something else? Specify the character encoding as the third parameter. It's also strongly recommended that you use the ENT_QUOTES flag so that single quotes will be escaped as well. Otherwise, single-quoted attributes will be entirely unprotected. If you're using UTF-8, you should also specify the ENT_SUBSTITUTE. This will replace faulty byte sequences with a special error character instead of returning an empty string. So you want something like this: htmlspecialchars($input, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');

My advice is: Don't do it. Without expertise and experience in the field of cryptography, you will screw up. What you describe as simple “encryption” of private messages is much more than that: You have to worry about key management, ensuring the integrity and authenticity of the messages, choosing the right techniques, getting all the ugly little details right etc. But what's much worse is that the concept is already flawed: You said that you don't want to be able to read the messages, but at the same time you want to manage the encryption process. That's a contradiction in terms. If you manage the keys, then of course anybody with sufficient access to the server can read the messages. In fact, it's questionable whether the encryption has any benefit at all. Since the keys are basically sitting right next to the ciphertext, an attacker may very well get both, in which case you might as well have stored plaintext. What's the concrete attack scenario you're trying to protect against? If you want to make sure that the message are indeed private, then you have to leave the encryption to the user. For example, they could use GPG to encrypt the message on their PC, paste the ciphertext into the message field, and then the receiver decrypts the ciphertext on their PC again. Your job would be limited to sending plaintext messages around (without any encryption whatsoever).

As I already said above, you need to delete all current subscriptions before you insert the new ones. If the “check engine” light of your car is on, do you solve the problem by turning it off? You need to fix the duplicate entries, not get rid of the error message. The message actually means something, it tells you that you have a bug in your code. And that's indeed the case: You're not deleting the current subscriptions. If you only removed the PRIMARY KEY constraint, then you'd end up with tons of wrong and duplicate subscriptions.

The function doesn't return anything. Besides that, never use SET NAMES in an application. Not sure where you even got this from, but it's a major security vulnerability. This query silently changes the character encoding while PDO still thinks you're using the old encoding. As a result, critical functionalities like escaping may break entirely, because they simply don't know the actual encoding. Even worse: Since you haven't turned off “emulated prepared statements” (PDO::ATTR_EMULATE_PREPARES), this problem will affect you even if you think you're using prepared statements. The character encoding must be specified in the DSN string with the charset attribute. <?php $database = new PDO('mysql:host=localhost;charset=utf8;dbname=SOME_DATABASE', 'SOME_USER', 'SOME_PASSWORD', array( // important: turn off "emulation" of prepared statements PDO::ATTR_EMULATE_PREPARES => false, // turn on exceptions PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // fetch associative arrays by default PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, )); And of course you should never connect to the database as the superuser, except maybe at a very early stage of development.

So what is the value of $connect then? What does your get_connected_db() function look like? Remember that we don't have access to your server, so if you want us to help you, you need to give us the relevant information. Also, please get rid of this terrible try-catch stuff. Do not catch exceptions unless you actually know how to solve the problem. It does not make sense to catch it only to display some meaningless error message on the screen. Now you've lost all the important information like what exactly happened, where it happened and in which context it happened. And what's the point of telling your users that you have issues with your database? They cannot help you with this, can they?

And the problem was what?

The client needs to tell the server the type of the request, just like the server needs to tell the client the type of the response. So you need a Content-Type header in the context array. The type is indeed application/x-www-form-urlencoded.

Instead of manually checking every query for errors, just turn on error reporting in the MySQLi driver: <?php $mysqli_driver = new mysqli_driver(); $mysqli_driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT;

It doesn't help to turn on error reporting after the error has happened. You obviously need to do that before the error, either in the php.ini file or on top of the script. The phpinfo() function tells you where your php.ini file is located.

Numbering the images is only acceptable if all images are public. Otherwise, the ID sequence leaks private information about which picture has been uploaded when (and potentially by whom). So if the images belong to particular accounts and not the general public, you need randomized filenames. However, it's not recommended to use the built-in PHP functions like mt_rand() or uniqid(). Those produce very poor results derived from trivial input like the server time and the process ID. For proper random numbers, you need to use the generator of your operator system. There are several interfaces for this: mcrypt_create_iv(), which requires the Mcrypt extension openssl_random_pseudo_bytes(), which requires the OpenSSL extension reading directly from the system device, e. g. /dev/urandom

Why do you tell PDO that the database system is running on port 80? That's the port for HTTP, i. e. your webserver. The default port for MySQL is 3306. Unless you've changed this setting, just leave out the port parameter. // Actually, this makes no sense at all. “localhost” means that you're connecting via a Unix domain socket. On the other hand, the port indicates that you're trying to connect via a network socket (IP + port). So which one is it?

Several things. I don't see the definition of $member_id anywhere, neither in the subscription part nor in the login() method. But what's much worse is that now you have a major SQL injection vulnerability in your subscription query, because you just drop the raw subs parameter into the query string. An attacker is now able to manipulate the query in any way they want. For example, they could fetch the password hash of the admin and try to break it offline. If your database isn't configured correctly, they may even be able to compromise the entire server. Never insert raw user input into a query string. I'm not even sure why you're doing this. You have a prepared statement right there. All you need to do is add a second parameter for the subscription ID. You also shouldn't constantly create the same prepared statement in the loop. One important property of prepared statements is that they can be reused: You create them once and execute them as often as you want. So create the prepared statement outside of the loop and execute it within the loop. <?php $subscription_stmt = $db->prepare(' INSERT INTO member_feed SET member_id = :member_id , feed_id = :feed_id ') foreach ($_POST['subs'] as $subscribed_feed) { $subscription_stmt->execute(array( 'member_id' => $_SESSION['member_id'], 'feed_id' => $subscribed_feed, )); } Last but not least, you need to delete the subscriptions which are no longer active. The easiest solution is to delete all current subscriptions of the member prior to the INSERT queries.

Why redirect to the error page? Why not simply output the HTML?

Why do you write “member_ID” with uppercase letters? The whole point of renaming the columns was to make them all-lowercase. So it's “member_id”.

I was thinking more about an actual HTML page. The HTML template I gave you in the other thread already has 300 bytes, so the complete page should easily exceed 512 bytes. Otherwise you can simply fill it up with some gibberish in a comment.

I fear you've misunderstood my reply. I did not say that you should create a column for each possible subscription. While this is not quite as bad as stuffing everything into one column, it still leads to major problems: Whenever you want to add, rename or delete an RSS service, you need to change the entire table structure and possibly update a lot of queries. This is a dead giveaway that there's something wrong with the data model. Some queries are extremely difficult to do. For example, you cannot even count the subscriptions of one user unless you hard-code all the possible feeds in the query (which means that the query has to be updated whenever the feeds change). So, no, this is not a solution. Instead, you need an extra table as I've described in my reply. This table assigns users to RSS feeds. For example, if the user #3 has subscribed the feed #2, you insert the row (3, 2) into the table.

Do not store comma-separated values in your database. This violates the First Normal Form (values must be atomic) and makes it practically impossible to do any complex queries. For example, you couldn't even select all users which have subscribed the CNN feed, because that information is buried within a long string. In the relational model, data is stored in rows. So what you want is one table for the users, one table for the various RSS feeds and one table which assigns users to RSS feeds (many-to-many relationship). The latter table would look something like this: user_id | feed_id --------+--------- 1 | 3 1 | 5 2 | 1 3 | 3 You also need to avoid “mixedCase” column names. While this may be pretty, it can lead to a lot of confusion and bugs, because some parts of your application are case-sensitive while others are not. MySQL itself ignores character case, so “memberid” is the same as “memberID”. But when you fetch the columns with PHP, character case does matter, which means it may not be able to find the column. Use the “underscore_notation” instead. As to your questions: 1) When you query the database for all possible feeds, also ask whether the feed has been subscribed by the user. This requires a left join. If you're not familiar with joins, now would be a good time to learn them. Otherwise, just fetch all feeds and all subscriptions separately. When you generate the list of checkboxes, you add the checked attribute to the feeds which are already subscribed. 2) Not sure what you mean. Are you asking how to merge the different feeds into one custom feed? Anyway, I think this can wait until the first task is finished.

Option b) makes no sense for your requirements, because the database file is in the document root. It's the document root of your subdomain. So what's left is a).

Letting people create custom directories in the root folder is a rather bad idea, because now you have to worry about name collisions with actual directories. For example, the name “about” is already reserved for your “about” page. And I'm sure there are many others. It's better to create a separate namespace like “yoursite.com/users/john-doe” or “users.yoursite.com/john-doe”. This would also solve your problem immediately, because you could simply map those URLs to a PHP script which displays the corresponding profile. If you absolutely must have your user folders in the root directory on the main domain, this will be more complicated. Either you hard-code all the “real” folders in your .htaccess and maps the rest to a profile script. Or you set up a front controller which processes all requests and decides what to do based on whether or not the URL was registered by a user.

There's nothing inherently wrong with WYSIWYG editors. The problem is that tend to produce absolutely awful markup which no sane human would ever write. I don't know why that is, but it seems to be an universal rule. Regarding XHTML vs. HTML: Just use plain HTML. Keep away from XHTML unless you know exactly what you're doing. It's a very special, very strict implementation of HTML and has specific requirements. Your document above has nothing to do with XHTML, so it's just incorrectly declared HTML which forces the browser into compatibility mode. A basic HTML template looks like this: <!DOCTYPE html> <html lang="en"> <head> <meta charset="utf-8"> <title>Your title</title> <link rel="stylesheet" href="your_style.css"> <script src="your_script.js"></script> </head> <body> <h1>Some title</h1> <p> Welcome to my site! </p> </body> </html> It's actually the other way round: Without a DOCTYPE declaration, you're forcing the browser into compatiblity mode, which heavily affects the layout. With a proper DOCTYPE, you get the standard mode. If you don't know how to transcode files your editor, you can use Notepad++. You mean “hoster”, not “hotel”.

You don't use a GROUP BY clause at all. The sole purpose of grouping is to apply an aggregate function like COUNT() or SUM() to specific subsets of the result set. No aggregate function, no grouping. In fact, a proper database system wouldn't even let you run the query. This violates the SQL standard in every possible way. Unfortunately, MySQL accepts almost anything, so even the worst mistakes tend to go unnoticed. Dropping the raw user input (like $_GET["aougust_form"]) into the queries also isn't the best idea. You think nobody would attack a day care site? I wouldn't bet on that.

The problem is that the entire code is broken beyond repair. Seriously, what on earth are you doing there? Not only do the queries make absolutely no sense. You've also copied and pasted them around like crazy. And why on earth do you download the entire tables only to count the rows? Never heard of COUNT(*)? I'm not surprised at all that this takes several minutes. You've done everything to make it as slow as possible. Unfortunately, this won't be an easy fix. What can I say? Throw away the script, learn the basics of PHP and SQL and start over. Or find a programmer who's willing to write the code for you.

On a side note: Andy, you should be aware that this so-called “secure login script” has several major security issues. In fact, it's little more than a fancy version of the usual bullshit. If you have any PHP skills whatsoever, you're better off doing some research about security and then writing your own code. It's generally not the best idea to blindly copy and paste stuff you found somewhere on the Internet. This “wikiHow” site may be great for sharing recipes, but it doesn't have any credibility whatsoever with regard to web security.