Advertisement
If you have a new account but are having problems posting or verifying your account, please email us on hello@boards.ie for help. Thanks :)
Hello all! Please ensure that you are posting a new thread or question in the appropriate forum. The Feedback forum is overwhelmed with questions that are having to be moved elsewhere. If you need help to verify your account contact hello@boards.ie
Hi all,
Vanilla are planning an update to the site on April 24th (next Wednesday). It is a major PHP8 update which is expected to boost performance across the site. The site will be down from 7pm and it is expected to take about an hour to complete. We appreciate your patience during the update.
Thanks all.

Intermittent image upload and database entries! Not sure if it's my PHP or SQL. Help!

Options
  • 21-11-2014 2:40pm
    #1
    Registered Users Posts: 109 ✭✭


    Ok guys, I'm having a problem, and the last time I had one, you guys helped me fixing it. So here's to you guys helping again!


    I'm developing an app, which allows users to leave pictures at lat, long points which their friends pick up when they arrive there. Users can also leave public images at points, where anyone can view them on a map. That's it very briefly.


    A user moves the map to where they want to leave the image, they press the camera button, take the picture, add a caption, press next, then they select whether or not it's going to be left viewable to the public or not, and then who they want to send it to from their friends. We have had public upload working for weeks, and recently got private upload working. It works when they arrive at the location they get the image. This is where the problem exists.


    If they select to upload it publicly, an entry is created in the publicImages table with the URL to the image on our server, the lat and long of the location of the image, the name of the uploader, the address of the location, when it was uploaded and the caption on the image.


    If they select to upload it privately, an entry is created in the privateImages table with the same data as the public images table but then an entry for each user that it was sent to is created in the undiscoveredPrivateImages table with the userID, the imageID, the lat, and the long.


    If they select it as public and private, both of the above are done.


    Sometimes when a user uploads an image, and selects it privately, whether or not it's public also, the image will upload, and be placed in our server. The entry for the private image in the database (such as who sent it, the url of the image on our database, the lat and long of it, the time it was sent, etc.) isn't created, nor is the public one (if it was marked public). The entries for the users and images relationship is however, but the imageid is 0 (probably because it doesn't know what image is for that user at the lat long because the private image entry wasn't created)


    This doesn't happen all the time, and i can't seem to figure out why it is happening when it happens! We have had hundreds of success image uploads when it's public, private, and public/private.


    Below is my php code for handling the image upload and database entries.
    http://pastebin.com/rGR2S66R[1]


    I tried to add in a clause the ID of the image entered is 0 to reenter the values in to the private table, but it doesn't work. The only log in the error log is "PHP Notice: Undefined variable: lastID in /var/www/vhosts/26/301269/webspace/httpdocs/niallw.com/php/imageupload.php on line 54"


    If I can get this problem sorted, I would be so greatful to whoever can help me.


    Thanks guys.

    Niall


Comments

  • Registered Users Posts: 4,757 ✭✭✭cython


    I can't offer a solution, but it looks to me like you may be being overly optimistic about assuming your MySQL queries will succeed every time. I'm not too familiar with PHP or mysqli any more, but from a quick search it looks as though if an error arises in processing the statement, it will return false rather than generating any error, and you never check for this. I would suggest checking the results of the executions of your insert statements for true/false, and if false log the error details. You could have failing inserts into your images tables but be glossing over them at the moment.

    Also, as a slight digression, is there a particular reason you went for separate public and private image tables rather than just having one table with a flag column (e.g. enum) or columns (boolean for each of public/private) on it? It seems like a significant duplication of both data and code to have to insert/update across both tables.


  • Registered Users Posts: 109 ✭✭GWNiall


    cython wrote: »
    I can't offer a solution, but it looks to me like you may be being overly optimistic about assuming your MySQL queries will succeed every time. I'm not too familiar with PHP or mysqli any more, but from a quick search it looks as though if an error arises in processing the statement, it will return false rather than generating any error, and you never check for this. I would suggest checking the results of the executions of your insert statements for true/false, and if false log the error details. You could have failing inserts into your images tables but be glossing over them at the moment.

    Also, as a slight digression, is there a particular reason you went for separate public and private image tables rather than just having one table with a flag column (e.g. enum) or columns (boolean for each of public/private) on it? It seems like a significant duplication of both data and code to have to insert/update across both tables.



    Ah fair enough. Never thought of it like this. I also went ahead an real escaped the captions, which seems to have fixed some bugs.

    In relation to your single table with a column.........We spoke about this, but I thought it would be quicker for large scale queries if we had public and private images seperate. Am I wrong?


  • Registered Users Posts: 4,757 ✭✭✭cython


    I guess it might be quicker (depending on how you do things) in that the data being filtered for each query is already a subset, but it may well mean having to do two queries and aggregation in memory on the application side (often less efficient), or otherwise unnecessary usage of UNION statements to generate resultsets from both. If you never want to display the data from both tables together then this might not be a concern, but ultimately I wouldn't imagine the performance hit from a consolidated table being anything that couldn't be overcome by proper design and indexing on the filtering columns.

    And now that you mention escaping the captions, if that has improved things then it's actually very possible that you could have had some issues with (non-malicious, and accidental) SQL injection inadvertently rendering your statements invalid - again checking for success around the DB calls might have revealed that, even if you just tried to log the failing statement if false came back. This is where prepared statements are often invaluable, as they do a lot of this boilerplate for you.


  • Registered Users Posts: 2,781 ✭✭✭amen


    right I'm not a php/mysql guy but even so your code needs to change. You have way too much duplication, multiple inserts, multiple file writes (saving images), same multiple updates etc.

    You really need to be using some functions that you call to perform the image save, the inserts to the tables, the updates etc.

    You are also not using Transactions which is never a good idea when performing database operations. The way you access the mysql db at the moment part of the mysql operations may succeed and other parts fail leaving the db an indeterminant state ie data in some tables but not others.


  • Registered Users Posts: 6,000 ✭✭✭Talisman


    As has already been suggested I think you should look at the architecture of your application.

    Instead of maintaining two database tables (Public and Private) you should only need a single table - keep things a simple as possible so they are easy to manage and troubleshoot.

    You can add a field to indicate the status of image.
    Value: 1 -> Private
    Value: 2 -> Public
    Value: 3 -> Private and Public

    Users should not be posting data directly to your database for two reasons.
    #1 : Security - the last thing you want to happen is the data to be poisoned.
    #2 : Scalability - how many concurrent uploads will the system be able to handle?

    Instead implement a queue. Upload the image to the server or Amazon S3 and attach a reference to it in a data structure along with the other posted data. A server side service should iterate through the queue and process the scrubbed data. This process will provide the database with an additional layer of protection and also make your application more scalable. The queue structure should also resolve the issue you are encountering with data being lost.

    Ideally the data should be inserted using prepared statements, unfortunately for you MySQLi doesn't support prepared statements but does support parameter binding.

    Your current SQL statement:
    [PHP]$publicSQLQuery = "INSERT INTO publicImages (id, image, caption, lat, lon, address, time, sender) VALUES ('', '$imageURL', '$caption', '$lat', '$lon', '$address', '$timeAndDate','$uploader')";[/PHP]

    A more secure MySQLi method using parameter binding:

    [PHP]$publicInsert = $mysqli->prepare(
    "INSERT INTO publicImages (image, caption, lat, lon, address, time, sender)
    VALUES (?, ?, ?, ?, ?, ?, ?)"
    );
    $publicInsert->bind_param($imageURL, $caption, $lat, $lon, $address, $timeAndDate, $uploader);
    $publicInsert->execute();[/PHP]

    Alternatively using PDO instead of MySQLi to connect to the database gives you the option of using a prepared statement:

    [PHP]$params = array(
    ':image' => $imageURL,
    ':caption' => $caption,
    ':lat' => $lat,
    ':lon' => $lon,
    ':address' => $address,
    ':time' => $timeAndDate,
    ':sender' => $uploader
    );

    $pdo->prepare(
    "INSERT INTO publicImages (image, caption, lat, lon, address, time, sender)
    VALUES (:image, :caption, :lat, :lon, :address, :time, :sender)"
    );

    $pdo->execute($params);[/PHP]

    In your second post you mention fixing bugs by escaping input strings, the PDO::quote() function places quotes around the input string if required and escapes special characters within the string, it uses a quote style that is appropriate to the underlying database driver. PDO takes the headache out of making the decision whether to escape the string or not and gives you a better workflow.

    PDO also gives you the option of switching to an alternative database without having to rewrite chunks of the application. MySQLi only supports MySQL, PDO gives you the option of switching out MySQL for almost any alternative. For example you might decide that PostgreSQL is a better option for your application because PostgreSQL + PostGIS is a fully featured spatial database, MySQL was late to the spatial database party and really doesn't compare.


  • Advertisement
  • Registered Users Posts: 109 ✭✭GWNiall


    Ok guys, thanks a million. My problem was fixed by escaping the string, but you guys have made me realise I need to learn a lot more about PHP and mySQL. I have been told several times to use prepared statements over the past few days, which I didn't know existed until this problem arose, so I will have to look into it and learn more about these.

    I will also look to using functions when possible. Thanks guys, I really appreciate the help. As I said earlier, I'm new to PHP and mySQL but am enjoying learning about it.

    :D


Advertisement