Resolved File Upload Syntax Error?

RonnyNishimoto · 07-18-2012, 12:12 AM

Just copy pasted into a new .php from a tutorial...

http://www.w3schools.com/php/php_file_upload.asp

doesn't even work?
Parse error: syntax error, unexpected T_BOOLEAN_AND in C:\xampp\htdocs\php\upload.php on line 8

PHP Code:


			
<?php

$allowedExts = array("jpg", "jpeg", "gif", "png");

$extension = end(explode(".", $_FILES["file"]["name"]));

if ((($_FILES["file"]["type"] == "image/gif")

|| ($_FILES["file"]["type"] == "image/jpeg")

|| ($_FILES["file"]["type"] == "image/pjpeg"))

&& ($_FILES["file"]["size"] < 20000))

&& in_array($extension, $allowedExts))

  {

  if ($_FILES["file"]["error"] > 0)

    {

    echo "Error: " . $_FILES["file"]["error"] . "<br />";

    }

  else

    {

    echo "Upload: " . $_FILES["file"]["name"] . "<br />";

    echo "Type: " . $_FILES["file"]["type"] . "<br />";

    echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";

    echo "Stored in: " . $_FILES["file"]["tmp_name"];

    }

  }

else

  {

  echo "Invalid file";

  }

?>

tangoforce · 07-18-2012, 12:20 AM

If that code came from w3 schools then their standards are really slipping.

These lines:
&& ($_FILES["file"]["size"] < 20000))
&& in_array($extension, $allowedExts))

Should be:
&& ($_FILES["file"]["size"] < 20000)
&& in_array($extension, $allowedExts))

Note the position of the brackets.

You also need to use a editor such as Notepad++ which will show you line numbers but also allow you to see how brackets are paired up. See the coding styles link in my signature for more information on that.

RonnyNishimoto · 07-18-2012, 12:27 AM

I will check notepad++, I've heard great praise of it.

Thank you for your help!

RonnyNishimoto · 07-18-2012, 01:11 AM

Wait! That doesn't work, they did add an extra ")" but it still doesn't work:

When I take the ")" out I get the error:

Strict Standards: Only variables should be passed by reference in C:\xampp\htdocs\php\upload.php on line 6
Invalid file

which is because of the (end(explode)) I assume? I cleaned up the code so that the array would be more clean, and I still got the same problem:

PHP Code:


			
<html>
    <head></head>
    <body>
    <?php
    $allowed = array("image/jpg", "image/jpeg", "image/gif", "image/png");
    $extension = end(explode(".", $_FILES["file"]["name"]));
    if (($_FILES["file"]["type"] == $allowed) && in_array($extension, $allowed))
    {
        if ($_FILES["file"]["error"] > 0)
        {
            echo "Error: " . $_FILES["file"]["error"] . "<br />";
        }
        else
        {
            echo "Upload: " . $_FILES["file"]["name"] . "<br />";
            echo "Type: " . $_FILES["file"]["type"] . "<br />";
            echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
            echo "Stored in: " . $_FILES["file"]["tmp_name"];
        }
    }
    else
    {
        echo "Invalid file";
    }
    ?>
    </body>
</html>

DrDOS · 07-18-2012, 01:29 AM

You code is better than theirs but still has built in error modes. What if the file is .JPG or .JPEG, it would fail

PHP Code:


			
$ftype = $_FILES['file']['type']; // check $ftype in your inarray statement.
$extension = basename($ftype);

If you have an application that reads the file header it may be able to get the actual file type as opposed to the mere alleged one, so you can detect scripts that are snuck in as .jpg, for instance. I use imagemagick identify for that.

RonnyNishimoto · 07-18-2012, 01:39 AM

I think $extension was meant to find the extension of the file, or the file type.

I think basename() would take the name of the file?
http://php.net/manual/en/function.basename.php

while when you set a limit to explode() it will give you a part of the string

Regardless, this doesn't work

If I take $extension out of in_array(), then what's the point of using it? Whether I use it or not, it just says Invalid File, though I'm uploading a .png

PHP Code:


			
<html>
    <head></head>
    <body>
    <?php
    $allowed = array("image/jpg", "image/jpeg", "image/gif", "image/png");
    $ftype = $_FILES["file"]["name"];
    $extension = basename($ftype); 
    if (($_FILES["file"]["type"] == $allowed) && in_array($extension, $allowed))
    {
        if ($_FILES["file"]["error"] > 0)
        {
            echo "Error: " . $_FILES["file"]["error"] . "<br />";
        }
        else
        {
            echo "Upload: " . $_FILES["file"]["name"] . "<br />";
            echo "Type: " . $_FILES["file"]["type"] . "<br />";
            echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
            echo "Stored in: " . $_FILES["file"]["tmp_name"];
        }
    }
    else
    {
        echo "Invalid file";
    }
    ?>
    </body>
</html>

DrDOS · 07-18-2012, 01:42 AM

Quote:

Originally Posted by RonnyNishimoto

I think $extension was meant to find the extension of the file, or the file type.

I think basename() would take the name of the file?
http://php.net/manual/en/function.basename.php

PHP Code:


			
<html>
    <head></head>
    <body>
    <?php
    $allowed = array("image/jpg", "image/jpeg", "image/gif", "image/png");
    $ftype = $_FILES["file"]["name"];
    $extension = basename($ftype); 
    if ( in_array($ftype, $allowed))
    {
        if ($_FILES["file"]["error"] > 0)
        {
            echo "Error: " . $_FILES["file"]["error"] . "<br />";
        }
        else
        {
            echo "Upload: " . $_FILES["file"]["name"] . "<br />";
            echo "Type: " . $_FILES["file"]["type"] . "<br />";
            echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
            echo "Stored in: " . $_FILES["file"]["tmp_name"];
        }
    }
    else
    {
        echo "Invalid file";
    }
    ?>
    </body>
</html>

That's what that code will do, but it will just be lower case. Since I don't know exactly what becomes of the downloaded file I can't really tell you much more. You still have to change more code for it to work, but I've used what I suggest and it's bulletproof code. You can also save the original filename and give the file a working filename if it will be modified. then when it's downloaded it can take the original filename.

RonnyNishimoto · 07-18-2012, 01:48 AM

Let's just try to get this code to work, since I'm trying to learn why I am making a mistake, not cover one up by using a different method

From my understanding, end() makes sure a file uploaded can't be spam.jpgspam.exe and explode() is to split the string where before a "." appears? Not that great of a method for security I suppose, since if there were multiple "."'s it would blow up?

And so the script is checking whether it is a image/(supported), as well as checking if it ends with .(supported), which seems redundant.

But either way, I'm much more curious as why I get this error:

Strict Standards: Only variables should be passed by reference in C:\xampp\htdocs\php\upload.php on line 6

PHP Code:


			
<html>
    <head></head>
    <body>
    <?php
    $allowed = array("image/jpg", "image/jpeg", "image/gif", "image/png");
    $extension = end(explode(".", $_FILES["file"]["name"]));
    if (($_FILES["file"]["type"] == $allowed) && in_array($extension, $allowed))
    {
        if ($_FILES["file"]["error"] > 0)
        {
            echo "Error: " . $_FILES["file"]["error"] . "<br />";
        }
        else
        {
            echo "Upload: " . $_FILES["file"]["name"] . "<br />";
            echo "Type: " . $_FILES["file"]["type"] . "<br />";
            echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
            echo "Stored in: " . $_FILES["file"]["tmp_name"];
        }
    }
    else
    {
        echo "Invalid file";
    }
    ?>
    </body>
</html>

RonnyNishimoto · 07-18-2012, 02:08 AM

Wait? end(explode()) would be to find the last place .(supported) was used...

so it's to prevent spam.jpgspam.exe

It would find .exe and say that $extension is not in_array of $allowed
It doesn't work, because $allowed has image/(supported), while it's checking for (supported)

Am I correct?

DrDOS · 07-18-2012, 08:29 PM

Quote:

Originally Posted by RonnyNishimoto

Wait? end(explode()) would be to find the last place .(supported) was used...

so it's to prevent spam.jpgspam.exe

It would find .exe and say that $extension is not in_array of $allowed
It doesn't work, because $allowed has image/(supported), while it's checking for (supported)

Am I correct?

I just checked out how 'end' works in php, and I know why you get an error with it. Simple. It modifies an array by removing all but the last element. So in your original post you need to use $extension[0] in the in_array test.

Fou-Lu · 07-18-2012, 08:35 PM

It doesn't actually remove it from an array.
End works by changing the pointer of the array to the last position, then issues a current to grab and return it. Because of the shift on the pointer, the array provided MUST be a variable, and not an argument that returns an array (such as a function).

PHP Code:


			
<?php

$a = array('cat', 'dog', 'mouse');
print end($a) . PHP_EOL; // mouse
while ($cur = current($a))
{
    print $cur . PHP_EOL; // mouse
    next($a);
}

reset($a);
while ($cur = current($a))
{
    print $cur . PHP_EOL; // all
    next($a);
}

Code:

mouse
mouse
cat
dog
mouse