Restrictions for uploaded files

I made some changes in php code of FileManager.inc.php and ArticleFileManager.inc.php to limit uploaded supplementary files type to zip files (as we want our authors to send all necessary files packed in one zip or tar.gz file).
And this works - I mean the OJS uploads only zip files (I used unzip shell command to test the file).
But the problem is that even if the file is not uploaded, because of the invalid format, OJS numbers it and displays on the list of article’s supplementary files as the blank item.
Could you please give me some hints how to fix this?

Thank you.
AS

I think this will largely depend on the changes you’ve made, probably to ArticleFileManager::handleUpload().

The easiest way for us to comment would be if you publish your changes to GitHub. Otherwise, you can show your modifications here and we can try to comment.

I am sending the changes I made to the code of FileManager.inc.php and ArticleFileManager.inc.php - they are described as commments.
(I am a beginner to php programming so I am not sure whether this method would be proper)
Thank you very much for your help.

AS


FileManager.inc.php

    //beginning of changes AS
    // function to test the uploaded file: if file format is not zip returns true (czyli wystapi blad)
    function unzipFile($fileName) {
        $tmpFileName= ($this->getUploadedFilePath($fileName));
        system('unzip -t '.escapeshellarg($tmpFileName), $status);
        if (!$status==0) return true;
        
    }    
    //end of changes AS    
 


    // beginning of changes  AS
    /**
     * Upload a supplementary file (only zip files allowed).
     * @param $fileName string the name of the file used in the POST form
     * @param $dest string the path where the file is to be saved
     * @return boolean returns true if successful - possible only for zip file
     */
    function uploadFileSupp($fileName, $destFileName) {
        if ($this->unzipFile($fileName)) return false;
        $destDir = dirname($destFileName);
        if (!$this->fileExists($destDir, 'dir')) {
            // Try to create the destination directory
            $this->mkdirtree($destDir);
        }
        if (!isset($_FILES[$fileName])) return false;
        if (move_uploaded_file($_FILES[$fileName]['tmp_name'], $destFileName))
            return $this->setMode($destFileName, FILE_MODE_MASK);
        return false;
    }
    //end of changes AS


ArticleFileManager.inc.php

/**
     * Upload a supp file.
     * @param $fileName string the name of the file used in the POST form
     * @param $fileId int
     * @param $overwrite boolean
     * @return int file ID, is false if failure
     */
    function uploadSuppFile($fileName, $fileId = null, $overwrite = true) {
        return $this->handleUploadSupp($fileName, ARTICLE_FILE_SUPP, $fileId, $overwrite);//here handleUploadSupp is called instead of handleUpload AS
    }
  //beginning of changes AS
	
	/**
	 * PRIVATE routine to upload the supplementary file and add it to the database (this function calls uploadFileSupp instead of uploadFile)
	 * @param $fileName string index into the $_FILES array
	 * @param $fileStage int identifying file stage (defined in ArticleFile)
	 * @param $fileId int ID of an existing file to update
	 * @param $overwrite boolean overwrite all previous revisions of the file (revision number is still incremented)
	 * @return int the file ID (false if upload failed)
	 */
	function handleUploadSupp($fileName, $fileStage, $fileId = null, $overwrite = false) {
		if (HookRegistry::call('ArticleFileManager::handleUploadSupp', array(&$fileName, &$fileStage, &$fileId, &$overwrite, &$result))) return $result;

		$articleFileDao =& DAORegistry::getDAO('ArticleFileDAO');

		$fileStagePath = $this->fileStageToPath($fileStage);
		$dir = $this->filesDir . $fileStagePath . '/';

		if (!$fileId) {
			// Insert dummy file to generate file id FIXME?
			$dummyFile = true;
			$articleFile =& $this->generateDummyFile($this->article);
		} else {
			$dummyFile = false;
			$articleFile = new ArticleFile();
			$articleFile->setRevision($articleFileDao->getRevisionNumber($fileId)+1);
			$articleFile->setArticleId($this->articleId);
			$articleFile->setFileId($fileId);
			$articleFile->setDateUploaded(Core::getCurrentDate());
			$articleFile->setDateModified(Core::getCurrentDate());
		}

		$articleFile->setFileType($this->getUploadedFileType($fileName));
		$articleFile->setFileSize($_FILES[$fileName]['size']);
		$articleFile->setOriginalFileName($this->truncateFileName($_FILES[$fileName]['name'], 127));
		$articleFile->setFileStage($fileStage);
		$articleFile->setRound($this->article->getCurrentRound());

		$newFileName = $this->generateFilename($articleFile, $fileStage, $this->getUploadedFileName($fileName));

		if (!$this->uploadFileSupp($fileName, $dir.$newFileName)) {
			// Delete the dummy file we inserted
			$articleFileDao->deleteArticleFileById($articleFile->getFileId());

			return false;
		}

		if ($dummyFile) $articleFileDao->updateArticleFile($articleFile);
		else $articleFileDao->insertArticleFile($articleFile);

		if ($overwrite) $this->removePriorRevisions($articleFile->getFileId(), $articleFile->getRevision());

		return $articleFile->getFileId();
	}
	
	//end of changes AS

I added new modifications to FileManager.inc.php file to allow downloading tar.gz file as a supplementary file:

//beginning of changes AS
    // function to test the uploaded file: if file format is not tar.gz returns true (czyli wystapi blad)
    function gunzipFile($fileName) {
        $tmpFileName= ($this->getUploadedFilePath($fileName));
        system('gunzip -t '.escapeshellarg($tmpFileName), $status);
        if (!$status==0) return true;
        
    }    
    //end of changes AS    

and now function uploadFileSupp is:

// beginning of changes  AS
    /**
     * Upload a supplementary file (only zip and tar.gz files allowed).
     * @param $fileName string the name of the file used in the POST form
     * @param $dest string the path where the file is to be saved
     * @return boolean returns true if successful - possible only for zip or tar.gz files
     */
    function uploadFileSupp($fileName, $destFileName) {
        if (($this->unzipFile($fileName)) && ($this->gunzipFile($fileName))) return false;
        $destDir = dirname($destFileName);
        if (!$this->fileExists($destDir, 'dir')) {
            // Try to create the destination directory
            $this->mkdirtree($destDir);
        }
        if (!isset($_FILES[$fileName])) return false;
        if (move_uploaded_file($_FILES[$fileName]['tmp_name'], $destFileName))
            return $this->setMode($destFileName, FILE_MODE_MASK);
        return false;
    }
    //end of changes AS

If a supplementary file is in ivalid format (not zip or tar.gz file) OJS does not upload it but still it numbers it and it appears on the supplementary file list as a blank item. I would be really greatful for some hints how to fix this.

Thank you.
AS

I think you can short-circuit a bit of this by moving your check to ArticleFileManager::handleUpload() instead. You can check the $fileStage to ensure it is a supplemental file:

And if so, test for the filetype(s) you want to allow. You want to prevent the execution of the bulk of the handleUpload() code, but your functions are coming in just a bit late.

Thank you very much for your help.

I followed your advice:
I came back to the original version of FileManager.inc.php file and made changes only in ArticleFileManager.inc.php.
I added:

// beginning of changes AS
	// function to test the uploaded file: if file format is not zip returns true (czyli wystapi blad)
	function unzipFile($fileName) {
        $tmpFileName= ($this->getUploadedFilePath($fileName));
		system('unzip -t '.escapeshellarg($tmpFileName), $status);
		if (!$status==0) return true;
	}	
	// end of changes AS	
	//beginning of changes AS
	// function to test the uploaded file: if file format is not tar.gz returns true (czyli wystapi blad)
	function gunzipFile($fileName) {
        $tmpFileName= ($this->getUploadedFilePath($fileName));
		system('gunzip -t '.escapeshellarg($tmpFileName), $status);
		if (!$status==0) return true;
	}	
	//end of changes AS	

And also I added to ArticleFileManager::handleUpload() :

function handleUpload($fileName, $fileStage, $fileId = null, $overwrite = false) {
		if ($fileStage==ARTICLE_FILE_SUPP && ($this->unzipFile($fileName)) && ($this->gunzipFile($fileName))) return false; //this line was added to check for file type: only zip //and tar.gz files are allowed	

All these changes made that only zip and tar.gz files are uploaded as supplementary files, OJS names them properly (now, with correct numbering) but still if we try to download other type files the blank items appeare on the supplementary files list. How to fix this listing?

Thank you very much, Anna

Can you describe in more detail what you are seeing with"downloading other type files"? I’m not clear on what is happening, or how your code changes would affect downloads.

I made some testing of downloading supplementary files on 24 August:

you can see those files on a Supplementary Files list - only zip or tar.gz files are downloaded.
When I download a file of some other type OJS does not download it but a new blank item appears on that list (see screenshot).

Thank you for your help.

Anna

In the above supplementary files list, downloaded files that were not zip or tar.gz file formats come as the 4, 5, 6, 7 blank items. How can I fix that listing? I would like these files didn’t appear on the list at all.

Thank you, Anna

Is this just a problem from the testing before you added the line to handleUpload():

		if ($fileStage==ARTICLE_FILE_SUPP && ($this->unzipFile($fileName)) && ($this->gunzipFile($fileName))) return false; //this line was added to check for file type: only zip //and tar.gz files are allowed

?

I don’t the code should be adding the new blank files with the return false as you have entered it.

Unfortunately, in spite of the code:

function handleUpload($fileName, $fileStage, $fileId = null, $overwrite = false) {
if ($fileStage==ARTICLE_FILE_SUPP && ($this->unzipFile($fileName)) && ($this->gunzipFile($fileName))) return false;

added to the ArticleFileManager::handleUpload(), OJS insists on displaying blank items for files that are not zip or tar.gz files.

You can see this on the above screenshot of another testing.
What could be a problem?

Anna

Hmmm… I think AuthorSubmitSuppFileForm::execute() is not adequately checking the return status of $articleFileManager->uploadSuppFile(). If this returns false, you don’t want to create a new SuppFile(), etc.