patternphpMinor
Can I further secure this blog post loading script?
Viewed 0 times
thisscriptcansecurefurtherpostloadingblog
Problem
The following code is developed around a mysql database where the posts table has 3 columns: post_ID, date and title.
config.php holds the values for
The content for each post is held in a directory format:
By querying the database, I retrieve the date, which lets me order the posts (newest first).
The file is called using include within another page which uses exterior formatting.
Whilst, this is outputting my desired result, I would like to know the following:
config.php holds the values for
$dsn, $username, $password, $options.The content for each post is held in a directory format:
year/month/day/(database column)title/content.txtBy querying the database, I retrieve the date, which lets me order the posts (newest first).
$new_date_dir formats the date to serve as the directory for the include link $link.$new_date_vis formats the date to serve as the date for the post.The file is called using include within another page which uses exterior formatting.
setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch (PDOException $e) {
echo 'Connection failed: ' . $e->getMessage();
}
function orderBlogPost($dbh) {
$statement = $dbh->prepare("
SELECT date, title
FROM post
ORDER BY UNIX_TIMESTAMP(date) DESC
LIMIT 0,5
");
$statement->execute();
$statement->bindColumn('date', $date);
$statement->bindColumn('title', $title);
while ($row = $statement->fetch(PDO::FETCH_BOUND)) {
$new_date_dir = date("Y-m-d",strtotime($date));
$new_date_vis = date("d-m-Y",strtotime($date));
$new_row = preg_replace("[-]", "/", $new_date_dir);
$link = $_SERVER['DOCUMENT_ROOT']."/content/".$new_row."/".$title."/content.txt";
echo "".$title."".$new_date_vis."";
include "$link";
}
}
orderBlogPost($dbh);
$dbh = null
?>Whilst, this is outputting my desired result, I would like to know the following:
- Is this procedural code basically secure/usable?
- Can you suggest further reading on refactoring?
- How would I benefit from using OOP in this scenario?
Solution
-
secure?
-
not entirely: you emit a debug message to the user when something goes wrong when connecting to the database which may leak information about the server (consider a dumb
Replace the echo with a forward to a 502 page and log the error message internally.
-
you also
You can use readFile to avoid the php code but not the XSS.
-
usable?
-
Is is that just emits a fixed number of titles in a fixed way. to do anything else you need to write a new function most of it will be copy and paste.
Instead just return an array with date-title pairs and let the calling code figure out how it should be displayed.
also add a parameter to allow the calling code to pass in a parameter for the number of records requested and at what point it should start to allow paging?
Besides that you store a date as a string in the database, let dates be a dates.
In fact the function itself is not necessary with how the code is used.
Make OOP? well you could create a querryable object that will return the aforementioned array and keeps the object handle private.
secure?
-
not entirely: you emit a debug message to the user when something goes wrong when connecting to the database which may leak information about the server (consider a dumb
PDO implementation that emits the $dsn, $options and $username in the error message).Replace the echo with a forward to a 502 page and log the error message internally.
-
you also
include a file from a text file repository and assume it doesn't contain any php code that may do Bad Things™ (while the $dbh is still in scope) or any XSS shenanigans. You can use readFile to avoid the php code but not the XSS.
-
usable?
-
Is is that just emits a fixed number of titles in a fixed way. to do anything else you need to write a new function most of it will be copy and paste.
Instead just return an array with date-title pairs and let the calling code figure out how it should be displayed.
also add a parameter to allow the calling code to pass in a parameter for the number of records requested and at what point it should start to allow paging?
Besides that you store a date as a string in the database, let dates be a dates.
In fact the function itself is not necessary with how the code is used.
Make OOP? well you could create a querryable object that will return the aforementioned array and keeps the object handle private.
Context
StackExchange Code Review Q#67451, answer score: 4
Revisions (0)
No revisions yet.