views:

476

answers:

5

This one should be easy, I think. I have a paginated image gallery, and under each image is a small link that says "Download Comp". This should allow people to quickly download the .jpg file (with a PHP generated watermark) to their computer.

Now, I know I can just link straight to the .jpg file, but that requires the user to have the image open in a new window, right click, Save As..., etc. Instead, I want the "Download Comp" link to initiate the download of the file immediately.

PHP.net seemed to suggest using readfile(), so each "Download Comp" link is being echoed as "?download=true&g={$gallery}&i={$image}".

Then at the top of the page I catch to see if the $_GET['download'] var isset, and if so, I run the following code:

if(isset($_GET['download'])) {
$gallery = $_GET['g'];
$image = $_GET['i'];
$file = "../watermark.php?src={$gallery}/images/{$image}";
header('Content-Description: File Transfer');
    header('Content-Type: application/jpeg');
    header('Content-Disposition: attachment; filename='.basename($file));
    header('Content-Transfer-Encoding: binary');
    header('Expires: 0');
    header('Cache-Control: public');
    header('Pragma: public');
    header('Content-Length: ' . filesize($file));
    ob_clean();
    flush();
readfile($file);

}

The link takes a lonnnnnnnnng time, and then it brings up a dialog prompt asking you to Open or Save the file, but once you Save and try to open it, it says the file is corrupt and can't be opened.

Any ideas?

+1  A: 
header('Content-Type: image/jpeg');

Perhaps?

Jed Smith
I think the Content-Type header just tells the browser how to deal with it. If the binary data is there, it should work fine regardless of the MIME type.
Marc W
A: 

I think you might need to follow the call to readfile() with a call to exit() to make sure nothing else gets written to the output buffer.

Eric Petroelje
A: 

Also, try file_get_contents() instead of readfile(). I find it works under more circumstances. I would also recommend you use ob_flush() after you output the image data. I've never needed to use ob_clean() or flush() to get this kind of thing to work.

And as Eric said, you may also want to put a call to exit() in there as well for good measure if it still isn't working just in case you are getting some junk data stuck at the end.

Marc W
`ob_clean()` will erase the buffer. It needs to be called before the image is written or the data will be cleared.
mcrumley
My bad. I was thinking of `ob_flush()`. Changing answer to reflect that.
Marc W
+1  A: 

This seems like a security issue.

What if someone enters:

$g = '../../../../../../';
$i = '../../sensitive file at root';

How about making .htaccess (if you are using apache) i for the gallery directory serve jpegs up as a download rather than normal.

easement
+3  A: 

Don't set $file to a relative url. The readfile function will try to access the php file on the server. That is not what you want. In your case it looks like the watermark.php file will send the contents you want, so you could possibly just set up the environment it needs and include it.

<?php
if(isset($_GET['download'])) {
    $gallery = $_GET['g'];
    $image = $_GET['i'];
    $_GET['src'] = "{$gallery}/images/{$image}";

    header('Content-Description: File Transfer');
    header('Content-Type: image/jpeg');
    header('Content-Disposition: attachment; filename='.basename($image));
    header('Content-Transfer-Encoding: binary');
    header('Expires: 0');
    header('Cache-Control: public');
    header('Pragma: public');
    ob_clean();
    include('../watermark.php');
    exit;
}

Another (simpler) way is to modify watermark.php. Add a query parameter to make it send the proper headers to force a download and link to that

<a href="watermark.php?src=filename.jpg&download=true)">...</a>

watermark.php:

<?php
if (isset($_GET['download']) && $_GET['download'] == 'true') {
    header('Content-Description: File Transfer');
    header('Content-Type: image/jpeg');
    header('Content-Disposition: attachment; filename='.basename($src));
    header('Content-Transfer-Encoding: binary');
    header('Expires: 0');
    header('Cache-Control: public');
    header('Pragma: public');
}
// continue with the rest of the file as-is

Also, you don't need the call to flush(). There should not be any output to send at that point, so it is not necessary.

mcrumley
mcrumley - thanks. For now I chose your first option just to get it working, and the file downloads much faster now. However, the file still can't be opened. Preview says "Couldn’t open the file. It may be corrupt or a file format that Preview doesn’t recognize." Any thoughts? The page is here: http://homewoodphoto.jhu.edu//gallery/20030101_test_gallery/
Jason Rhodes
The file I tried was empty. Is the include file path correct? Are you getting any errors in the error log?
mcrumley