1. 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

Discussion in 'Resolved Bug Reports' started by Mike Tougeron, Mar 26, 2013.

  1. Mike Tougeron

    Mike Tougeron Well-Known Member

    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
     
  2. Mike Tougeron

    Mike Tougeron Well-Known Member

    Weird, actually in testing I found that it would need to be
    if (preg_match('#^/|\\\|[a-z0-9]+:#i', $path))
     
    Adam Howard likes this.
  3. Mike

    Mike XenForo Developer Staff Member

    Oops yeah. The \\ is seen in the regex as \, so it means a literal |.

    I've just changed it to if (preg_match('#^/|\\\\|[a-z0-9]:#i', $path))
     
    Slavik and Adam Howard like this.
  4. Mike Tougeron

    Mike Tougeron Well-Known Member

    Thanks! I'll let you know if I find any other S3 related tweaks. :)
     
  5. Mike Tougeron

    Mike Tougeron Well-Known Member

    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)-|| substr($name, -1) == '/') {
     
    Marcus, Adam Howard and Andy.N like this.
  6. Mike Tougeron

    Mike Tougeron Well-Known Member

    Marcus and Adam Howard like this.
  7. Mike

    Mike XenForo Developer Staff Member

    I've implemented these fixes in a slightly different manner. I'll send them over to you Mike to confirm that they still meet your requirements.
     
    Marcus, Slavik and Sim like this.

Share This Page