Download script inserting line feeds into end of downloaded file

General PHP Help
#1

I have a download script that inserts data into a database table when someone downloads a file. For some reason, the insert is adding several linefeeds to the end of the file being downloaded even if I comment out the database code. If I remove the insert code everything works as expected. Can someone see what I’m doing wrong?

<?php

$php_scripts = '../../php/';
require $php_scripts . 'PDO_Connection_Select.php';
require $php_scripts . 'GetUserIpAddr.php';
function mydloader($l_filename=NULL)
{
$ip = GetUserIpAddr();
if (!$pdo = PDOConnect("foxclone_data"))
{	
    exit;
}
    if( isset( $l_filename ) ) {       
        header('Content-Type: octet-stream');
        header("Content-Disposition: attachment; filename={$l_filename}");
        header('Pragma: no-cache');
        header('Expires: 0');        
        readfile($l_filename); 
        $stmt = $pdo->prepare("INSERT INTO download (address, filename) VALUES (?, ?)");
        $stmt->execute([$ip, $l_filename]) ; 
        }
    else {
        echo "isset failed";
        }  
}

mydloader($_GET["f"]);

Thanks in advance

#2

The symptom of adding extra characters to the beginning/end of a downloaded file is typically caused by those characters existing before/after the php tags in your main script or characters from any required file and the symptom would not change due to any change made inside the php code unless something was being echoed by the php code.

Is the above code everything in the main file, particularly what comes after the mydloader($_GET[“f”]); call and what if anything is after the last closing php tag ?>, and what are the exact versions of the code where you commented out the database code and where you removed the insert code?

I suspect you have a number of new-lines after your closing php tag ?> in the main file and through the edits you have made they have been added or removed from the various versions of the code, resulting in one version of the code, without them, that worked.

BTW - you should NOT accept the raw filename as an input (I would use ids, then query for/lookup the actual filename), without validating it to make sure it is exactly and only a permitted filename. The current code will allow ANY file within your hosting account to be specified and downloaded, such as the file holding your database connection credentials. All external data can be set to anything, cannot be trusted, and MUST be validated before using it.

#3

Thanks for your reply. This code is the entirety of mydloader.php. It is called from the main script (index.php) by a button click. That code is as follows:

          <?php
            $isos = glob('download/*.iso');
            $iso = $isos[count($isos) -1];
            $isoname =  basename($iso);
            $isodown = "/".$isoname;
            $md5file = md5_file($iso);
           ?>

             <div class="container">
                <div class="divL">                       
                    <h3>Get the "<?php echo "{$isoname}";?>" file (approx. 600MB)</h3>
                     <center>   <a href="download/mydloader.php?f=<?php echo $isoname;?>"><img src="images/button_get-the-app.png" alt=""> </a><center>

There is additional code in index.php to download other files.

re: your security concerns: Credentials to the database are stored in a .php file outside of the public_html folder.

#4

Sorry for not including the code with the database insert code in my last response. Here it is:

<?php

$php_scripts = '../../php/';
require $php_scripts . 'PDO_Connection_Select.php';
require $php_scripts . 'GetUserIpAddr.php';
function mydloader($l_filename=NULL)
{
$ip = GetUserIpAddr();
if (!$pdo = PDOConnect("foxclone_data")):
  {
	echo "Failed to connect to database" ;
    exit;
  }
else:
  {
        header('Content-Type: octet-stream');
        header("Content-Disposition: attachment; filename={$l_filename}");
        header('Pragma: no-cache');
        header('Expires: 0');        
        readfile($l_filename); 

        $stmt = $pdo->prepare("INSERT INTO download (address, filename) VALUES (?, ?)");
        $stmt->execute([$ip, $l_filename]) ; 
  }

endif;

}
mydloader($_GET["f"]);
?>
#5

Doesn’t matter. I can send a get parameter of

?f=../../php/pdo_connection_select.php

to the script and it will happily readfile() it and download it to me.

#6

I didn’t realize that. Thanks for pointing that out.

#7

In the case of PDO_Connection_Select.php, how would I query for it before I have a connection to the database?

#8

You aren’t trying to do that. As part of the processing the code is doing, you would be querying to find if the requested download file IS a permitted downloadable file.

My suggestion of - “I would use ids, then query for/lookup the actual filename” assumes that you have stored the downloadable file information in a database table, with columns for an id (auto-increment primary index), filename, title, description, and possibly a category_id. You would then build the download links with the id on the end of them, not the filename. The download code would then use the submitted id, through a SELECT query, to get the actual filename that the rest of the download code uses. By doing this, the ONLY files that can be downloaded are ones that exist in the database table and match a submitted id value.

Since you apparently don’t have this downloadable file information stored in a database table, why not? You are missing out on having titles, descriptions, organizing the files by categories, and performing keyword searches on these fields.

#9

The files get updated almost monthly, with only the version number changing. I could create a table to store the 5 base filenames with an " *.ext" on the end and an id number. The updated files are uploaded to the website via FTP, so no processing.

Back to my initial question, why is the download script inserting LF’s into the downloading file?

#10

The characters are being output by the download script. Since they are on the end of the downloaded file, it’s something after the readfile() statement, either in the php code or after the closing ?> php tag.

That the symptom changed when you edited the file, it’s likely characters outside of and after the php code.

How are you viewing the downloaded file to see these extra characters? I would bet that if you scroll to the right-end of the last line of actual data in the iso file, that you will see a php error message as part of the file, followed by the new-lines that you are mentioning.

The posted code has changed from using php’s alternate conditional logic syntax, with the : on the end of the statements, including extra {} which as not used with that syntax, to using the original conditional logic syntax. I’m not sure what mixing the two styles of syntax would do. Did these extra characters in the downloaded file occur at the same time you made the change in the php conditional logic syntax?

#11

I modified the script as follows and it’s working properly. Seems that having the database insert code after the download code might have been the problem or not having an exit at the end, not sure which.

$php_scripts = '../../php/';
require $php_scripts . 'PDO_Connection_Select.php';
require $php_scripts . 'GetUserIpAddr.php';
function mydloader($l_filename=NULL)
{
$ip = GetUserIpAddr();
if (!$pdo = PDOConnect("foxclone_data"))
{	
    exit;
}
    if( isset( $l_filename ) ) {  
        $stmt = $pdo->prepare("INSERT INTO download (address, filename) VALUES (?, ?)");
        $stmt->execute([$ip, $l_filename]) ;         

        header('Content-Type: octet-stream');
        header("Content-Disposition: attachment; filename={$l_filename}");
        header('Pragma: no-cache');
        header('Expires: 0');        
        readfile($l_filename);
        }
    else {
        echo "isset failed";
        }  
}
mydloader($_GET["f"]);
exit;

Thanks for the help.

Sponsor our Newsletter | Privacy Policy | Terms of Service