SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 47
  1. #1
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)

    Problem accessing Array

    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...
    PHP 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. #2
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    @DoubleDee

    Have you dumped $imageDetails
    Code:
    echo '$imageDetails: '; echo var_dump($imageDetails) . '<br /><br />';
    to see the results of the should be array? What does it output?

    Steve
    ictus==""

  3. #3
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ServerStorm View Post
    @DoubleDee

    Have you dumped $imageDetails
    Code:
    echo '($imageDetails)'; echo var_dump($imageDetails) . '<br /><br />';
    to see the results of the should be array? What does it output?

    Steve
    Yes I did.

    And it gives a really *bizzare* format...
    Code:
    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...
    PHP Code:
    list($width$height$type$dimen$bits$channels$mime) = @getimagesize($tempFile); 

    Debbie

  4. #4
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    693
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    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:
    PHP Code:
           $imageDetails getImageSize('icon-valid.gif');
            if (
    $imageDetails)
            {
                echo 
    'Type: ' $imageDetails['mime'] . "\n";
            } 
    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. #5
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    @DoubleDee

    That bizarre format is just a associative array that is mixing numeric and string keys. You can recreate doing:
    Code:
    $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:
    Code:
    var_dump($imageDetails['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
    ictus==""

  6. #6
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ahundiak View Post
    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
    list() only works on numerical arrays and assumes the numerical indices start at 0.
    Okay, I didn't know that.


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


    Just do:
    PHP Code:
           $imageDetails getImageSize('icon-valid.gif');
            if (
    $imageDetails)
            {
                echo 
    'Type: ' $imageDetails['mime'] . "\n";
            } 
    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...
    PHP Code:
    $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. #7
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ServerStorm View Post
    @DoubleDee

    That bizarre format is just a associative array that is mixing numeric and string keys. You can recreate doing:
    Code:
    $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.
    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...
    PHP Code:
        list($width$height$mime) = @getimagesize($tempFile); 

    Try echoing this directly:
    Code:
    var_dump($imageDetails['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. #8
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    <removed>
    ictus==""

  9. #9
    Non-Member bronze trophy
    Join Date
    Nov 2009
    Location
    Keene, NH
    Posts
    3,760
    Mentioned
    23 Post(s)
    Tagged
    0 Thread(s)
    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...

    Code:
    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';
    }
    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"
    Code:
    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.

  10. #10
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    693
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    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
    PHP Code:
           $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. #11
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by deathshadow60 View Post
    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.
    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!!


    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"
    Code:
    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. #12
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ahundiak View Post
    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
    PHP Code:
           $imageDetails getImageSize('icon-valid.gif');
            if (
    $imageDetails)
            {
                list(
    $width$height$type$dimen$bits$channels$mime) = array_values($imageDetails);
                echo 
    $mime;
            } 
    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...
    PHP Code:
        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. #13
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    @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).
    PHP Code:
    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:
    PHP Code:
      $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.
    PHP Code:
    $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
    ictus==""

  14. #14
    Non-Member bronze trophy
    Join Date
    Nov 2009
    Location
    Keene, NH
    Posts
    3,760
    Mentioned
    23 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DoubleDee View Post
    Valid point, but being VERBOSE is better on me
    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:


    Quote Originally Posted by DoubleDee View Post
    At first glance I'd say an "infinite loop"...
    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. #15
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by deathshadow60 View Post
    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...
    Instead of doing this...
    PHP Code:
        // Check Upload Method.
        
    if (!is_uploaded_file($_FILES['userPhoto']['tmp_name'])){ 
    I prefer this...
    PHP Code:
        $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...
    PHP Code:
        // ********************
        // 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. #16
    Non-Member bronze trophy
    Join Date
    Nov 2009
    Location
    Keene, NH
    Posts
    3,760
    Mentioned
    23 Post(s)
    Tagged
    0 Thread(s)
    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:
    Code:
    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. #17
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    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...


    Quote Originally Posted by deathshadow60 View Post
    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:
    Code:
    if ($imageDetails = @getImageSize($tempFile)) {
      $width = $imageDetails[0];
      $height = $imageDetails[1];
      $imageType = $imageDetails['mime'];
    } else {
    // not an image
    }
    Being the more appropriate approach.
    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...

    (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. #18
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    @DoubleDee

    From post #13 the code should be:
    PHP Code:
      $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
    PHP Code:
    $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 
    ictus==""

  19. #19
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    @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.
    • 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';
    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
    ictus==""

  20. #20
    Non-Member bronze trophy
    Join Date
    Nov 2009
    Location
    Keene, NH
    Posts
    3,760
    Mentioned
    23 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DoubleDee View Post
    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...
    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.

    Quote Originally Posted by DoubleDee View Post
    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!!!
    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.

  21. #21
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ServerStorm View Post
    @DoubleDee

    From post #13 the code should be:
    I appreciate your well-thought-out solution above, but you made the assumption I know OOP...

    Will have to save that for another day, but thanks!


    Debbie

  22. #22
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    693
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    @DoubleDee

    Just to backup what @ServerStorm said, just the very fact that you even know that dealing with uploaded files is dangerous puts you light years ahead of many other developers. Keep plugging away. I do have a few more thoughts on security but again, get your code going.

  23. #23
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Just got back from a quick supper...


    Quote Originally Posted by ServerStorm View Post
    @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.
    Thanks for the kind words. (Everyone else has been making me feel like I'm "lazy" and an "idiot" so I'm not having a good day...


    Please stick with it.
    I am trying, but week's like this where I have nothing to show for my efforts makes me feel like I don't have what it takes to build my own website...


    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.
    As just mentioned, I'm sorry to "look a gift-horse in the mouth", but I'm not ready for OOP. (If you can help me fix my existing Procedural code, that would be welcome!


    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.
    I don't have any error-handling for the path to the images. Guess that is another I need to do.

    As far as directory structure, that is a whole other topic I need advice on?!

    Right now, I have my Document Root and in it I have a "members" folder which contains "upload.php" and then I have an "uploads" folder which is where I was thinking of putting Users' photos, although in all of my reading the last several days, some people say you should store photos OUTSIDE OF the Web Root and only allow access to them through a function. (I'm very curious about this...)

    Also, for everyone's advice, realize that I have a VPS but have virtually no Linux or System Admin experience, so I need secure yet reasonable solutions...


    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
    Thanks for the kind words.

    Up until this afternoon, I *thought* I was making good progress until DeathShadow "exposed" my newbie coding...



    Debbie
    Last edited by Mittineague; Feb 15, 2012 at 02:24. Reason: fixing mismatched tags

  24. #24
    SitePoint Wizard DoubleDee's Avatar
    Join Date
    Aug 2010
    Location
    Arizona
    Posts
    3,930
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Just got back from a quick supper...


    Quote Originally Posted by ServerStorm View Post
    @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.
    Thanks for the kind words. (Everyone else has been making me feel like I'm "lazy" and an "idiot" so I'm not having a good day...


    Please stick with it.
    I am trying, but week's like this where I have nothing to show for my efforts makes me feel like I don't have what it takes to build my own website...


    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.
    As just mentioned, I'm sorry to "look a gift-horse in the mouth", but I'm not ready for OOP. (If you can help me fix my existing Procedural code, that would be welcome!


    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.
    I don't have any error-handling for the path to the images. Guess that is another I need to do.

    As far as directory structure, that is a whole other topic I need advice on?!

    Right now, I have my Document Root and in it I have a "members" folder which contains "upload.php" and then I have an "uploads" folder which is where I was thinking of putting Users' photos, although in all of my reading the last several days, some people say you should store photos OUTSIDE OF the Web Root and only allow access to them through a function. (I'm very curious about this...)

    Also, for everyone's advice, realize that I have a VPS but have virtually no Linux or System Admin experience, so I need secure yet reasonable solutions...


    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
    Thanks for the kind words.

    Up until this afternoon, I *thought* I was making good progress until DeathShadow "exposed" my newbie coding...


    Debbie
    Last edited by Mittineague; Feb 15, 2012 at 02:04. Reason: fixing mismatched tags

  25. #25
    Non-Member bronze trophy
    Join Date
    Nov 2009
    Location
    Keene, NH
    Posts
    3,760
    Mentioned
    23 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by DoubleDee View Post
    I appreciate your well-thought-out solution above, but you made the assumption I know OOP...
    Might be time to give a good hard look at learning it -- shame PHP is the LAST place I'd suggest learning OOP concepts, but since it's model is so hopelessly crippled unless you plan on using some other language in the future, might as well start here.

    It's not the be-all end-all solution many people claim it is -- I've grown weary of dealing with code where even the most mundane of things is thrown inside an object, but in this case an object based handler for what you're doing may indeed be your best bet for handling it as it would... greatly simplify the process.

    Gimme a few minutes and I'll toss together an example object that's drop-in code for you -- it will combine a lot of your different threads about this into one codebase -- checking if GD is installed, checking if the image is an image, resizing it, and finally saving a resized copy that will fit inside a maximum specified dimensions. (without any extra padding/background around it).... for an old salt like me that's ten or fifteen minutes worth of code; particularly as a object... I'll give it some nice error handling logic while at it.

    Really if you're thinking security anyways you should be learning objects to use PDO... or at the very least mysqli "properly" instead of sticking with the slow procedural wrappers.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •