New answers tagged php
2
I don't really like the kind of a review that is popular on this site which only states that your code has some problems but doesn't really offer any solution or improvement.
Given for some problems listed it's really hard to find a plausible solution, I would prefer an answer where author at least tries to provide a solution, so it will be sort of a ...
2
The usage of the database seems pretty secure and standard to me, but I think you knew that. There's no difference, in security, between the usage of ? or :username, both function in the same way, although the latter is clearer, in my opinion.
I would have preferred more checks of the email address. You're probably actually going to use it, for instance to ...
3
Functions used in your code are not deprecated.
However, most of them are highly not recommended in your use case, namely
die() should never be used to report errors. There are much better ways. See this answer on Stack Overflow for the details.
$mysqli->query should never be used to run a query that includes a variable or your site is busted through ...
4
You could use route model binding to remove some lines from your code, but that would require changing the route so it passes the MenuItem's id.
public function update(UpdateMenuItemRequest $request)
public function update(MenuItem $item, UpdateMenuItemRequest $request)
{
$validated = $request->validated();
$item = MenuItem::findOrFail($validated['...
2
For sequences (list, str, tuple, etc), this is rather straight forward:
def chunks(sequence, size):
return [sequence[index:index+size] for index in range(0, len(sequence), size)]
3
You have some odd stuff going on in chunk_with_slice; specifically these lines:
def chunk_with_slice(input_list: List['T'], chunk_size: int) -> List['T']:
if chunk_size <= 0 or not isinstance(input_list, list):
return False
First, you're using strings to represent type vars (List['T']). As mentioned in the comments, strings are used to ...
3
Argument Checking and Duck-typing
Two problems with this code:
if chunk_size <= 0 or not isinstance(input_list, list):
return False
If the arguments are the correct type, the function returns a list of lists. If not, it returns False? This is going to complicate the code which uses the function. It is better to raise an exception:
...
3
I think it looks good, a little verbose. Could be simplified to this
$oldurl = JUri::getInstance();
$bad_urls = ['match1','match2','match3'];
$newurl = str_replace($bad_urls, 'rewritewiththis', $oldurl);
if(in_array($oldurl, $bad_urls))
exit(header("Location: $newurl", true, 301));
If you need to use something like this in many locations, or you need ...
2
PDO has some rather handy fetching options. FETCH_GROUP is good for grouping the first level (product categories), but some extra handling is necessary to group the second level (product name-sizes). To perform this nested grouping, I'll show the implementation of a temporary variable to track whether or not the current iteration is processing a new group ...
0
return isset($_SESSION[$name]) ? true : false; is more succintly written as:
return isset($_SESSION[$name]);
The boolean evaluation of isset() is all you need.
You can use the null coalescing operator to tight up:
if(isset($_SESSION[$name])){
return $_SESSION[$name];
}else {
return false;
}
Into:
return $_SESSION[$name] ?? false;
You never need ...
2
Assuming that you are trying to extract ALL of the rating values from your table, I agree with Unnamed that you should simply GROUP BY and return the Rating, and the COUNT() in the result set.
Two things to add here...
Without apply default values, you need to be aware that Unnamed's query will potentially deliver gaps where there is no count for a ...
6
It would be better to use GROUP by in your SQL. By this way you will get rid of long SQL query.
SELECT Rating, COUNT(id) FROM list WHERE songID=:songID GROUP BY Rating;
1
You are right, but there are better ways to do that.
One way is using dotenv. Trending way for managing configurations.
from dotenv project:
Storing configuration in the environment is one of the tenets of a
twelve-factor app. Anything that is likely to change between
deployment environments – such as database credentials or credentials
for 3rd ...
3
Your script is like a var_export()-var_dump() hybrid with flare (I've not come across highlight_string() before -- pretty cool).
I ran some more tests with a few samples that I had laying around and noticed that your code is having some difficulties accessing the written "resource" branch. I tested with fopen($filename, 'r'); the var_export() evaluation of ...
1
Unless I am missing something in your requirements, I don't think there is a terrific need for the overhead of a class. (I could be wrong)
I do spy some opportunities to ratchet up your code though. Consider these points and if you decide to further develop your script into a class, then this should move you in that direction with some single ...
3
Are there any risks of using function parameters for column names in prepared statement?
Well, of course you shouldn't. "Function parameters" is not a protection measure in any sensible way. It's just a transport, to deliver some value into a function. As is.
Are there some kind of risks of this solution,
ANY of them. All kinds of SQL injection are ...
0
Sorry for poking and old dog but here is an idea, serialization.
This is not about right or wrong but my preferred solution is to, as far as possible, separate data from logic. The complex solution would be to create a class library for a HTML Document Object Model.
But on the other end of the spectrum is the simple solution with regular arrays and a (...
1
You should be validating and sanitizing the variables that are not being parameterized.
When there is a strict set of acceptable values, write an array of whitelisted values and check against that.
If input is coming from a text type input or textarea, then it will be harder to validate, and you may have to settle for stripping html or characters which you ...
1
I don't personally know of any advantage to prefixing php functions with a backslash, so consider removing the unnecessary character. (or tell me why it is useful)
Perform arithmetic once. AKA "Don't Repeat Yourself" (D.R.Y.)
$additionPointsCount = \count($currentContainer->addition_points);
becomes:
$lastPoint = count($currentContainer->...
2
PHP
Constants
The first five variables in db.php could be declared as constants since they shouldn't be changed:
define('DB_HOST', '127.0.0.1');
define('DB_USER', '');
define('DB_PASS, '');
define('DB_NAME, 'doodle');
define('DB_CHARSET', 'utf8mb4');
And notice the prefix DB_ was added to those names to specify that those values pertain to the database. ...
3
w3fools
following a W3 tutorial
I would say the most important suggestion you can get is never ever use any tutorial from w3shcools. They are dubbed as "w3fools" for a reason. Their tutorials are outdated, buggy and - as you can already see from other answers - far from being optimal making you a monkey writing a lot of useless code that repeats again ...
2
You got the SQL part quite well. If you only had been as careful when generating the HTML.
At the very least, you should wrap every bit of output through htmlspecialchars. I'm sure there are better ways, but that's the basic building block, and you should learn it.
function html_println($label, $value) {
echo htmlspecialchars($label) . ": " . ...
3
w3schools gets its placement from shelling out big bucks for high ranking in search engine results (as I've read), not for being the best resource for web development advice. When in doubt, read Stack Exchanges sites. SE sites enjoy the benefit of constant peer review. If something is not quite right, eventually a whistleblower is going to find it and ...
4
You are missing charset in your DSN and you have emulated prepared statements still on. This could potentially leave your vulnerable to SQL injection. See Are PDO prepared statements sufficient to prevent SQL injection?
As I have pointed out to you in an answer on Stack Overflow you should set the proper charset and disable emulated prepared statements.
$...
6
I would limit my review to db.php file only.
First of all, I see you made use of some of my advises already, probably visited some of my articles. Sadly, not all recommendations are followed, but that's OK, we'll review them.
Connection
The connection part is simply OK, nothing to review
Structure
Regarding the structure, I would say this file is a bit ...
1
I have found an inherent flaw in the backup and copy part. Namely
sh "ssh ${SSH_USERNAME}@${SSH_SERVER_NAME} tar -cvpzf ${BACKUP_FNAME} ${DEPLOY_DIR}/*"
sh "ssh ${SSH_USERNAME}@${SSH_SERVER_NAME} rm -R -f ${DEPLOY_DIR}/*"
Which both exand the asterisk on the local mashine, and https://stackoverflow.com/questions/22222838/shell-script-calling-ssh-how-to-...
Top 50 recent answers are included
