I have designed a file upload form in PHP where it takes 3 pictures and stores it as a varchar datatype in database and move it to the destination directory.
before uploading or inserting the values to the database i want to make sure that
a) it is of file type image/jpeg
b) the three images should have a different fixed dimensions values (which i am fetching the dimension values from getimagesize(); and validating it)
c) and to check if the duplicate of file name exist in the directory (which i am checking through file_exists())
if any one of the conditions returns false then the script should die()
I have defined the code for it but the problem with my code is, if the last condition returns false then the upper conditions will execute the code and do some operations like move_uploaded_file which i dont want to happen, please have a look at the code
Please check the last 4 if conditions as i want that to reformat.
$w_title = 685;
$h_title = 50;
$w_brief = 685;
$h_brief = 177;
$w_detail = 685;
if(empty($_POST['ns_title']) || empty($_FILES["ns_pic_title"]["name"]) || empty($_FILES["ns_pic_brief"]["name"]) || empty($_FILES["ns_pic_detail"]["name"])) {
echo "<script type=\"text/javascript\">" . "alert(\"Please Fill All the Required Fields\");" . "</script>";
echo "<meta http-equiv=\"refresh\" content=\"0;post-news.php\"/>";
die();
}
else {
$ns_title = htmlspecialchars(strip_tags(mysql_real_escape_string($_POST['ns_title'])));
if($_FILES["ns_pic_title"]["type"] == "image/jpeg" && $_FILES["ns_pic_brief"]["type"] == "image/jpeg" && $_FILES["ns_pic_detail"]["type"] == "image/jpeg") {
$ns_pic_title_loc= $_FILES["ns_pic_title"]["tmp_name"];
$ns_pic_title_name = $_FILES["ns_pic_title"]["name"];
list($width_title, $height_title) = getimagesize($ns_pic_title_loc);
$ns_pic_brief_loc = $_FILES["ns_pic_brief"]["tmp_name"];
$ns_pic_brief_name = $_FILES["ns_pic_brief"]["name"];
list($width_brief, $height_brief) = getimagesize($ns_pic_brief_loc);
$ns_pic_detail_loc = $_FILES["ns_pic_detail"]["tmp_name"];
$ns_pic_detail_name = $_FILES["ns_pic_detail"]["name"];
list($width_detail, $height_detail) = getimagesize($ns_pic_detail_loc);
if(file_exists($ns_target.$ns_pic_title_name)) {
echo "<script type=\"text/javascript\">" . "alert(\"File Already Exists, Please Choose a Different Name for the File\");" . "</script>";
echo "<meta http-equiv=\"refresh\" content=\"0;post-news.php\"/>";
die();
}
if(!$width_title == $w_title && !$height_title == $h_title) {
echo "<script type=\"text/javascript\">" . "alert(\"Incorrect File Dimension for Title News, please make sure it is (685 X 50)\");" . "</script>";
echo "<meta http-equiv=\"refresh\" content=\"0;post-news.php\"/>";
die();
}
else {
move_uploaded_file($ns_pic_title_loc, $ns_target.$ns_pic_title_name);
}
if(!$width_brief == $w_brief && !开发者_Go百科$height_brief == $h_brief) {
echo "<script type=\"text/javascript\">" . "alert(\"Incorrect File Dimension for Brief News, please make sure it is (685 X 177)\");" . "</script>";
echo "<meta http-equiv=\"refresh\" content=\"0;post-news.php\"/>";
die();
}
else {
move_uploaded_file($ns_pic_brief_loc, $ns_target.$ns_pic_brief_name);
}
if(!$width_detail == $w_detail) {
echo "<script type=\"text/javascript\">" . "alert(\"Incorrect File Dimension for Detail News, please make sure it is (685 in width)\");" . "</script>";
echo "<meta http-equiv=\"refresh\" content=\"0;post-news.php\"/>";
die();
}
else {
move_uploaded_file($ns_pic_brief_loc, $ns_target.$ns_pic_brief_name);
}
how do i reformat the code so that
a) it should check all the three condition
b) and if any one of it returns false then it should stop the execution at once
thank you
I would recommend moving your logic to a function to reduce duplication.
Note: I have no way to test any of this, so won't try, but it should get you started.
function fail($error)
{
echo '<script type="text/javascript">alert("' . $error . '");</script>';
echo '<meta http-equiv="refresh" content="0;post-news.php"/>';
}
function valid_image($image, $width, $height = 0)
{
if ($image['type'] != 'image/jpeg')
{
fail('File must be of type image/jpeg');
return false;
}
if(file_exists($ns_target . $image['name']))
{
fail('File Already Exists, Please Choose a Different Name for the File');
return false;
}
list($image_width, $image_height) = getimagesize($image['tmp_name']);
if ($image_width != $width || ($image_height && $image_height != $height))
{
fail('Incorrect File Dimension for ' . $image['name'] .
', please make sure it is (' . $width .
($height ? ' X ' . $height : ' in width'). ')');
return false;
}
return true;
}
if(empty($_POST['ns_title']) ||
empty($_FILES["ns_pic_title"]["name"]) ||
empty($_FILES["ns_pic_brief"]["name"]) ||
empty($_FILES["ns_pic_detail"]["name"]))
{
fail('Please Fill All the Required Fields');
die();
}
if (valid_image($_FILES['ns_pic_title'], 685, 50) &&
valid_image($_FILES['ns_pic_brief'], 685, 177) &&
valid_image($_FILES['ns_pic_detail'], 685))
{
move_uploaded_file($_FILES['ns_pic_title']['tmp_name'],
$ns_target . $_FILES['ns_pic_title']['name']);
move_uploaded_file($_FILES['ns_pic_brief']['tmp_name'],
$ns_target . $_FILES['ns_pic_brief']['name']);
move_uploaded_file($_FILES['ns_pic_detail']['tmp_name'],
$ns_target . $_FILES['ns_pic_detail']['name']);
}
I can offer a few tips:
No need for if ... else
You can simplify the code by reducing nesting.
This is bad
if(cond) {
do this;
die();
} else {
do other things
}
Change above to:
if(cond) {
do this;
die();
}
do other things
If in the above code, all you want to do is to output some text you can further simplify it as
if(cond) die('text to output');
do other things
Check for exception, not conformance
Instead of
if($_FILES["ns_pic_title"]["type"] == "image/jpeg" && ...) {
do stuff
...
Use
if($_FILES["ns_pic_title"]["type"] !== "image/jpeg" || ...) exit;
do stuff
...
Use comments
even if the code is for your personal consumption, use as much comments as you can, virtually no amount of comment is too much.
精彩评论