When user tries to download .iso, tar.gz, or .deb from my website php is opening the file instead of downloading it

[foxclone]

I'm having a problem with a download script. It runs fine on my local apache server, but when I put it on the my webhost's apache server, it goes crazy. Here's the script:

<?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 "unable to connect";
          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);

      }
        
    else {
        echo "isset failed";
        }  
}
mydloader($_GET["f"]);
exit;

When I say "it goes crazy", I get errors saying that the header values have already been set by apparently by the PDOConnect. Then it actually reads the file. The files being downloaded are .iso, .deb, and .tar.gz. This isn't a php.ini problem; the files are the same on my local machine and the web host. Here's a screenshot:
 

 

Does anyone have any idea why this is happening?

Edited March 1, 2022 by foxclone

[gw1500se]

Headers must be first. There must be some text or white space being output before the headers. Make sure there is not even a blank line before the <?PHP tag. Check the HTML source to see it then find where it is coming from.

[ginerjm]

Since the error is in the connection script how about showing us that?  (hide the credentials if present)

PS - that connect script should not be sending anything out.  If you have a message to be sent either exit right there or return it to the calling script and let that script respond (and exit).

Edited March 1, 2022 by ginerjm

[benanamen]

Your "downloader" doesn't do anything. Why not just link directly to the files?

You create a database connection that serves no purpose whatsoever. Your $ip is a variable for nothing and does nothing.

[ginerjm]

Actually the readfile line would generate some output, no?  You are correct about the db code tho.

[foxclone]

Here's the full code, including the reason for the db connection.

<?php

error_reporting(E_ALL);



ini_set('display_errors', '1');

$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 "unable to connect";

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);



$ext = pathinfo($l_filename, PATHINFO_EXTENSION);

$stmt = $pdo->prepare("INSERT INTO download (address, filename,ip_address) VALUES (?, ?, inet_aton('$ip'))");

$stmt->execute([$ip, $ext]) ;



$test = $pdo->query("SELECT lookup.id FROM lookup WHERE inet_aton('$ip') >= lookup.ipstart AND inet_aton('$ip') <= lookup.ipend");

$ref = $test->fetchColumn();

$ref = intval($ref);



$stmt = $pdo->prepare("UPDATE download SET lookup_id = '$ref' WHERE address = '$ip'");

$stmt->execute() ;

}

else {

echo "isset failed";

}

}

mydloader($_GET["f"]);

exit;

Here's the code for the db connection:

<?php

function PDOConnect($l_dbname=NULL, $l_msg=null, $l_options=null)  {

if ($l_options == null)

{ // set my default options

// echo "db name is '$l_dbname'";

$l_options = array(PDO::ATTR_EMULATE_PREPARES => false,

PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,

PDO::MYSQL_ATTR_FOUND_ROWS => true,

PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',

PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC);

}

if ($l_dbname == null)

$host="mysql:host=localhost;charset=utf8";

else

$host="mysql:host=localhost;dbname=$l_dbname;charset=utf8";

$uid = "xxxxxxx";

$pswd = "xxxxxxxxxxx";

try   {

$mysql = new PDO($host, $uid, $pswd, $l_options);

}

catch (PDOException $e)   {

if (strtoupper($l_msg) == "SHOWMSG")

    echo "Fatal Error<br>Failed to connect to mysql via PDO.<br>PDO Error msg is:<br>".$e->getMessage();

else

     echo "Fatal Error<br>Possible bad dbname?<br>Failed to connect to mysql via PDO. Sensitive error msg may be viewed with additional parm to call to

     PDOConnect(dbname,'showmsg')".$e->getMessage();

     return false;

}

if (!$mysql)

    return false;

else // all worked - return handle to pdo connection.

     return $mysql;

}

 

 

[gizmola]

One pro tip:

Never have a php end tag (ie. ?> ) in your scripts.  It is not needed, and can lead to bugs like this  remove all php end tags whenever they are the last thing in a .php file.

You can start with looking at PSR-1, and perhaps graduate to PSR-12 if you are looking for some standards to help guide you.

[benanamen]

I would suggest you start with the most minimal code needed to create a DB connection and start from there.

Edited March 1, 2022 by benanamen

[gizmola]

You don't need to exit from a php script.  exit and die are the same function.  You use them in functions/methods.

[ginerjm]

OK - the pdo connection logic (mine!) is showing nothing wrong.  

As for the mainline code - I don't see any problems other than your use of this:  inet_aton('$ip')

Is the "inet_aton" variable an array?  If so you are using the wrong syntax.  Indices need to be wrapped in [] not parens.

Tip: Don't bury a query statement inside a function.  Assign it to a variable and use that var in the function call.  That way if you need to dump the query to debug it you can simply echo it.

Here's how I would do it:

error_reporting(E_ALL);
ini_set('display_errors', '1');
$php_scripts = '../../php/';
require $php_scripts . 'PDO_Connection_Select.php';
require $php_scripts . 'GetUserIpAddr.php';
//*******************************
if (!$pdo = PDOConnect("foxclone_data"))
{
	echo "unable to connect";
	exit;
}
$result = mydloader($pdo, $_GET["f"]);
if ($result === true)
	echo "Success";
else
	echo $result;
exit;
//***********************************
function mydloader($pdo, $l_filename=NULL)
{
	$ip = GetUserIpAddr();
	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);
		$ext = pathinfo($l_filename, PATHINFO_EXTENSION);
		$stmt = $pdo->prepare("INSERT INTO download (address, filename,ip_address) VALUES (?, ?, inet_aton('$ip'))");
		$stmt->execute([$ip, $ext]);
		$test = $pdo->query("SELECT lookup.id FROM lookup 
				WHERE inet_aton('$ip') >= lookup.ipstart AND 
					inet_aton('$ip') <= lookup.ipend");
		$ref = $test->fetchColumn();
		$ref = intval($ref);
		$stmt = $pdo->prepare("UPDATE download SET lookup_id = '$ref' 
					WHERE address = '$ip'");
		$stmt->execute();
		return true;
	}
	else 
	{
		return "No filename provided";
	}
}

Questions

Why are you reformatting the 'id' from one table to the other?  Makes it hard to later connect/compare them.

What is the inet_aton variable?  You are not passing to the function so you should be getting an error message I think.

I moved the connect call outside of the function and passed the handle as a function parameter.

I added a result to the call to your function and eliminated the buried exit.  And I moved the function to the bottom instead of the top.  That way when one looks at the script they can see right away what is trying to accomplish (which I still don't see) without having to browse thru a lot of code.

And I removed all of the excess spaces.  No need for them. As well as the extra blank lines.  That may have been caused by your editor when you uploaded it to here.

Edited March 1, 2022 by ginerjm
fixed a cpl lines

[foxclone]

Thanks to everyone who responded. Removing blank spaces and moving the db connection code after the headers works on both my server and the web host. 

@gizmola - thanks for those links. I'll study them and hopefully, do better coding.

[ginerjm]

Actually neither of those changes should have mattered.  It was just the proper way to code (IMHO). There must of been another change too.

HTH!   And glad to see my code being used.

[benanamen]

@ginerjm, inet_aton is a mysql/mariadb function and is for properly storing/retrieving IP addresses in the DB.

https://mariadb.com/kb/en/inet_aton/

See also

https://mariadb.com/kb/en/inet_ntoa/

https://mariadb.com/kb/en/inet6_aton/

https://mariadb.com/kb/en/inet6_ntoa/

Edited March 1, 2022 by benanamen

[foxclone]

5 minutes ago, ginerjm said:

Actually neither of those changes should have mattered.  It was just the proper way to code (IMHO). There must of been another change too.

HTH!   And glad to see my code being used.

Other than moving that block of code, I did nothing else. Your code had served me well, thanks for posting it!

[gizmola]

I want to reiterate:  

exit is only for immediate exit from a script. You should not be using it here.  When a script ends it exits and cleans up everything.  Don't get in the habit of adding exits when you don't need them.

You use exit like you otherwise might use an exception.  Mostly you use it for debugging sessions (as die('something') or exit('something') only to remove it when you have figured out your issue.

The only exception to that rule is in the case of a header('Location: ...) call.  Since that is meant to redirect the browser (yet still requires the browser to act upon the redirect) it is good practice to follow a header('location....) with an exit.  

 

As to your problem it is being caused either by an echo, error or some output from the PDO_Connection_Select.php script.  Make sure you don't have a php end tag in that script.  Otherwise any error will trigger output.

 

Having your code littered with this type of thing is amateurish, hard to maintain and bug prone.

$php_scripts = '../../php/';

 

formalize your primary settings into a script that every script includes, and set a variable or constant in that script which you use everywhere else.

Often people will name this file config.php or bootstrap.php.

Let's say your project structure is this

your_app/
├─ public/
│  ├─ module1/
│  ├─ js/
│  ├─ css/
├─ config/
   ├─ config.php

 

Inside config.php you will have something like this:

<?php

$app_root = DIRNAME(__DIR__);
// or if you prefer
DEFINE('APP_ROOT', DIRNAME(__DIR__));

All you need do in any script is make sure to include the config.php using whatever relative path you have, and from there, you will be able to make new paths using $app_root or APP_ROOT, without ever having to use relative pathing.  If it isn't clear, $app_root will always be /path/to/your_app

[foxclone]

@gizmola - can this be used giving an absolute path rather than a relative path? For example can I use $php_scripts = '/php' rather than $php_scripts = '../../php/';

Edited March 1, 2022 by foxclone
path correction

[foxclone]

@gizmola - For example, in config.php,  could I replace the relative path with the absolute path?

<?php

$app_root = DIRNAME(__DIR__);

$connection = '/php/PDO_Connection_Select.php';

$get_ip = '/php/GetUserIpAddr()';

Instead of

<?php

$app_root = DIRNAME(__DIR__);

$connection = '../../php/PDO_Connection_Select.php';

$get_ip = '../../php/GetUserIpAddr()';

 

 

Edited March 1, 2022 by foxclone

[gizmola]

Yes that is the point of it.  Within all your scripts you will only ever need to be aware of the relative path from the root of your project.  

So let's assume you have a directory beneath your project directory named  /path/to/project/images.

To get a path to that, no matter what script or where all you need to do is have included the config.php, and from there you specify all file paths relative to your project root.

// at top of script
require_once('../config/config.php');


// now $app_root will be available
$imagepath = $app_root . '/images';

 

This will work for any requires/includes or any actual filesystem paths you need.  You only ever need to be concerned with how to require the project/config/config.php script relative to the script you are using it in.  

I hesitate to bring this up with everything else, but this is an advantage of using the "front controller" pattern that comes with most frameworks.   It facilitates MVC and in particular handling routing in your application.  You really only have one php script under your webroot, and never run individual scripts directly.  The front controller script (which will be index.php) has everything that is not a .css or .js or image file redirected to it, and it parses up the request and figures out how to "route" the request.  You create controller scripts that map to routes, and do your actual work.  

If you were to use most any php framework (symfony, laravel, cakephp, etc) this is how things work, and there are a lot of advantages and problems solved by MVC.  With that said, this technique of establishing a reliable app root and working from it, while baked into those frameworks, can be used by any application, assuming you understand what you need to do to use it.

Once you start using the config.php script, you will find it is a handy place for putting other things like database connect strings, or making other path variables.  For example the same script can be added upon, by just creating the global path strings you want directly in the config script (ie. define $imagepath in the config.php).

[foxclone]

@gizbola - I understand what you're saying. My problem is that in some cases the relative path to the php folder is ../../php and in other cases the path is ../php. That's why I was wondering about using absolute path.

EDIT: I just re-read your last post.  I could do $phppath = $app_root . '../php';

Edited March 1, 2022 by foxclone

[gizmola]

2 minutes ago, foxclone said:

@gizmola - For example, in config.php,  could I replace the relative path with the absolute path?

<?php

$app_root = DIRNAME(__DIR__);

$connection = '/php/PDO_Connection_Select.php';

$get_ip = '/php/GetUserIpAddr()';

Instead of

<?php

$app_root = DIRNAME(__DIR__);

$connection = '../../php/PDO_Connection_Select.php';

$get_ip = '../../php/GetUserIpAddr()';

 

Again this assumes that under your project root directory you have a /config directory, and in that directory you have a config.php file which defines your $app_root variable.

I don't know what your project structure looks like, but if you want to require something in a directory named  /path/to/project/php then 

 

<?php
require_once('../../config/config.php');

require_once($app_root . '/php/PDO_Connection_Select.php');

$connection = ... make_your_connection_function();

// this should not be some global you are making in PDO_Connection as a side effect. 
//If you want a global db connection variable, just create it in the script and by inclusion you can start using it

// Again this is a function, so as long as you included its definition you just use it.
$get_ip = GetUserIpAddr();

 

 

[gizmola]

5 minutes ago, foxclone said:

@gizbola - I understand what you're saying. My problem is that in some cases the relative path to the php folder is ../../php and in other cases the path is ../php. That's why I was wondering about using absolute path.

That is what this technique does for you.  The ONLY script you need to know the relative path to is your config script.  All other paths are specified relative to $app_root, as if it was a virtual filesystem.  You never need to use .. to traverse up the tree.  $app_root will always be the path to the root of your project.  

[foxclone]

@gizmola - So, does this look correct for a config.php?

Quote

<?php

$app_root = DIRNAME(__DIR__);

$php_path = $app_root.'../php/';

$connection = $php_path.'PDO_Connection_Select.php';

$get_ip = $php_path.'GetUserIpAddr()';

 

Edited March 1, 2022 by foxclone

[foxclone]

@gizmola - A question for you. Can I include the file that has your PDO connection script in the config.php? This is what I was thinking:

<?php

$app_root = DIRNAME(__DIR__);

$php_path = $app_root.'../php/';

include_once $php_path.'PDO_Connection_Select.php';

$get_ip = $php_path.'GetUserIpAddr()';

Are the variables declared in config.php available in the file that has the include 'config.php' statement?

[gizmola]

I can't answer you entirely because I don't know what your project directory structure is.  With that said a couple of things:

• An include or require is like a "copy" routine.  Whatever code is in the .php being required, is essentially copied into the parent script at that point.  

So your path *might be right".  

This is completely wrong:  

$connection = $php_path.'PDO_Connection_Select.php';

$get_ip = $php_path.'GetUserIpAddr()';

The first line, sets the variable $connection = to a string that looks like a path to your PDO_Connection_Select.php script.  Again, you should just be requiring that script, which copies it into config.php.  Any functions and variable you set in that script will be available globally at that point, and will also be available to scripts that include config.php.

The 2nd line, is again trying to make a string.  It looks like you are trying to call the function GetUserIpAddr().  Looking back at your initial question, it appears that is defined in a script in the same php directory.  So... this is probably what you want to do if you are moving this all to config.php.

//config/config.php
$app_root = DIRNAME(__DIR__);
$php_path = $app_root . '/php/';
require_once($php_path . 'PDO_Connection_Select.php');
require_once($php_path . 'GetUserIpAddr.php');

 

With that said, I wouldn't move the requires up into config unless you are certain you need the things in those scripts (functions, variables, classes) in every script in your system.  Only you can say for sure.  Certainly you can make the path variables.  After that you can use them in other scripts as I illustrate here:

 

<?php
//In any script, but this path must be correct to load config.php
require_once('../config/config.php');

// Now require whatever you want using $php_path to get to the scripts in /project/php.  
// It doesn't matter where this calling script it.  It could 20 directories down in the tree.  These computed paths will be correct to things in /php directory

require_once($php_path . 'PDO_Connection_Select.php');
require_once($php_path . 'GetUserIpAddr.php');

// Now you can call functions or use variables in those included scripts.

$get_ip = GetUserIpAddr();

 

I hope this explains this for you.  

Speaking editorially:  why do you have a directory named php?  Your entire project is made up of php scripts.  It's a terrible name for a directory for a php project.  Maybe it should be /lib.  The directory layout of any project will organize and facilitate some of these features.  

 

[foxclone]

@gizmola - re: php directory - That is part of the structure provided by the web host.

I've searched all over the web trying to find a tutorial for building a config.php file without success. Lots of info on using php.ini but nothing specifically about config.php. Can you point me to something that will help me learn how to use it?