A little problem in my code.. Anyone knows whats wrong?

Discussion in 'PHP' started by kalle437, Jan 18, 2007.

  1. #1
    Hello!
    I am programming a imageuploading host. And I want the function to do this; If the image that is uploaded already exists in the directory, the file should be renamed to rand(1,99999)imagename. So there won't be any errors if a person uploads a file witch's name already exists in the database. But I get an never eding loop. What could be wrong?
    The code is the following:
    
    // FIX THE IMAGE
    include "config.php";
    if (!isset($HTTP_POST_FILES['userfile'])) exit;
    if (is_uploaded_file($HTTP_POST_FILES['userfile']['tmp_name'])) {
    if ($HTTP_POST_FILES['userfile']['size']>$ms) {
    echo "Your File Size is too big, please reduce the size and try again.<br>\n"; exit; }
    if(($HTTP_POST_FILES['userfile']['type']=="image/gif") || 
    ($HTTP_POST_FILES['userfile']['type']=="image/pjpeg") || ($HTTP_POST_FILES['userfile']['type']=="image/jpeg") || 
    ($HTTP_POST_FILES['userfile']['type']=="image/jpg") || 
    ($HTTP_POST_FILES['userfile']['type']=="image/jpg") || 
    ($HTTP_POST_FILES['userfile']['type']=="image/bmp") || 
    ($HTTP_POST_FILES['userfile']['type']=="image/JPEG") ||
    ($HTTP_POST_FILES['userfile']['type']=="image/png")) {
    
    $zufall = rand(1,99999);
    $fupl = "$zufall";
    $res = copy($HTTP_POST_FILES['userfile']['tmp_name'], "./" .$path .$HTTP_POST_FILES['userfile']['name']);
    if (!$res) { echo "Upload Failed, please try again<br>\n"; exit; } else {
    ?>
    <br>
    <?php
    //SET UP THE URL OF THE IMAGE
    $domst = "";
    $drecks = "/";
    $imgf = $HTTP_POST_FILES['userfile']['name'];
    $thbf = $tpath.$imgf;
    $urlf = $domst .$domain .$drecks .$path .$imgf;
    $filename = $HTTP_POST_FILES['userfile']['name'].$HTTP_POST_FILES['file']['type'];
    
    while (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name'])) {
    $zufall = rand(1,99999);
    $fupl = "$zufall";
    if (!$res) { echo "Upload Failed, please try again<br>\n"; exit;} else {
    
    
    
    $domst = "";
    $drecks = "/";
    $imgf = $fupl.$HTTP_POST_FILES['userfile']['name'];
    $thbf = $tpath.$imgf;
    $urlf = $domst .$domain .$drecks .$path .$imgf;
    $filename = $fupl.$HTTP_POST_FILES['userfile']['name'].$HTTP_POST_FILES['file']['type'];
    
    
    
    	
    }
    
    }
    
    Code (markup):
    Thanks,
    Kalle :)
     
    kalle437, Jan 18, 2007 IP
  2. solidphp

    solidphp Peon

    Messages:
    46
    Likes Received:
    2
    Best Answers:
    0
    Trophy Points:
    0
    #2
    while (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    PHP:
    Because this is always true. $HTTP_POST_FILES['userfile']['name'] is a single file name and if it's there, file_exists() will always return true.
     
    solidphp, Jan 18, 2007 IP
  3. kalle437

    kalle437 Peon

    Messages:
    283
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    0
    #3
    Right. What can I write instead of that? Is there anything like file_exists_in or something, that could solve this?
     
    kalle437, Jan 18, 2007 IP
  4. solidphp

    solidphp Peon

    Messages:
    46
    Likes Received:
    2
    Best Answers:
    0
    Trophy Points:
    0
    #4
    I think just changing the while to an if would work.

    Instead of this:

    while (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    PHP:
    Make it this:

    if (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    PHP:
     
    solidphp, Jan 18, 2007 IP
  5. kalle437

    kalle437 Peon

    Messages:
    283
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    0
    #5
    Ok, but you said that:
    
    while (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    
    PHP:
    always would be true. So if I write:

    
    if (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    
    PHP:
    the file_exist will always be true. So there is no meaning of doing that if thing then :(

    Anyone else knows how to do?
     
    kalle437, Jan 18, 2007 IP
  6. clancey

    clancey Peon

    Messages:
    1,099
    Likes Received:
    63
    Best Answers:
    0
    Trophy Points:
    0
    #6
    The difference is that while will loop until the statement is false.

    If only executes once, if the statement is true.
     
    clancey, Jan 18, 2007 IP
  7. kalle437

    kalle437 Peon

    Messages:
    283
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    0
    #7
    Yes, clancey I know :) The problem is that I want a while loop to loop until the new rolled name don't exist in the containing folder.

    I will explain again ;); What I want this script to do is:
    When someone uploads a file, the script will check if that filename already exists in a specific folder, and will change the name of the file until the name is unique for that folder.

    Kalle :)
     
    kalle437, Jan 18, 2007 IP
  8. solidphp

    solidphp Peon

    Messages:
    46
    Likes Received:
    2
    Best Answers:
    0
    Trophy Points:
    0
    #8
    Just to add to this:

    if (file_exists("./".$path . $HTTP_POST_FILES['userfile']['name']))
    PHP:
    What that means is if this file exists, do something. In your case, you want to rename the file to have a random element in it if a file of the same name is already in the file system. Give it a try changing the while to an if. I think it will work for you.
     
    solidphp, Jan 18, 2007 IP
  9. solidphp

    solidphp Peon

    Messages:
    46
    Likes Received:
    2
    Best Answers:
    0
    Trophy Points:
    0
    #9
    A really good way to do that is to keep it using an IF statement and use this for the random bit instead of rand():

    $random=substr(md5(microtime()), -7); 
    PHP:
    I've use similar to create unique license strings for my licensing software. Never ran into an issue or duplicate yet.
     
    solidphp, Jan 18, 2007 IP
  10. kalle437

    kalle437 Peon

    Messages:
    283
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    0
    #10
    It worked, it gave the file some extra numbers. But the thing is, that that file didn't exist in the folder. So I wonder if you know of a way to specify where the script should look for the file, if that is possible :)

    Sorry if I have made myself unclear :)

    Kalle :)
     
    kalle437, Jan 18, 2007 IP
  11. clancey

    clancey Peon

    Messages:
    1,099
    Likes Received:
    63
    Best Answers:
    0
    Trophy Points:
    0
    #11
    I reviewed your code quickly and I do not understand why you are talking about using random numbers. You appear to be allowing the user to name the file which will be uploaded. If that is the case, check to see if the file already exists. If it does, ask the user for a new name.

    Otherwise, do not let users name files. Try combine a random number and the time stamp to generate a unique file name. This has a chance of always being unique the first time. The while loop then tests to see if the file exists. If it does it increments the proposed file name.

    If you do not do that, you might try:

    
    $path = "/path_to_web_dir/upload_dir/";
    $filename = $HTTP_POST_FILES['userfile']['name'];
    $count = 0;
    while ( file_exists( $path .  $count . $HTTP_POST_FILES['userfile']['name'] ) )
       { $count++; }
    # then create the file $path .  $count . $HTTP_POST_FILES['userfile']['name'] 
    
    PHP:

    This generates a unqiue file name and tests to see if it is already in the directory. Once you have a clean name, you should then allow the upload with the new file name, but not before.

    I do not see where you are testing to make sure the user is not trying to hack the file name to cause the file to loaded into an inappropriate directory using "../"

    As you know dira/dirb/file looks for a file in dirb; whereas dira/../file looks for a file in dira
     
    clancey, Jan 18, 2007 IP
  12. kalle437

    kalle437 Peon

    Messages:
    283
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    0
    #12
    Well, I havn't come to the testing of security wholes yet. Want to finish this upload page first.

    And, no, I don't let my users choose a name, I let them upload a picture. Thats why I use the random thing.

    At first, all files got a random number infront of them and that worked. But I wanted to develop this thing so only files witch needed a random number infront of them got it. But I will try your thing with counting upwards to a number that is unique. Thats brilliant :)

    Kalle:)
     
    kalle437, Jan 18, 2007 IP