@programmer Well, first off. You need to take it slow. One way or another, you’re going to have to transition into OOP style. There are tons of things that require OOP such as prepared statements, MVC, .etc.
Now, what I suggest is take each part of your code and work with it. Here are some examples of doing so.
NOTE: This is not your actual codes, this is an example to help you better your codes.
<?php
require_once('connection_db.php');
///////////////////
// Include in DB //
///////////////////
// define("HOST", "localhost");
// define("USERNAME", "username");
// define("PASSWORD", "password");
// define("DATABASE", "database");
// $mysqli = new mysqli(HOST, USERNAME, PASSWORD, DATABASE);
// if($mysqli->connect_error) {
// echo "Please fix your database connections";
// exit();
// }
///////////////////
// End Include //
///////////////////
// All of the above inside the "Include in DB" should be included in your connection_db.php file
// You check to see which method the form was submitted. Don't use GET in this
// Because you don't want people updating your data without any limitation.
// When you allow someone to update and edit something via the $_GET parameter, you allow them to do something like this
// http://localhost/edit_joke.php?joke=Mwahahahahahahahhhah You can't sensor what I can type, I just basically screwed up your database. This isn't even a real number, the joke is set, but the joke is a long sentence that doesn't even exist&submit=Mwhaahahahaha, even more trolling because the "submit" button was set, but this isn't what was suppose to be set.
// When you are updating something in your database, you really don't want to be using $_GET because anyone with the URL can edit it such as the one I just did up there.
if($_SERVER['REQUEST_METHOD'] == "POST") {
$url_id = isset($_GET['joke']) ? $_GET['joke'] : '0'; // You make it so that the URL joke is a digit and not a string of letters
if(isset($url_id)) {
if($url_id < 0) {
// We just use 0 here because in every single database, auto increment only starts at 1. So 0 already indicates something is wrong.
echo "The URL is lower than 0 which means that the URL doesn't exist as 0 in the DB or it isn't even a digit";
} else {
if($_POST['caption'] == "") {
echo "Please don't leave the caption field empty";
} elseif($_POST['content'] == "") {
echo "Please don't leave the content field empty";
} elseif($_POST['author_id'] == "") {
echo "Author ID doesn't exist, it was modified";
} else {
// You shouldn't be trusting user inputs. Not everyone is nice you know. Especially if someone doesn't care if your application breaks or not
// You also don't want to be passing a get variable inside a hidden field. Someone might just use a web dev tool and inspect your page source then change the HTML code
// And they can change the value of the hidden field. So even though the actual ID of the particular joke we're in right now is something like
// 1, someone might modify it as id="adfasdfadfjfajlsdjfa". Then oops, your application is broken. This is what you're going to allow your users to do
// http://xkcd.com/327/
$update_query = $mysqli->prepare("UPDATE joked SET caption = ?, content = ?, author_id = ? WHERE id = ? LIMIT 1");
$update_query->bind_param("ssii", $caption, $content, $author_id, $url_id);
$caption = $_POST['caption'];
$content = $_POST['content'];
$author_id = $_POST['authour_id'];
$_SESSION["message"] = "joke updated.";
header("admin_display_jokes.php");
}
}
}
} else {
// The form was not submitted so this must be just the first step
// You shouldn't be using the * because if someone gets into your database, they can see anything you don't want to show them.
// Just specify what columns you really need
$query = $mysqli->prepare("SELECT id, joke FROM joked WHERE id = ? LIMIT 1");
$query->bind_param("i", $joke_url);
$joke_url = isset($_GET['joke']) ? $_GET['joke'] : '0';
$query->execute();
$query->store_result();
// The row with that ID exists
if($query->num_rows) {
$query->bind_result($id, $joke);
while($query->fetch()) {
// Separating your HTML codes from your PHP codes, this makes it more easy to work with.
// Plus since all of the HTML codes are in one file.
require('html_file.php');
}
} else {
// The row with that ID doesn't exist
// Display your own error handler
}
}
This isn’t your actual full code yet, but you get the point. Some may say that there is no security issues in this topic, but why not use prepare statements when there’s a WHERE clause? Prepared statements are a great way of separating what are actual SQL lines and what are just regular user inputs.
Also, I highly suggest that you don’t use the $_GET parameter if you are trying to update something. The only thing that you should actually be using the $_GET parameter for is when you are trying to retrieve the ID from the database or anything that has to do with retrieving. As you can see with the comments, someone can simply do that and then modify your database without you even knowing. Even worst, they can do something like
edit_post.php?id=2’); DROP TABLE joke;
Just like it showed on Bobby Tables. When you use $_GET, you allow users to modify anything in the URL and if you allow the $_GET parameter to update your data, then you’re allowing them to do anything they want using the $_GET parameter.
Well, if you get a lot of feedback saying you should change your codes, then it’s best to do so. If it’s not just on one site, I would take that as an advice.
