From 76d90c5ca3549891302ca7c4e5b5045989d0d9e7 Mon Sep 17 00:00:00 2001
From: Peter Rotich <peter@osticket.com>
Date: Sun, 3 Mar 2013 19:21:13 -0500
Subject: [PATCH] Move $_FILES reformatting & validation to class.file.php
---
include/class.faq.php | 4 ++--
include/class.file.php | 47 ++++++++++++++++++++++++++++++++++++++
include/class.format.php | 13 -----------
include/class.osticket.php | 27 ----------------------
scp/canned.php | 4 ++--
tickets.php | 9 ++++----
6 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/include/class.faq.php b/include/class.faq.php
index f902dd0f0..447719fbd 100644
--- a/include/class.faq.php
+++ b/include/class.faq.php
@@ -170,7 +170,7 @@ class FAQ {
}
//Upload new attachments IF any.
- if($_FILES['attachments'] && ($files=Format::files($_FILES['attachments'])))
+ if($_FILES['attachments'] && ($files=AttachmentFile::format($_FILES['attachments'])))
$this->uploadAttachments($files);
$this->reload();
@@ -282,7 +282,7 @@ class FAQ {
if(($faq=self::lookup($id))) {
$faq->updateTopics($vars['topics']);
- if($_FILES['attachments'] && ($files=Format::files($_FILES['attachments'])))
+ if($_FILES['attachments'] && ($files=AttachmentFile::format($_FILES['attachments'])))
$faq->uploadAttachments($files);
$faq->reload();
diff --git a/include/class.file.php b/include/class.file.php
index 27a888160..6908dd8ee 100644
--- a/include/class.file.php
+++ b/include/class.file.php
@@ -210,6 +210,53 @@ class AttachmentFile {
return ($id && ($file = new AttachmentFile($id)) && $file->getId()==$id)?$file:null;
}
+
+ /*
+ Method formats http based $_FILE uploads - plus basic validation.
+ @restrict - make sure file type & size are allowed.
+ */
+ function format($files, $restrict=false) {
+ global $ost;
+
+ if(!$files || !is_array($files))
+ return null;
+
+ //Reformat $_FILE for the sane.
+ $attachments = array();
+ foreach($files as $k => $a) {
+ if(is_array($a))
+ foreach($a as $i => $v)
+ $attachments[$i][$k] = $v;
+ }
+
+ //Basic validation.
+ foreach($attachments as $i => &$file) {
+ //skip no file upload "error" - why PHP calls it an error is beyond me.
+ if($file['error'] && $file['error']==UPLOAD_ERR_NO_FILE) {
+ unset($attachments[$i]);
+ continue;
+ }
+
+ if($file['error']) //PHP defined error!
+ $file['error'] = 'File upload error #'.$file['error'];
+ elseif(!$file['tmp_name'] || !is_uploaded_file($file['tmp_name']))
+ $file['error'] = 'Invalid or bad upload POST';
+ elseif($restrict) { // make sure file type & size are allowed.
+ if(!$ost->isFileTypeAllowed($file))
+ $file['error'] = 'Invalid file type for '.Format::htmlchars($file['name']);
+ elseif($ost->getConfig()->getMaxFileSize()
+ && $file['size']>$ost->getConfig()->getMaxFileSize())
+ $file['error'] = sprintf('File %s (%s) is too big. Maximum of %s allowed',
+ Format::htmlchars($file['name']),
+ Format::file_size($file['size']),
+ Format::file_size($ost->getConfig()->getMaxFileSize()));
+ }
+ }
+ unset($file);
+
+ return array_filter($attachments);
+ }
+
/**
* Removes files and associated meta-data for files which no ticket,
* canned-response, or faq point to any more.
diff --git a/include/class.format.php b/include/class.format.php
index 220f20742..fc957172d 100644
--- a/include/class.format.php
+++ b/include/class.format.php
@@ -34,19 +34,6 @@ class Format {
return preg_replace('/\s+/', '_', $filename);
}
- /* re-arrange $_FILES array for the sane */
- function files($files) {
-
- foreach($files as $k => $a) {
- if(is_array($a))
- foreach($a as $i => $v)
- $result[$i][$k] = $v;
- }
-
- return $result?array_filter($result):$files;
- }
-
-
/* encode text into desired encoding - taking into accout charset when available. */
function encode($text, $charset=null, $encoding='utf-8') {
diff --git a/include/class.osticket.php b/include/class.osticket.php
index c357ba0e6..4445aca6c 100644
--- a/include/class.osticket.php
+++ b/include/class.osticket.php
@@ -146,33 +146,6 @@ class osTicket {
return ($ext && is_array($allowed) && in_array(".$ext", $allowed));
}
- /* Function expects a well formatted array - see Format::files()
- It's up to the caller to reject the upload on error.
- */
- function validateFileUploads(&$files, $checkFileTypes=true) {
-
- $errors=0;
- foreach($files as &$file) {
- //skip no file upload "error" - why PHP calls it an error is beyond me.
- if($file['error'] && $file['error']==UPLOAD_ERR_NO_FILE) continue;
-
- if($file['error']) //PHP defined error!
- $file['error'] = 'File upload error #'.$file['error'];
- elseif(!$file['tmp_name'] || !is_uploaded_file($file['tmp_name']))
- $file['error'] = 'Invalid or bad upload POST';
- elseif($checkFileTypes && !$this->isFileTypeAllowed($file))
- $file['error'] = 'Invalid file type for '.Format::htmlchars($file['name']);
- elseif($file['size']>$this->getConfig()->getMaxFileSize())
- $file['error'] = sprintf('File (%s) is too big. Maximum of %s allowed',
- Format::htmlchars($file['name']),
- Format::file_size($this->getConfig()->getMaxFileSize()));
-
- if($file['error']) $errors++;
- }
-
- return (!$errors);
- }
-
/* Replace Template Variables */
function replaceTemplateVariables($input, $vars=array()) {
diff --git a/scp/canned.php b/scp/canned.php
index edd4a4c36..c085e4116 100644
--- a/scp/canned.php
+++ b/scp/canned.php
@@ -44,7 +44,7 @@ if($_POST && $thisstaff->canManageCannedResponses()) {
}
}
//Upload NEW attachments IF ANY - TODO: validate attachment types??
- if($_FILES['attachments'] && ($files=Format::files($_FILES['attachments'])))
+ if($_FILES['attachments'] && ($files=AttachmentFile::format($_FILES['attachments'])))
$canned->uploadAttachments($files);
$canned->reload();
@@ -58,7 +58,7 @@ if($_POST && $thisstaff->canManageCannedResponses()) {
$msg='Canned response added successfully';
$_REQUEST['a']=null;
//Upload attachments
- if($_FILES['attachments'] && ($c=Canned::lookup($id)) && ($files=Format::files($_FILES['attachments'])))
+ if($_FILES['attachments'] && ($c=Canned::lookup($id)) && ($files=AttachmentFile::format($_FILES['attachments'])))
$c->uploadAttachments($files);
} elseif(!$errors['err']) {
diff --git a/tickets.php b/tickets.php
index d175d49b5..d1293db87 100644
--- a/tickets.php
+++ b/tickets.php
@@ -40,12 +40,11 @@ if($_POST && is_object($ticket) && $ticket->getId()):
if(!$errors) {
//Everything checked out...do the magic.
- if(($msgid=$ticket->postMessage(array('message'=>$_POST['message']), 'Web'))) {
-
- //Upload files
- if($cfg->allowOnlineAttachments() && $_FILES['attachments'])
- $ticket->uploadFiles($_FILES['attachments'], $msgid, 'M');
+ $vars = array('message'=>$_POST['message']);
+ if($cfg->allowOnlineAttachments() && $_FILES['attachments'])
+ $vars['files'] = AttachmentFile::format($_FILES['attachments'], true);
+ if(($msgid=$ticket->postMessage($vars, 'Web'))) {
$msg='Message Posted Successfully';
} else {
$errors['err']='Unable to post the message. Try again';
--
GitLab