-1

My desired goal here is to allow the uploading of images to the database through PHP, using PDO.

The database is not going to have a lot of values, at most ten to fifteen so I feel more comfortable with allowing the images to be uploaded there.

Here is my PHP code:

<?php 
require '../../app/start.php';
if (!empty($_POST)) {
$name       = $_POST['name'];
$position   = $_POST['position'];
$detail     = $_POST['detail'];
$imageData = addslashes(file_get_contents($_FILES['userImage']['tmp_name']));
$imageProperties = getimageSize($_FILES['userImage']['tmp_name']);
$insertPage = $db->prepare("
INSERT INTO about (name, position, detail, imageType, imageData)
VALUES (:name, :position, :detail, {$imageProperties['mime']}, {$imageData})
");
$insertPage->execute([
'name'      => $name,
'position'  => $position,
'detail'    => $detail,
'imageType' => $imageProperties['mime'],
'imageData' => $imageData
]);
}
header('Location: ' . BASE_URL . '/admin/about/list.php');
require VIEW_ROOT . '/admin/about/add.php'; 
?>

and here is my HTML:

<form action="<?php echo BASE_URL; ?>/admin/about/add.php" method="POST" enctype="multipart/form-data" autocomplete="off">
        <label for="name">
            Name
            <input type="text" name="name" id="name" value="">
        </label>

        <label for="position">
            Position
            <input type="text" name="position" id="position" value="">
        </label>

        <label for="detail">
            Detail
            <textarea name="detail" id="detail" cols="30" rows="10"></textarea>
        </label>

        <label for="imageId">
            Upload Image File:
            <input name="userImage" id="userImage" type="file" class="inputFile" />
        </label>

        <input type="submit" value="Add Page">
    </form>

Table:

id  int(11) NO  PRI     auto_increment  
imageId tinyint(3)  NO              
imageType   varchar(25) YES             
imageData   mediumblob  NO              
name    varchar(35) NO              
position    varchar(35) NO              
detail  varchar(120)    NO              

I'm unsure if the above defines the table. I honestly am not familiar with what you are referring to.

These are the two errors I'm getting when I'm trying to upload the image:

Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: mixed named and positional parameters in /admin/about/add.php on line 24    
Warning: Cannot modify header information - headers already sent by (output started at /admin/about/add.php:24) in /admin/about/add.php on line 26

Using this, I get a couple warnings and the image does not upload.

How can I fix my code to allow the uploading of images to the database?

Edit: I apologize, I previously copy and pasted the wrong html code.

Álvaro González
  • 142,137
  • 41
  • 261
  • 360
YABOI
  • 29
  • 1
  • 6
  • How is your table defined? – Alfabravo Feb 13 '17 at 15:56
  • 5
    If you're database is meant to be kept small, why upload images to it? That will have the opposite effect. That logic went over my head. There are very very few cases where uploading images to a database is a good idea. Images are files, so just keep them in the file system. – M. Eriksson Feb 13 '17 at 15:57
  • @Alfabravo I apologize but I do not know what you mean by that question. – YABOI Feb 13 '17 at 15:58
  • See http://stackoverflow.com/questions/3748/storing-images-in-db-yea-or-nay – Matt S Feb 13 '17 at 15:58
  • 2
    *The database is meant to by kept small as it is not a big company* well in that case you don't upload images on the database you store them in a folder then save their path/url to the database – Masivuye Cokile Feb 13 '17 at 15:59
  • @MagnusEriksson Because I want to display the image together with the text; I believe I may have had poor choice of words. I meant that there is not a lot of data going into the table. – YABOI Feb 13 '17 at 16:00
  • 4
    Rather, store the images in a file location and put the path of the location in the database – George Jones Feb 13 '17 at 16:02
  • You won't save any hassle by saving it in the database. Like other commenters also pointed out, save the file path in the database together with the text instead. You need to do a database query every single time you need to show an image. There's only downsides to this. – M. Eriksson Feb 13 '17 at 16:05
  • If you want to store images in a database and it currently fails, we need to see how did you define the table you're using. Also, follow the advice given by others in comments and save only the path to a folder where you store those files. – Alfabravo Feb 13 '17 at 16:10
  • @MagnusEriksson Okay, I understand; I will look into retrieving the file path then. – YABOI Feb 13 '17 at 16:11
  • @Alfabravo I believe I edited what you're asking for into the main question. I apologize, I am not familiar with the terms. I'll work on retrieving the file path but just for future knowledge and understanding I would like to know how to fix this. – YABOI Feb 13 '17 at 16:16
  • Don't use `addslashes()`, use prepared statements properly... as you do with all other variables. – Álvaro González Feb 13 '17 at 16:16
  • @ÁlvaroGonzález this is not a duplicate of that one! – Alfabravo Feb 13 '17 at 16:18
  • @ÁlvaroGonzález Thank you for the tip, I will look into it later. May I note that my question mentions nothing about preventing SQL injections. – YABOI Feb 13 '17 at 16:19
  • @ÁlvaroGonzález what you just did is wrong, this like many other questions you guys close as duplicate while they are not. this question you marked as a duplicate with has nothing to do with storing images. Please don't abuse your powers. – Masivuye Cokile Feb 13 '17 at 16:21
  • @Alfabravo It's alright. But whatever the problem is (the OP does not even care about sharing error messages) messing binary data with random functions like `addslashes()` is not going to help. – Álvaro González Feb 13 '17 at 16:21
  • @MasivuyeCokile Please note that images are only bytes. Storing a picture in a database is not different from storing other data types, and it's more sensitive to incorrect escaping because it's a binary format. I'm sorry that trying to keep the site useful is considered an abuse of powers. – Álvaro González Feb 13 '17 at 16:25
  • @ÁlvaroGonzález if you need further details, ask for them. Don't abuse your powers in such way! – Alfabravo Feb 13 '17 at 16:30
  • @Alfabravo This is what happens for trying to help. Perhaps I should just downvote silently all around as others do and let this become the new Yahoo! Answers :_( – Álvaro González Feb 13 '17 at 16:36
  • @ÁlvaroGonzález so you're just gonna downvote because i used addslashes() ? I mean, was there anything wrong with how i asked the question – YABOI Feb 13 '17 at 16:39
  • 2
    @MagnusEriksson there are reasons to store images to database and not file system - one reason being load balanced setups that utilize multiple php nodes (and probably one MySQL cluster) so you don't know which node might end up storing the file. Of course, it's possible to rectify this problem using some sort of a network mount, but simply sticking stuff to db can suffice. There's nothing wrong with storing images to the database. In our world of IT, we can't deal with absolutes, like you have just now. Reason is - you can be *very* wrong, like you have been. – Mjh Feb 13 '17 at 16:46
  • @YABOI Nope, I saw you that your usage of prepared statements did not look right and I linked your question to an existing question that already has several useful answers, together with a specific note about your code to help you get started. Another user considered that was wrong, so I removed the link. – Álvaro González Feb 13 '17 at 16:59
  • @ÁlvaroGonzález If I'm not mistaken, the other user thought that marking the post as a duplicate was wrong. I'm not trying to criticize or anything. But I guess it's whatever now.. – YABOI Feb 13 '17 at 17:03
  • @YABOI Yes, that's what I meant with "removed the link" (sorry, that wasn't clear at all). Questions marked as duplicates are linked with the "original", which is pretty useful when you google here. – Álvaro González Feb 13 '17 at 17:10
  • @Mjh If you have such a big application that you need several PHP nodes and MySQL clusters, I would argue that putting the files on an external CDN and just keep the references in the DB would be a much better solution than uploading the whole thing to the DB. – M. Eriksson Feb 13 '17 at 22:33

1 Answers1

2

This

$insertPage = $db->prepare("
INSERT INTO about (name, position, detail, imageType, imageData)
VALUES (:name, :position, :detail, {$imageProperties['mime']}, {$imageData})
");
$insertPage->execute([
'name'      => $name,
'position'  => $position,
'detail'    => $detail,
'imageType' => $imageProperties['mime'],
'imageData' => $imageData
]);

should be

$insertPage = $db->prepare("
INSERT INTO about (name, position, detail, imageType, imageData)
VALUES (:name, :position, :detail, :imageType, :imageData)
");
$insertPage->execute([
'name'      => $name,
'position'  => $position,
'detail'    => $detail,
'imageType' => $imageProperties['mime'],
'imageData' => $imageData
]);

note the change in the VALUES section of the INSERT query. This will fix the error you get. I'm not digging in what you are trying to achieve with this query and with the point that from my perspective it is better to store images in the filesystem and urls in the DB

Lelio Faieta
  • 6,457
  • 7
  • 40
  • 74
  • Using your changes I get a slightly different error code: SQLSTATE[HY093]: Invalid parameter number: parameter was not defined – YABOI Feb 13 '17 at 16:44
  • Are you sure you have posted here the right query? This error means that you have defined a number of params in the query different from the number of items in the array – Lelio Faieta Feb 13 '17 at 16:48
  • I must have missed something. I copy and pasted your changes this time and it worked. Thank you! – YABOI Feb 13 '17 at 16:52
  • you are welcome. Please mark the answer as solved then – Lelio Faieta Feb 13 '17 at 16:53