mevdschee / php-crud-api

Single file PHP script that adds a REST API to a SQL database
MIT License
3.62k stars 1.01k forks source link

(in)direct access to $_FILES in cutomization.beforeHandler #703

Closed thipages closed 4 years ago

thipages commented 4 years ago

Hi, edited with a more comprehensive use case

the minimal use case is the following

client side

<form>
<input type="file" name='upload" />
<input type="text" name="myField" />
</form>
<script>
// call api.php with an update operation after form submission
</script>

Config server side

'middlewares'=>'customization',
'customization.beforeHandler'=>function($operation,$table,$request,$env){
        if (isset($_FILES['upload'])) {
           $filePath="somePath/".$_FILES[$key]['name'];
            move_uploaded_file($_FILES[$key]['tmp_name'], $filePath);
         } else {
              file_put_contents('err.log','nok');
         }
        return $request;
    }
  1. With this code, I never enter the ifcase. but writes in err.log BUT gets the update of database field myField
  2. I have looked at the past issues without succeess. A very similar question is #316 (not the #215), but there is no answer
  3. I have looked at the MessageInterface and the $request method getUploadedFiles may fit, but it di not work.

Thank you.


WARNING from @mevdschee: THIS CODE IS NOT SECURE!

mevdschee commented 4 years ago

Hi, thank you for opening this issue. Did you see:

https://github.com/mevdschee/php-crud-api/blob/master/examples/clients/upload/vanilla.html

It shows how to upload using base64 encoded binary.

Kind regards, Maurits

thipages commented 4 years ago

Yes, I have looked at it. But I would like to access $_FILES or similar from the api in order to move the file from temp to a defined folder

Is there a way to do this even if it is a bit tricky. Thierry

thipages commented 4 years ago

more info : the files I would like to store on the server are text or XML files.

mevdschee commented 4 years ago

Is there a way to do this even if it is a bit tricky.

First I would need to know whether you run the API standalone or as part of a framework and if you use a framework, then which one?

mevdschee commented 4 years ago

Second.. you should be using "multipart/form-data" like this:

    <form action="api.php/records/files" method="post" enctype="multipart/form-data">

But the code does not expect/support "multipart/form-data", so your middleware should transform the data.

mevdschee commented 4 years ago

Also the PSR implementation has a way of handling files that you should understand. This may help:

https://github.com/Nyholm/psr7/blob/fec7c1647166f1653842528f1fc0f870d6c75c45/tests/ServerRequestTest.php

I think we could be making some middleware that could deal with it.

thipages commented 4 years ago

Hi,

  1. I use no framework (server and client side)
  2. Adding `enctype="multipart/form-data" is not improving the situation
  3. Yes I have seen that PSR has its own way. I have tried to reproduce this.

A middleware implementation is a good idea. However, I need first to get it working in a quick/dirty impl.

Thierry

thipages commented 4 years ago

Hi, I finally solved it. Correct, it was due to multipart/form-data. The easiest way is to use FormData object in js which already has the right enc

thipages commented 4 years ago

WARNING from @mevdschee: THIS CODE IS NOT SECURE!


Hi, here is the code I use to process files. It can be a start for a middleware discussion or not. there are pending questions on my side

btw, I did not unserstand how to manage the code according to psr even with the example you gave me.

    'middlewares'=>'customization',
    'customization.beforeHandler'=>function($operation,$table,$request,$env){
        processUploadedFiles($env,'./upload/','move',true);
        return $request;
    }
/**
 * @param $env, global variable changed for a possible customization.afterHandler usage
 * Once called, $env->files contains an array
 * [0]:process ok or not (bool)
 * [1]:array copy of $_FILES with an addition of _name and _processed field
 * @param $uploadPath
 * @param $transfertMode, ('copy' or 'move') for copy or move files fom tmp folder to upload one
 * @param $filePrefixed, if true the file name will be prefixed by a timestamp
 */
function processUploadedFiles(&$env, $uploadPath, $transfertMode, $filePrefixed) {
    $files=[];
    $validity=[true];
    if ($_FILES) {
        foreach ($_FILES as $fileData) {
            $file=$fileData;
            $sourcePath=$fileData['tmp_name'];
            $targetName=$fileData['name'];
            if ($filePrefixed) $targetName=(microtime(true)*10000)."_".$targetName;
            $file['_name']=$targetName;
            // https://www.php.net/manual/fr/function.move-uploaded-file.php
            $processed=
                file_exists($sourcePath)
                && filesize($sourcePath)!==0;
                //&& !mb_strlen($targetName,"UTF-8") > 225 // may limit file length
                //&& preg_match("`^[-0-9A-Z_\.]+$`i",$targetName) // may limit the filename characters
            if ($processed) {
                if ($transfertMode==='copy') {
                    $processed=copy($sourcePath, $uploadPath . $targetName);
                } else if ($transfertMode==='move') {
                    $processed=move_uploaded_file($sourcePath, $uploadPath . $targetName);
                } else {
                    $processed=false;
                }
            }
            $file['_processed']=$processed;
            $validity[]=$processed;
            $files[]=$file;
        }
    }
    $env->files=[allTrue($validity),$files];
}
function allTrue($a) {
    return !in_array('false',$a,true);
}

WARNING from @mevdschee: THIS CODE IS NOT SECURE!

mevdschee commented 4 years ago

@thipages Do not trust the user input from $fileData['name']:

$targetName=$fileData['name'];

You should not have that as part of the $destination_path in either copy or move_uploaded_file. You should apply a hashing function to the filename, like this:

$targetName=md5($fileData['name']);

If you want to maintain the file extension, then use an extension whitelist and do not trust the user input either.

Read: https://www.php.net/manual/en/features.file-upload.php

thipages commented 3 years ago

Here is an update of the previous code which might be safer but have limitations (see uploadAndStore function doc) The idea is to upload files while inserting files reference in database with any additional data, part of the POST request.

'customization.beforeHandler' => function ($operation, $tableName, $response, $environment) {
        return $tableName==='fileReferenceTable'
                ? UploadFiles::uploadAndStore($response,'filesPath','filename_md5', 'filename_user_optional')
                : $response;
}
class UploadFiles {
    /**
     * Limitations
     * 1. file are stored without extension and mime_content_type can not solve it (eg gpx is text/xml)
     * 2. if several files are upload, the same original body is used for storing the files records
     * 3. if a file at least is not processed, no files will be moved to the filePath
     * ISSUE : in case 3, there will be a record inserted with null data
     *     workaround : have a $field_md5 NOT NULL column, thus an error 1010 code will be returned
     *     and no record will be inserted
     * 
     * @param Psr\Http\Message\ServerRequestInterface $response
     * @param string $uploadPath files folder location
     * @param string $field_md5 DB field for md5 name storage
     * @param string $field_user DB field for user name storage (default null)
     * @return Psr\Http\Message\ServerRequestInterface|null
     *
     */
    public static function uploadAndStore(&$response,$uploadPath,$field_md5,$field_user=null) {
        $resFiles=self::upload($uploadPath);
        if (count($resFiles)===0) {
            $res= null;
        } else {
            $body = $response->getParsedBody();
            $newBody = [];
            for ($i = 0; $i < count($resFiles); $i++) {
                $clone = clone $body;
                $clone->$field_md5 = $resFiles[$i][0];;
                if ($field_user) $clone->$field_user = $resFiles[$i][1];
                array_push($newBody, $clone);
            }
            $res= $response->withParsedBody($newBody);
        }
        return $res;
    }
    /**
     * @param string $uploadPath files folder location
     * @return array<String,String>
     *     0 : filename stored in file system as md5
     *     1 : user filename
     */
    public static function upload($uploadPath) {
        $uploadResult=[];
        $paths=[];
        $processed=false;
        if ($_FILES) {
            foreach ($_FILES as $file) {
                $source=$file['tmp_name'];
                $name_user=$file['name'];
                $name_md5=md5($file['name']);
                $path_md5=$uploadPath.$name_md5;
                $processed=
                    file_exists($source)
                    && filesize($source)>0
                    && $file['error']===UPLOAD_ERR_OK;
                $paths[]=[$source,$path_md5];
                $uploadResult[]=[$name_md5,$name_user];
                if (!$processed) break;
            }
        }
        if ($processed) {
            for($i=0;$i<count($uploadResult);$i++) {
                move_uploaded_file($paths[$i][0], $paths[$i][1]);
            }
        } else {
            $uploadResult=[];
        }
        return $uploadResult;
    }
}