-1

I am using a PHP script that outputs an image, what I am trying to do is track when the image is opened on an email, The code works to display the image but does not run the database query and update the count.

The PHP code as follows.

$image = $_GET['image'];

    require_once('connections/site.php');

    mysql_select_db($database_site, $site);
    $query_requests = "SELECT count FROM tracker WHERE id = '1'";
    $requests = mysql_query($query_requests, $site) or die(mysql_error());
    $row_requests = mysql_fetch_assoc($requests);

    $count = $row_requests['count'];

    $newcount = $count++;

    $query_update = "UPDATE count SET count = '$newcount' WHERE id = '1'";
    $update = mysql_query($query_update, $site) or die(mysql_error());
    header("Content-Type: image/jpeg");
    readfile('https://mysite.co.uk/images/'.$image);

Maybe I should be using a different method? I was searching around for a way of tracking a standard image open but I couldn't seem to find a decent method so I thought I would try and cook something up to do this.

AppleTattooGuy
  • 1,145
  • 8
  • 17
  • 39
  • **warning ** your code is vulnerable to sql injection attacks. – Daniel A. White Sep 19 '13 at 13:36
  • What if the email provider doesn't display the images (which is the default on providers like gmail)? – Amal Murali Sep 19 '13 at 13:37
  • 1
    Are you getting any errors on your sql queries? Either way, you should probably just do ``UPDATE tracker SET count = count+1 WHERE id = '1'`` rather than do both a select and update. – Kippie Sep 19 '13 at 13:40
  • The `mysql_*` functions are **no longer maintained** and shouldn't be used in any new codebase. It is being phased out in favor of newer APIs. Instead you should use [**prepared statements**](https://www.youtube.com/watch?v=nLinqtCfhKY) with either [PDO](http://php.net/pdo) or [MySQLi](http://php.net/msqli). – tereško Sep 19 '13 at 23:35

2 Answers2

1

put readfile('https://mysite.co.uk/images/'.$image); at the bottom of your code and add this line just before it:

header("Content-Type: image/jpeg");

Joran Den Houting
  • 3,149
  • 3
  • 21
  • 51
  • Thanks for your answer, I have updated my question to reflect this, I started with that at the bottom and this gives the same results – AppleTattooGuy Sep 19 '13 at 13:40
1

The problem is that readfile reads and directly writes to the output buffer, hence if you're going to use that method, you'll need to move the readfile to the end of your script.

However, there are a few other concerns:

  1. Your script is using a deprecated database API functions (mysql_*) - you really need to read How can I prevent SQL injection in PHP?

  2. You're potentially exposing other files as you're not attempting to validate what's being fetched via readfile. For example, if $_GET['image'] contains ../connections/site.php, your script will potentially output raw (i.e.: un-parsed) PHP files containing sensitive database settings, etc.) See the existing Preventing Directory Traversal in PHP but allowing paths question/answer for more information.

  3. You're not outputting Content-Type, or Content-Length headers, etc.

Community
  • 1
  • 1
John Parker
  • 54,048
  • 11
  • 129
  • 129
  • Thanks for your answer, I did originally have the readfile at the end but that did not work, I have updated the code in my question. – AppleTattooGuy Sep 19 '13 at 13:41
  • @JamesLeist Why are you using a URL with readfile, rather than simply providing the absolute path to the image file on disk? – John Parker Sep 19 '13 at 13:44