Hot answers tagged

8

You properly protect against SQL injection, that's good. I worry about the filtering on the password. If I choose <script>' OR 1=1 ">>> as my password, the filtering should not reduce this to just ' OR 1=1 ", just because some part of it looks like HTML tags. A password is not supposed to be displayed anywhere, it is not supposed to ...


5

It will be better practice to url encoding the query string of your $activation_link in case the $fname is not url-compliant. I recommend http_build_query() -- here's a post about the delimiting ampersands. I will also recommend that you not generate this string until you determine that it is actually needed down script. It is good that you are properly ...


5

Although the preface is quite disheartening and basically says "I don't want this code to be improved or evolved into something bigger or better", but we can consider this notion as short-sighted and discard it for sake of providing a good code review. The code is overall more good than bad, especially from the security standpoint. However it could ...


4

Declaring ini_set('display_errors', 1); while in the development stage is fine, but when you move to production, you mustn't be presenting error messages to the end user. if (!$_GET) isn't of much use given the following expression.Just use if (empty($_GET['path'])). ctype_alnum() is locale specific and can possibly let you down. (Here's a demo I prepared ...


3

Why not something simple like this: var filter = Months.SelectedIndex == 0 ? null : $" where month(date_purchased) = {Months.SelectedIndex};" Also that dictionary initialization code is a lot of copy & paste, you could easily write it like this: var allOptions = new Dictionary<string, string>() { ["0"] = "All"...


3

I agree with Anonymous that there isn't much to review here. I would like to point out a few general things. The db connection credentials are the type of thing that I'd put in a config.php file and include it where necessary. $timezone is probably going to be useless in the longterm. date_default_timezone_set() returns a boolean value to inform you of ...


3

Regarding setting up your database connection and error reporting, please refer to this recent post from YCS. I'm noticing that you are unconditionally mutating several of the user submitted values. If you have any concerns that someone might legitimately access this page without submitting these variables, then you should have a condition to determine if ...


2

First of all you are doing great. This code is already above the average. But of course it can be improved. The process is eternal, actually. So don't forget to post the next iteration as well :) The first thing that catches my eye is inconsistent user level error reporting. Why some error messages are shown as is and some are lavishly decorated and sent ...


2

Do people still put MySQL credential into the plain php-files? Yes. Whether it is ok is a different question... Do people still put MySQL credential into the env. variables? Yes. That's probably the preferred way. You shall prevent access to files containing secrets outside of PHP. A PHP script is executed from scratch for every request. You don't want to ...


2

PEP8 Your member variables such as DIMENSIONS, methods such as Draw and arguments such as Dimensions should be in lower_snake_case. Static code These lines: pygame.init() pygame.font.init() should not go in what's effectively the static section of Window. Move them to a function. Hard-coded credentials Do not hard-code these: DBLOGIN = {"host": &...


2

Sadly, the PHP part is neither efficient nor optimal. Apart from already mentioned issues, your function does too much work, which makes it non-reusable and which bloats your code in general. Remember the rule of thumb: each unit of code should mind its own business. While your function interacts with a client through $_POST and echo, connects to a database, ...


1

While it is sensible to some people to precisely specify the cause of the submission failure, I prefer to return compound messages rather than a length error message and a quality error message. I would probably save on screen space (versus piling upto a dozen separate email messages on the user), and merge some responses like: First name is a required ...


Only top voted, non community-wiki answers of a minimum length are eligible