md5sums wrong on downloaded files

[foxclone]

The I had this problem solved, but it isn't. Still having a problem with downloaded files having the wrong md5sum. I've scrubbed the script again to be sure there aren't any tabs or blank spaces following the commands. Also no php errors. Here's the code.

<?PHP
error_reporting(E_ALL);
ini_set('display_errors', '1');

function mydloader($filename=NULL) {
    if (isset($filename)) {
        $filename = preg_replace("/\s+/u", " ", $filename);  // Make sure no hidden characters in filename
        $ext = pathinfo($filename, PATHINFO_EXTENSION);{  // Get file extension
        if ($ext == '.iso')
            header('Content-Type: application/x-cd-image');
        elseif ($ext == '.gz')
            header('Content-Type: application/zip');
        else
            header('Content-Type: octet-stream');}
            header("Content-Disposition: attachment; filename={$filename}");
            header('Pragma: no-cache');
            header('Expires: 0');
            readfile($filename);
}
       else {
          echo "isset failed";
            }
}
mydloader($_GET["f"]);  // Filename passed from calling script

I'm totally confused as to why this isn't working correctly. I spent the entire weekend trying go get this working so I'd really appreciate some help on this.

[dodgeitorelse3]

I see nothing in code for md5sum so how do you know what is being returned?

[kicken]

If I download a file with your code and compare the downloaded file's sha1 hash to the original's sha1 hash they are the same.  Are you sure you're doing to comparisons correctly?

There are a few things you should fix with your code.

1. Your formatting needs work.
2. You have a pair of {} that are unnecessary and hard to even see.
3. Your if/elseif/else blocks should have {} around them for readability.  The way you indented your code makes it look like all your headers are part of the else branch but only one of them actually is.
4. You should add code to disable output buffering or you might hit memory errors on large files.
5. You should add a Content-length header so the browser's download progress bar works.

function mydloader($filename=NULL) {
	if (isset($filename)) {
		$filename = preg_replace("/\s+/u", " ", $filename);  // Make sure no hidden characters in filename
		$ext = pathinfo($filename, PATHINFO_EXTENSION); // Get file extension

		if ($ext == '.iso'){
			header('Content-Type: application/x-cd-image');
		} elseif ($ext == '.gz'){
			header('Content-Type: application/zip');
		} else {
			header('Content-Type: octet-stream');
		}

		header("Content-Disposition: attachment; filename={$filename}");
		header('Pragma: no-cache');
		header('Expires: 0');
		header('Content-length: '.filesize($filename));

		if (ob_get_level()) {
			ob_end_clean();
		}
		readfile($filename);
	}
	else {
		echo "isset failed";
	}
}

 

[foxclone]

Thanks to both of you for your replies.

I check the downloaded file's md5sums manually. Users have been complaining.

@kicken - You said the following:

Your formatting needs work.

I know. I just started using VS Code instead of a text editor.

You have a pair of {} that are unnecessary and hard to even see.

I'm not sure where you're talking about

Your if/elseif/else blocks should have {} around them for readability.  The way you indented your code makes it look like all your headers are part of the else branch but only one of them actually is. 

I'll reformat it

You should add code to disable output buffering or you might hit memory errors on large files.

The largest file is 870Mb and we've had no problems.

You should add a Content-length header so the browser's download progress bar works.

Will do.

-----------------------------------------------------------------------------------------------------

Again, thanks to both of you. I'll post revised code soon.

Edited March 7, 2022 by foxclone
correct misspellings

[kicken]

1 hour ago, foxclone said:

You have a pair of {} that are unnecessary and hard to even see.

I'm not sure where you're talking about

Here:

$ext = pathinfo($filename, PATHINFO_EXTENSION);{  // Get file extension
//---------------------------------------------^
if ($ext == '.iso')
    header('Content-Type: application/x-cd-image');
elseif ($ext == '.gz')
    header('Content-Type: application/zip');
else
    header('Content-Type: octet-stream');}
//---------------------------------------^
    header("Content-Disposition: attachment; filename={$filename}");
    header('Pragma: no-cache');
    header('Expires: 0');
    readfile($filename);

That pair of braces is unnecessary.  For a while I only noticed one of them and was wondering why you weren't getting a syntax error with the code.

Edited March 7, 2022 by kicken

[foxclone]

@kicken - I thought the md5sum problem was fixed but it's not. Here's an example of the differences:

md5sum of original foxclone50_amd64.deb file: a00bd61088cf4f4a0138d51f24e554f0

md5sum of downloaded foxclone50_amd64.deb file:  6f23af927c0a3348345ed1415fb89e36

Here's the current code:

<?php

function mydloader($filename=NULL)  {
    if( isset( $filename ) ) {        
        $ext = pathinfo($filename, PATHINFO_EXTENSION);
            {
            if ($ext == '.iso')
                header('Content-Type: application/x-cd-image');
            elseif ($ext =='.gz')
                header('Content-Type: application/zip');
            else
                header('Content-Type: octet-stream'); 
            }
        header('Content-Length: ' .filesize($filename));
        header("Content-Disposition: attachment; filename={$filename}");
        header('Pragma: no-cache');
        header('Expires: 0');        
        readfile($filename);
                  
      }
        
    else {
        echo "isset failed";
        }  
}
mydloader($_GET["f"]);

Thanks for your help.

[ginerjm]

Once again I'll show you how I would write this and clear up some of the code at the same time....

(assuming already in php mode....)
$result = mydloader($_GET["f"]);
if ($result !== true)
{
	echo $result
	(something else?)
}
//*********************
function mydloader($filename=NULL)
{
	if(isset($filename))
	{
		$ext = pathinfo($filename, PATHINFO_EXTENSION);
	//  ENTIRELY UN-NEEDED BRACE HERE: {
		if ($ext == '.iso')
			header('Content-Type: application/x-cd-image');
		elseif ($ext =='.gz')
				header('Content-Type: application/zip');
			else
				header('Content-Type: octet-stream');
	//  ENTIRELY UN-NEEDED BRACE HERE: }
		header('Content-Length: ' .filesize($filename));
		header("Content-Disposition: attachment; filename={$filename}");
		header('Pragma: no-cache');
		header('Expires: 0');
		readfile($filename);
	}
	else
	{
		return "Filename missing in mydloader function";
	}
	return true;
}

There is no reason to place your function code at the top of a script nor before it is actually called.  It is (hopefully) well-debugged code that can be included or placed below the mainstream portion of your script (out of the way) to avoid having to scroll by it or scan thru it while working on your task.

Question - is this function something that you will call repeatedly in this script?  If not, why is it even a function?

[kicken]

2 hours ago, foxclone said:

 I thought the md5sum problem was fixed but it's not. Here's an example of the differences:

Look at the contents of the two files and see what the difference is. 

xxd foxclone50_amd64.deb >original
xxd downloaded_foxclone50_amd64.deb >downloaded
diff original downloaded

 

[foxclone]

@kicken - That didn't tell us anything at all:

larry@t430:~/Downloads$ diff /var/www/foxclone/public_html/download/foxclone50_amd64.deb /home/larry/Downloads/foxclone50_amd64.deb
Binary files /var/www/foxclone/public_html/download/foxclone50_amd64.deb and /home/larry/Downloads/foxclone50_amd64.deb differ

They are different in the hex editors.

Original file shows: Signed 32 bit as: 1769108595

Downloaded file shows: Signed 32 bit as: 1751347809

Question for you: could that preg_replace() in the code be causing this issue?

Edited March 8, 2022 by foxclone

[kicken]

52 minutes ago, foxclone said:

@kicken - That didn't tell us anything at all:

That's why I showed using xxd to get a hex dump of the file contents and then diff'ing that and not the files directly.  That way you can actually see what the differences are.

 

[foxclone]

20 minutes ago, kicken said:

That's why I showed using xxd to get a hex dump of the file contents and then diff'ing that and not the files directly.  That way you can actually see what the differences are.

 

@kicken - I did that and when i go diff original download, it just scrolls up my screen. here are the size difference:

original size: 5674712
download size: 5674783 

I've compared the files in Meld and it appears that the filename is being pre-pended to the downloaded file:

00000000: 7374 7269 6e67 2832 3029 2022 666f 7863  string(20) "foxc
00000010: 6c6f 6e65 3530 5f61 6d64 3634 2e64 6562  lone50_amd64.deb
00000020: 220a 213c 6172 6368 3e0a 6465 6269 616e  "

Edited March 8, 2022 by foxclone

[kicken]

Then you probably have a var_dump somewhere outputting that which you have not shown in your code here.

 

[foxclone]

@kicken - I just rechecked the code; no var_dumps.

[kicken]

@foxcloneNothing you've posted here would explain that output being included in the downloaded file.   It's coming from somewhere else in your code that you haven't posted so there's really nothing anyone here can do to help you more at this point.

You need to re-examine every scrap of code that is run when you make a download request and try to track down where that output is coming from.

[foxclone]

@kicken -  I've been looking at this code since last Friday. I think I need to take a few days away from this project so I'm not seeing "What should be" instead of "What is".

[ginerjm]

Hmmm....

You're saying that since Friday we've been looking at your 19 lines of code and that is ALL there is to this whole topic?  C'mon - Where is the rest of your script?  That is what Kicken is asking to see.  He wants to help you out.

[foxclone]

The rest of the code is solely for inserting a record into the database table, but I'll include it here.

<?php

function mydloader($filename=NULL)  {
    if( isset( $filename ) ) {        
        $ext = pathinfo($filename, PATHINFO_EXTENSION);
            {
            if ($ext == '.iso')
                header('Content-Type: application/x-cd-image');
            elseif ($ext =='.gz')
                header('Content-Type: application/zip');
            else
                header('Content-Type: octet-stream'); 
            }
        header('Content-Length: ' .filesize($filename));
        header("Content-Disposition: attachment; filename={$filename}");
        header('Pragma: no-cache');
        header('Expires: 0');        
        readfile($filename);
        
        
        $php_scripts = "https://foxclone.com/php/";
        require $php_scripts.'PDO_Connection_Select.php';
        require $php_scripts.'GetUserIpAddr.php';

        $ip = GetUserIpAddr();
        if (!$pdo = PDOConnect("foxclone_data"))   {	
               exit;
        }

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

        $ext = pathinfo($l_filename, PATHINFO_EXTENSION);
        $stmt = $pdo->prepare("INSERT INTO download (`address`, `filename`,`ip_address`, `lookup_id`) VALUES (?, ?, inet_aton('$ip'),?)");
        $stmt->execute([$ip, $ext,$ref]) ; 


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

 

 

 

[mac_gyver]

1 hour ago, foxclone said:

$php_scripts = "https://foxclone.com/php/";

when you require a .php file using a URL, on a correctly configured and functioning web server, you don't get the php code in the file, you only get any output produced by that php code.  you must use a filesystem path in order to require .php files.

[ginerjm]

And as Kicken has already asked you - where is the part concerning your posted topic title?  Can not see the fact that there is nothing related to anything 'md5 ish' in this code?  

PS - suggestion.  Do your includes at the top of any script so that you know right away what the script has to work with.  Dont bury your requires inside functions.  It's kind of like burying a function inside a function.  Calling one is ok - including one is not a good practice.

[foxclone]

All problems fixed. Thanks to everyone who replied.

[ginerjm]

And I have no idea how I solved this topic.    Oh, well.....