• This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn more.

Fixed Enable streams for Internal/External data paths

Mike Tougeron

Well-known member
#1
It would be nice to be able to allow streams for the Internal & External data paths. This would allow you to use Zend_Service_Amazon_S3 stream wrapper for file/attachment uploads.

Files changed: XenForo_Helper_File
change: if (preg_match('#^/|\\|[a-z]:#i', $path))
to: if (preg_match('#^/|\\\\|[a-z0-9]:#i', $path))
in getExternalDataPath() and getInternalDataPath()

This will allows paths like "s3://my-bucket/path/to/internal_data"

edit: changed the request to match @Mike's actual change
 

Mike Tougeron

Well-known member
#5
There were just a couple of other changes that I needed to make in order for this to work.

File: XenForo_Helper_File
method: createDirectory()
This makes it so that the file being checked keeps the trailing / which is needed for the Zend_Service_Amazon_S3_Stream class to recognize that you want the url_stat method to check for a directory and not a file. Removing that trailing / isn't necessary for either of the checks.
PHP:
        if (file_exists($path) && is_dir($path))
        {
            return true;
        }
 
        $path = preg_replace('#/+$#', '', $path);
The next two changes I can do via an extension but it'd be nice to have it in the core so other users can take advantage of it too.
File: XenForo_DataWriter_AttachmentData
method: _writeAttachmentFile()
line: 179
PHP:
            $directory = dirname($filePath);
            if (substr($directory, -1) != '/') {
                $directory .= '/';
            }
File: XenForo_DataWriter_AttachmentData
method: _moveFile()
The problem here is that you cannot rename() or
move_uploaded_file() files across stream types. So the locally uploaded file cannot be moved to the s3:// stream. I added a simple check to see if the source is a local file path and did a copy/unlink if the destination was not local. I'm fairly confident that this doesn't introduce any security issues but a peer review would be good.

PHP:
    protected function _moveFile($source, $destination)
    {
        if (is_uploaded_file($source))
        {
            if ($this->_fileIsLocal($source) && !$this->_fileIsLocal($destination)) {
                $success = copy($source, $destination);
                unlink($source);
            } else {
                $success = move_uploaded_file($source, $destination);
            }
        }
        else
        {
            if ($this->_fileIsLocal($source) && !$this->_fileIsLocal($destination)) {
                $success = copy($source, $destination);
                unlink($source);
            } else {
                $success = rename($source, $destination);
            }
        }
 
        if ($success)
        {
            XenForo_Helper_File::makeWritableByFtpUser($destination);
        }
 
        return $success;
    }
   
    protected function _fileIsLocal($filename)
    {
        if (preg_match('#^/|[a-z]:#i', $filename)) {
            return true;
        } else {
            return false;
        }
    }
The last change I had to make I'm not sure how to address. I had to make a minor tweak to Zend_Service_Amazon_S3_Stream. Since ZendFramework has moved on to version 2 I doubt this will get accepted into it but I'm going to try and get it accepted.
File: Zend_Service_Amazon_S3_Stream
Line: 454
I had to add a check if the end of the name is a / to see if it is a directory or not. Otherwise it assumed it was a file and the is_dir() checks failed.
PHP:
if(($slash = strchr($name, '/')) === false || $slash == strlen($name)-1 || substr($name, -1) == '/') {