Problem accessing Array

PHP
1

I am having trouble accessing the MIME-type in the getImageSize() array.

I tried adding $mime, mime and ‘mime’ to my list() but I kept getting errors.

Below you can see I get that information in lines 8-10, but I would expect to capture it with $width and $height in list()

Here is my code…


	// Check for Image.
	list($width, $height) = @getimagesize($tempFile);

	if ($width && $height){
		echo 'Is an Image';

		// Get of Image Details.
		$imageDetails = getImageSize($tempFile);
		$imageType = $imageDetails['mime'];
		echo '<p>imageType = ' . $imageType . '</p>';
	}else{
		// Not Image File.
		$errors['upload'] = 'File-type must be either GIF, JPG, or PNG';
	}

What am I doing wrong?

Debbie

2

@DoubleDee

Have you dumped $imageDetails


echo '$imageDetails: '; echo var_dump($imageDetails) . '<br /><br />';

to see the results of the should be array? What does it output?

Steve

3

Yes I did.

And it gives a really bizzare format…


array
  0 => int 217
  1 => int 216
  2 => int 1
  3 => string 'width="217" height="216"' (length=24)
  'bits' => int 5
  'channels' => int 3
  'mime' => string 'image/gif' (length=9)

I tried something like this last night but it kept failing on the 4-6 items…

list($width, $height, $type, $dimen, $bits, $channels, $mime) = @getimagesize($tempFile);

Debbie

4

Not so bizarre. Documented here: http://php.net/manual/en/function.getimagesize.php

And if you look up the list function: http://us3.php.net/manual/en/function.list.php

We find

list() only works on numerical arrays and assumes the numerical indices start at 0.

I seldom use list. Seems fragile to rely on argument order. Just do:


       $imageDetails = getImageSize('icon-valid.gif');
        if ($imageDetails)
        {
            echo 'Type: ' . $imageDetails['mime'] . "\
";
        }

And I’d lose the @ as well. If there is a problem with the file name itself then you should probably deal with it instead of hiding it.

5

@DoubleDee

That bizarre format is just a associative array that is mixing numeric and string keys. You can recreate doing:


$info[] = 217;
$info[] = 216;
$info[] = 1;
$info[] = 'width="217" height="216"';
$info['bits'] = 5;
$info['channels'] = 3;
$info['mime'] = 'image/gif';
var_dump($info);

I ran your code (from post 1) and it functioned properly.

Try echoing this directly:


var_dump([COLOR=#0000BB][FONT=monospace]$imageDetails[/FONT][/COLOR]['mime']);

What do you get? You should get

string(9) “image/gif”

It is strange that you are returning an array that contains the ‘mime’ key and you can’t access that index?

Steve

6

Is to bizzare.

Why mix Numeric and Non-Numeric Indices?!

And if you look up the list function: http://us3.php.net/manual/en/function.list.php

We find

[QUOTE]list() only works on numerical arrays and assumes the numerical indices start at 0.
[/quote]

Okay, I didn’t know that.

I seldom use list. Seems fragile to rely on argument order.

Agreed!!

Just do:


       $imageDetails = getImageSize('icon-valid.gif');
        if ($imageDetails)
        {
            echo 'Type: ' . $imageDetails['mime'] . "\
";
        }

I don’t follow your code. I was just looking for a way to simplify my code and hopefully roll “MIME” into the LIST().

Anyways.

And I’d lose the @ as well. If there is a problem with the file name itself then you should probably deal with it instead of hiding it.

I have been fighting with getImageSize() since last night and have had a few people confirm it is buggy

Last night my code looked like this…


$tempName = $_FILES['userPhoto']['tmp_name'];

// Determine if Image was uploaded via HTTP POST.
if (is_uploaded_file($tempName)){
	// File uploaded via HTTP.

	// Get of Image Details.
	$imageDetails = getImageSize($tempName);

	// Get Image Type
	$imageType = $imageDetails['mime'];

	echo '<p>var_dump($imageDetails) = ' . var_dump($imageDetails) . '</p>';

	echo '<p>var_dump($imageType) = ' . var_dump($imageType) . '</p>';

Whenever I tried to upload “test.php.jpg” I got this error…

Notice: getimagesize() [function.getimagesize]: Read error! in /Users/user1/Documents/DEV/++htdocs/05_Debbie/members/upload.php on line 56

…where Line 56 strat with $imageDetails = getImageSize($tempName);

The var_dump() showed boolean false but I was still getting that error.

After playing around with other files, I finally got this same file working with no error.

This error comes and goes with no rhyme or reason…

So I switched to the new format I have now because it always seems to work, but I added the ampersand @ just in case!!

If you can explain why I was getting these random, unexplainable errors, then maybe I’d be willing to lose the @ otherwise I think it is safer to leave it?!

Thanks,

Debbie

7

Oh, I know it works. But the purpose of this thread was just an attempt to streamline my code and I was hoping I could do this…

	list($width, $height, $mime) = @getimagesize($tempFile);

Try echoing this directly:


var_dump([COLOR=#0000BB][FONT=monospace]$imageDetails[/FONT][/COLOR]['mime']);

What do you get? You should get

It is strange that you are returning an array that contains the ‘mime’ key and you can’t access that index?

Steve

Again, I think you misunderstood me.

See my last post above this one.

Thanks,

Debbie

8

<removed>

9

BTW, your “saving code” by splitting it into separate variables may result in less SOURCE, but it’s actually more code to be executed since you’re increasing memory use (more variables), executing more code (mapping the array to those variables) and on the whole making PHP work harder.

On the whole 99% of the time people use ‘list’ they’re just making more code and slowing down execution… and damned if I can figure out why.

I’d also combine down some things…


if ($imageData=@getimagesize($tempFile)) {
	echo '
		Is an Image
		<p>imageType =',$imageData['mime'],'</p>';
} else {
	$errors['upload']='File-type must be either GIF, JPG, or PNG';
}

[ot]also, avoid using string addition instead of delimiters on echo… it’s bad practice and it can bite you if you use procedural output – it’s as bad or worse than “double quotes for nothing” :smiley:


function test() {
	echo 'test ';
}

echo '
	This is a '.test().'<br />
	This is a ',test();

You might be surprised by what this outputs… and it’s just a small part of why string addition in echo should be avoided unless inside a inlined evaluation.
[/ot]

10

I don’t follow your code. I was just looking for a way to simplify my code and hopefully roll “MIME” into the LIST().

Well if you really want to use list even though we both agree it’s fragile then just use array_values to extract an indexed array from $imageDetails


       $imageDetails = getImageSize('icon-valid.gif');
        if ($imageDetails)
        {
            list($width, $height, $type, $dimen, $bits, $channels, $mime) = array_values($imageDetails);
            echo $mime;
        }

Be aware that getImageSize returns false if it’s not an image so a simple if suffices to see if it worked.

And we can leave the @ battle for another day. Get the stuff working then refine.

11

Valid point, but being VERBOSE is better on me - the newbie programmer - so that wins out, because if Debbie doesn’t understand her code then the computer won’t either!! :wink:

Off Topic:

also, avoid using string addition instead of delimiters on echo… it’s bad practice and it can bite you if you use procedural output – it’s as bad or worse than “double quotes for nothing” :smiley:


function test() {
	echo 'test ';
}

echo '
	This is a '.test().'<br />
	This is a ',test();

You might be surprised by what this outputs…

At first glance I’d say an “infinite loop”…

Debbie

12

I’ll just stick with what I have.

Be aware that getImageSize returns false if it’s not an image so a simple if suffices to see if it worked.

But did you follow what I said above about it not always working??

That really has me freaked out…

According to the manual - which sorta contradicts itself above…

On failure, FALSE is returned.

reject note Errors/Exceptions
If accessing the filename image is impossible, or if it isn’t a valid picture, getimagesize() will generate an error of level E_WARNING. On read error, getimagesize() will generate an error of level E_NOTICE.

That is the random issue I have been getting.

Now that I am using the format…


	list($width, $height) = @getimagesize($tempFile);

	// Check for Dimensions.
	if ($width && $height){
		// Image File.
		echo '<p>Is an Image</p>';

	}else{
		// Not Image File.
		$errors['upload'] = '<p>Only Images can be uploaded (i.e. GIF, JPG, or PNG)</p>';
	}//End of CHECK FOR DIMENSIONS

…this seems to catch all files properly?!

And we can leave the @ battle for another day. Get the stuff working then refine.

Hey, I’m all for getting things right. But I also know that I have been chasing down phantom errors for the last 2 days and I need to be practical.

Please enlighten me if you see something I have been missing…

Debbie

13

@DoubleDee

I created this ImageValues object for you to simplify things a little. You would copy this to the bottom of your script or you could require_one(‘ImageValues.php’) where you copied this to a file called ImageValues.php.

This will work like storing it in the list which you were attempting to but ran into its’ limitations. This object also assigns a string key to each value. The possible keys are ‘width’, ‘height’, ‘type’, ‘dimen’, ‘bits’, ‘channels’ and ‘mime’. You can access them using the Image Values __get() method like $o_ImageInfo->__get(‘mime’) … (See below).


class ImageValues {
  protected $width;
  protected $height;
  protected $type;
  protected $dimen;
  protected $bits;
  protected $channels;
  protected $mime;

  public function setImageProperties(Array $imageValues){
    foreach($imageValues as $key => $value){
      if (is_int($key)){
        switch ($key){
          case 0:
            $key = 'width';
            break;
          case 1: 
            $key = 'height';
            break;
          case 2:
            $key = 'type';
            break; 
          case 3:
            $key = 'dimen';
            break;
        }
      }
      $this->__set($key,$value);
    }  
  }
  public function __get($var_name){
      return $this->$var_name;
  }
  public function is_image(){
    if($this->width != NULL){
      return 1;
    } else {
      return 0;
    }
  }
  protected function __set($var_name, $value){
     $this->$var_name = $value;  
  }
}

You use it like this:


  $imageDetails = getImageSize($tempFile);
  $o_ImageInfo = new ImageValues(); //Instantiate the Image Values Object
  $o_ImageInfo->setImageProperties(imageDetails); //Pass the $imageDetails Array into the object where it is parsed.
  if ($o_ImageInfo->is_image() == 1){ //check it is an image
        echo 'Is an Image'; 
        echo '<p>imageType = ' . $o_ImageInfo->__get('mime') . '</p>'; //Use the Objects __get() method
    }else{ 
        // Not Image File. 
        $errors['upload'] = 'File-type must be either GIF, JPG, or PNG'; 
    }

Here is another example of getting more of the values from the object.


$o_ImageInfo = new ImageValues();
$o_ImageInfo->setImageProperties(imageDetails);
echo $o_ImageInfo->__get('dimen'); // get images dimensions
echo $o_ImageInfo->__get('mime'); // get mime type
echo $o_ImageInfo->__get('channels'); // get mime type

Hope this helps.
Steve

14

So you’re reducing the verbosity by expanding it into new variables?!? Huh? Or are you referring to the numerical indexes? I guess I’m just not seeing how breaking it out makes it any more or less verbose…

Off Topic:

No, the latter echo is not in the function. No looping, the latter echo just calls the declared function twice – What it outputs is:

test This is a
This is a test

because the first one is done with string addition (periods) meaning the string has to be built by PHP before being sent to echo, running the inlined function BEFORE the echo executes… using commas as delimiters is treated as separate values sent to echo individually, which is faster, involves less shoving memory around, and means it will execute in the order most people would ‘think’ it should.

15

Instead of doing this…


	// Check Upload Method.
	if (!is_uploaded_file($_FILES['userPhoto']['tmp_name'])){

I prefer this…


	$tempFile = $_FILES['userPhoto']['tmp_name'];

	// Check Upload Method.
	if (!is_uploaded_file($tempFile)){

Different example, but same concept.

I just find it easier to assign things to variables and then work with the more concisely named variable.

Back to our original conversation - and taking some of your advice - I chose this style…


	// ********************
	// Check File-Type.		*
	// ********************
	if (empty($errors)){
		$imageDetails = @getImageSize($tempFile);
		$width = $imageDetails[0];
		$height = $imageDetails[1];
		$imageType = $imageDetails['mime'];

		// Check for Dimensions.
		if ($width && $height){
			// Image File.

Debbie

16

Well, just so you know, numerics are indexed BEFORE associatives, and list omits array values past it’s index limit so…

list($width, $height, $type, $attr) = getimagesize(filename);

actually works.

It’s funny though, what you’re calling more concise and verbose I find less so, because you’re not listing where it came from. The only reason I’d put it in a variable is so as to avoid calling a function more than once or calling array indexes unnecessarily – and that’s more a performance choice than a clarity one.

Also, your ‘check’ is broken, as if the getimagesize fails…

$imageDetails = @getImageSize($tempFile);
// if this fails, $imageDetails===false
$width = $imageDetails[0];
// Error - imageDetails is Not an array!!!
$height = $imageDetails[1];
// Error - imageDetails is Not an array!!!
$imageType = $imageDetails[‘mime’];
// Error - imageDetails is Not an array!!!
if ($width && $height){
// Error – $width and $height are undeclared/undefined.

hence:


if ($imageDetails = @getImageSize($tempFile)) {
  $width = $imageDetails[0];
  $height = $imageDetails[1];
  $imageType = $imageDetails['mime'];
} else {
// not an image
}

Being the more appropriate approach.

Though even if I was using getImageSize, I’d not actually be using it for dimensions until AFTER I’ve loaded it for the resize, at which point I’d be using imagesx and imagesy. This is particularly true since some image formats do not actually return their widths or heights in pixels. (part of why getImageSize has the string with the HTML width and height attributes in it, the values can differ!)

You get a jpeg that’s reporting [0]==1 and [1]==2, when the size string is width=“150” height=“300” you’ll know what I mean.

17

I’m dying on this…

I have spent the last 2 1/2 days on my “upload.php” and most of my time has been spent on the whole damn “Is this a valid image?” and every new article I read or new person I speak to tells me to go a different way… :frowning: :frowning:

I don’t follow you…

In your code above, if the Upload File (i.e. $tempFile) is not an Image File, then @getImageSize($tempFile) becomes FALSE - assuming it works correctly - and then $imageDetails would become FALSE…

So how then is there any difference between your code and mine because now $imageDetails[0] would have no logical meaning, right?

(And I have stayed away from fileinfo() because everything I read says it is poorly supported and buggy at best?!)

I would post my entire - to date - upload.php script here, but I fear you’d make minced-meat out of it… :frowning:

(Why in the hell don’t they make better functions for what I am trying to do?! This entire thing should be handled in one line of code, not 2 1/2 days of waffling…)

I am trying my damnedest to create an Upload Form that is SECURE but this just seems sooo complicated between PHP and Apache and all of the things that can go wrong!!!

Frustrated,

Debbie

18

@DoubleDee

From post #13 the code should be:


  $imageDetails = getImageSize($tempFile);
  $o_ImageInfo = new ImageValues(); //Instantiate the Image Values Object
  $o_ImageInfo->setImageProperties($imageDetails);
  if ($o_ImageInfo->is_image() == 1){ //check it is an image
        echo 'Is an Image'; 
        echo '<p>imageType = ' . $o_ImageInfo->__get('mime') . '</p>'; //Use the Objects __get() method
  }else{ 
        // Not Image File. 
        $errors['upload'] = 'File-type must be either GIF, JPG, or PNG'; 
  }

and the other examples should be


$o_Img = new ImageValues();
$o_Img->setImageProperties($imageDetails);
echo $o_Img->__get('dimen'); // get images dimensions
echo$$o_Img->__get'mime'); // get mime type
echo $o_Img->__get('channels'); // get channels type
19

@DoubleDee

For a relative newcomer to PHP you have been doing quite well. File handling is a frustrating experience in PHP and it is by no means easy, so it is not unusual for someone to spend days working an upload script out, and many developers fail to weave the security into their forms so you have added even more complexity - important yes, but also complex.

Please stick with it.

[LIST]
[]I wrote the object for you so that you can handle the properties more cleanly and consistently. You don’t have to mess around with array indexes as much.
[]I am sure that you are already doing so, but make sure you validate the paths of the images ($tempFile) that you are passing into getImageSize($tempFile),because you are doing this secure, I am sure that you are not running your upload.php in the same folder where the images are located, so a bad path or weird characters could cause the file to fail, thus rendering an error.
[*]Initialize all your variables to NULL; if reusing a variable then reinitialize it to null; so $myvar = NULL; then $myvar = ‘a’;
[/LIST]I’m sorry you are frustrated… hopefully we on this forum do not further add to your confusion; although this can be hard as we all have our own ways, beliefs and experience.

Regards,
Steve

20

Which means the if statement sees that FALSE and it’s ELSE condition triggers… rather than wasting time trying to create THREE variables that shouldn’t even have values plugged into them in the first place. (since NONE of those should resolve).

In yours – you are trying to do an array access on a variable containing a boolean false; If PHP was a REAL programming language it would halt and throw a syntax or typecasting error, but because PHP is TOO STUPID to do that, it silently throws an error in the background (filling up your error log) and plods along like nothing went wrong (one of the things I dislike about PHP – firm believer all errors should be fatal unless manually declared otherwise!)… Also why your use of @ is a bit dangerous, since you’ve turned off error reporting AND ignored the resultant error state of the function!

Your code should be throwing at LEAST four warnings and possibly some fatal errors… and would if PHP bothered with typecasting rules.

More than that though – one of the most important things about using IF statements is that they should go ‘as far outside’ setting variables as you can – that way you aren’t allocating memory and running code you don’t have to. Since the function to even pull up the values returns false on a non-image, trap it there… don’t start trying to access values that don’t even exist and try assigning them to variables if you don’t have to. Likewise, why run two comparisons when one will do the job? (which is what that logical ‘and’ is doing).

Of course someone from the peanut gallery will pipe in with that lame “premature optimization” excuse – then wonder why all their applications are slow as molasses monstrosities choking quad xeons to death. :smiley:

Welcome to why it has historically been THE number one exploit hole in forum and CMS software. You’re talking about letting users upload files to your server, basically unsupervised and untrusted… and it’s VERY hard to take untrustworthy user data and wrap security around it – many MANY times more so when you are talking an entire FILE. One PHP file renamed jpeg and one directory path exploit in your system, and poof, exploit in action.

It is complicated – that’s why it’s called work and not happy happy fun time – it’s why so many people just sleaze by on a off the shelf solution without ever learning how this stuff works under the hood - leading to why they usually aren’t qualified to say if what they choose is secure or even worth using in the first place – biting them in the backside sooner or later.

If it was easy, everyone would be doing it.