diff --git a/include/ajax.draft.php b/include/ajax.draft.php
index e24be81efc5ef46f44527bbb2333bf7cdb3a859f..41fde2be24ff955b793f1fbf773b9d8372616c0b 100644
--- a/include/ajax.draft.php
+++ b/include/ajax.draft.php
@@ -67,6 +67,8 @@ class DraftAjaxAPI extends AjaxController {
}
function _uploadInlineImage($draft) {
+ global $cfg;
+
if (!isset($_POST['data']) && !isset($_FILES['file']))
Http::response(422, "File not included properly");
@@ -76,9 +78,26 @@ class DraftAjaxAPI extends AjaxController {
$_FILES['image'][$k] = array($v);
unset($_FILES['file']);
- $file = AttachmentFile::format($_FILES['image'], true);
+ $file = AttachmentFile::format($_FILES['image']);
# TODO: Detect unacceptable attachment extension
# TODO: Verify content-type and check file-content to ensure image
+ $type = $file[0]['type'];
+ if (strpos($file[0]['type'], 'image/') !== 0)
+ return Http::response(403,
+ JsonDataEncoder::encode(array(
+ 'error' => 'File type is not allowed',
+ ))
+ );
+
+ # TODO: Verify file size is acceptable
+ if ($file[0]['size'] > $cfg->getMaxFileSize())
+ return Http::response(403,
+ JsonDataEncoder::encode(array(
+ 'error' => 'File is too large',
+ ))
+ );
+
+
if (!($ids = $draft->attachments->upload($file))) {
if ($file[0]['error']) {
return Http::response(403,
diff --git a/include/ajax.forms.php b/include/ajax.forms.php
index da6bb6560724604e4603925ec12dd1fbfd971315..070606298089eb337009a6203dcddd97dcf3d6c6 100644
--- a/include/ajax.forms.php
+++ b/include/ajax.forms.php
@@ -92,14 +92,14 @@ class DynamicFormsAjaxAPI extends AjaxController {
Http::response(400, 'Upload to a non file-field');
return JsonDataEncoder::encode(
- array('id'=>$impl->upload())
+ array('id'=>$impl->ajaxUpload())
);
}
function attach() {
$field = new FileUploadField();
return JsonDataEncoder::encode(
- array('id'=>$field->upload())
+ array('id'=>$field->ajaxUpload(true))
);
}
}
diff --git a/include/api.tickets.php b/include/api.tickets.php
index ae0450a2d869c68bd2ccfcc1bd7a427acbd72458..47d978a498aac405381d4f5659f2b8072d092da6 100644
--- a/include/api.tickets.php
+++ b/include/api.tickets.php
@@ -60,30 +60,33 @@ class TicketApiController extends ApiController {
if(!parent::validate($data, $format, $strict) && $strict)
$this->exerr(400, __('Unexpected or invalid data received'));
- //Nuke attachments IF API files are not allowed.
- if(!$ost->getConfig()->allowAPIAttachments())
+ // Use the settings on the thread entry on the ticket details
+ // form to validate the attachments in the email
+ $tform = TicketForm::objects()->one()->getForm();
+ $messageField = $tform->getField('message');
+ $fileField = $messageField->getWidget()->getAttachments();
+
+ // Nuke attachments IF API files are not allowed.
+ if (!$messageField->isAttachmentsEnabled())
$data['attachments'] = array();
//Validate attachments: Do error checking... soft fail - set the error and pass on the request.
- if($data['attachments'] && is_array($data['attachments'])) {
- foreach($data['attachments'] as &$attachment) {
- if(!$ost->isFileTypeAllowed($attachment))
- $attachment['error'] = sprintf(__('Invalid file type (ext) for %s'),Format::htmlchars($attachment['name']));
- elseif ($attachment['encoding'] && !strcasecmp($attachment['encoding'], 'base64')) {
- if(!($attachment['data'] = base64_decode($attachment['data'], true)))
- $attachment['error'] = sprintf(__('%s: Poorly encoded base64 data'), Format::htmlchars($attachment['name']));
+ if ($data['attachments'] && is_array($data['attachments'])) {
+ foreach($data['attachments'] as &$file) {
+ if ($file['encoding'] && !strcasecmp($file['encoding'], 'base64')) {
+ if(!($file['data'] = base64_decode($file['data'], true)))
+ $file['error'] = sprintf(__('%s: Poorly encoded base64 data'),
+ Format::htmlchars($file['name']));
}
- if (!$attachment['error']
- && ($size = $ost->getConfig()->getMaxFileSize())
- && ($fsize = $attachment['size'] ?: strlen($attachment['data']))
- && $fsize > $size) {
- $attachment['error'] = sprintf('File %s (%s) is too big. Maximum of %s allowed',
- Format::htmlchars($attachment['name']),
- Format::file_size($fsize),
- Format::file_size($size));
+ // Validate and save immediately
+ try {
+ $file['id'] = $fileField->uploadAttachment($file);
+ }
+ catch (FileUploadError $ex) {
+ $file['error'] = $file['name'] . ': ' . $ex->getMessage();
}
}
- unset($attachment);
+ unset($file);
}
return true;
diff --git a/include/class.file.php b/include/class.file.php
index 702936c7d5204caa2ab40aa1115947ae7aff9d04..15f4ea17a0f8dc294672bfa4d695a7a12dc1ff09 100644
--- a/include/class.file.php
+++ b/include/class.file.php
@@ -534,9 +534,8 @@ class AttachmentFile {
/*
Method formats http based $_FILE uploads - plus basic validation.
- @restrict - make sure file type & size are allowed.
*/
- function format($files, $restrict=false) {
+ function format($files) {
global $ost;
if(!$files || !is_array($files))
@@ -562,16 +561,6 @@ class AttachmentFile {
$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);
diff --git a/include/class.forms.php b/include/class.forms.php
index 790a48be9c5681c34ef2d7654e42915e253e0eab..b994c738faa1b7b3989e0c0fccd800542823da53 100644
--- a/include/class.forms.php
+++ b/include/class.forms.php
@@ -997,12 +997,17 @@ class ThreadEntryField extends FormField {
'label'=>__('Enable Attachments'),
'default'=>$cfg->allowOnlineAttachments(),
'configuration'=>array(
- 'desc'=>__('Enables attachments submitted with the issue details'),
+ 'desc'=>__('Enables attachments on tickets, regardless of channel'),
),
)),
)
+ $attachments->getConfigurationOptions();
}
+
+ function isAttachmentsEnabled() {
+ $config = $this->getConfiguration();
+ return $config['attachments'];
+ }
}
class PriorityField extends ChoiceField {
@@ -1278,7 +1283,10 @@ class FileUploadField extends FormField {
);
}
- function upload() {
+ /**
+ * Called from the ajax handler for async uploads via web clients.
+ */
+ function ajaxUpload($bypass=false) {
$config = $this->getConfiguration();
$files = AttachmentFile::format($_FILES['upload'],
@@ -1289,8 +1297,12 @@ class FileUploadField extends FormField {
$file = array_shift($files);
$file['name'] = urldecode($file['name']);
- // TODO: Check allowed type / size.
- // Return HTTP/413, 415, 417 or similar
+ if (!$bypass && !$this->isValidFileType($file['name']))
+ Http::response(415, 'File type is not allowed');
+
+ $config = $this->getConfiguration();
+ if (!$bypass && $file['size'] > $config['size'])
+ Http::response(413, 'File is too large');
if (!($id = AttachmentFile::upload($file)))
Http::response(500, 'Unable to store file: '. $file['error']);
@@ -1298,6 +1310,65 @@ class FileUploadField extends FormField {
return $id;
}
+ /**
+ * Called from FileUploadWidget::getValue() when manual upload is used
+ * for browsers which do not support the HTML5 way of uploading async.
+ */
+ function uploadFile($file) {
+ if (!$this->isValidFileType($file['name']))
+ throw new FileUploadError(__('File type is not allowed'));
+
+ $config = $this->getConfiguration();
+ if ($file['size'] > $config['size'])
+ throw new FileUploadError(__('File size is too large'));
+
+ return AttachmentFile::upload($file);
+ }
+
+ /**
+ * Called from API and email routines and such to handle attachments
+ * sent other than via web upload
+ */
+ function uploadAttachment(&$file) {
+ if (!$this->isValidFileType($file['name']))
+ throw new FileUploadError(__('File type is not allowed'));
+
+ if (is_callable($file['data']))
+ $file['data'] = $file['data']();
+ if (!isset($file['size'])) {
+ // bootstrap.php include a compat version of mb_strlen
+ if (extension_loaded('mbstring'))
+ $file['size'] = mb_strlen($file['data'], '8bit');
+ else
+ $file['size'] = strlen($file['data']);
+ }
+
+ $config = $this->getConfiguration();
+ if ($file['size'] > $config['size'])
+ throw new FileUploadError(__('File size is too large'));
+
+ if (!$id = AttachmentFile::save($file))
+ throw new FileUploadError(__('Unable to save file'));
+
+ return $id;
+ }
+
+ function isValidFileType($name, $type=false) {
+ $config = $this->getConfiguration();
+
+ // Return true if all file types are allowed (.*)
+ if (strpos($config['extensions'], '.*') || !$config['extensions'])
+ return true;
+
+ $allowed = array_map('trim', explode(',', strtolower($config['extensions'])));
+
+ $ext = strtolower(pathinfo($name, PATHINFO_EXTENSION));
+
+ //TODO: Check MIME type - file ext. shouldn't be solely trusted.
+
+ return ($ext && is_array($allowed) && in_array(".$ext", $allowed));
+ }
+
function getFiles() {
if (!isset($this->attachments) && ($a = $this->getAnswer())
&& ($e = $a->getEntry()) && ($e->get('id'))
@@ -1785,8 +1856,10 @@ class FileUploadWidget extends Widget {
// Handle manual uploads (IE<10)
if ($_SERVER['REQUEST_METHOD'] == 'POST' && isset($_FILES[$this->name])) {
foreach (AttachmentFile::format($_FILES[$this->name]) as $file) {
- if ($id = AttachmentFile::upload($file))
- $ids[] = $id;
+ try {
+ $ids[] = $this->field->uploadFile($file);
+ }
+ catch (FileUploadError $ex) {}
}
return array_merge($ids, parent::getValue() ?: array());
}
@@ -1798,4 +1871,6 @@ class FileUploadWidget extends Widget {
}
}
+class FileUploadError extends Exception {}
+
?>
diff --git a/include/class.mailfetch.php b/include/class.mailfetch.php
index 105b5438d4e4c410264681554ba6ede5591efc04..45758c75c052057061c354e77e2c5faef639e4ff 100644
--- a/include/class.mailfetch.php
+++ b/include/class.mailfetch.php
@@ -665,8 +665,14 @@ class MailFetcher {
$errors=array();
$seen = false;
+ // Use the settings on the thread entry on the ticket details
+ // form to validate the attachments in the email
+ $tform = TicketForm::objects()->one()->getForm();
+ $messageField = $tform->getField('message');
+ $fileField = $messageField->getWidget()->getAttachments();
+
// Fetch attachments if any.
- if($ost->getConfig()->allowEmailAttachments()) {
+ if ($messageField->isAttachmentsEnabled()) {
// Include TNEF attachments in the attachments list
if ($this->tnef) {
foreach ($this->tnef->attachments as $at) {
@@ -680,15 +686,11 @@ class MailFetcher {
}
}
$vars['attachments'] = array();
- foreach($attachments as $a ) {
+
+ foreach ($attachments as $a) {
$file = array('name' => $a['name'], 'type' => $a['type']);
- //Check the file type
- if(!$ost->isFileTypeAllowed($file)) {
- $file['error'] = sprintf(_S('Invalid file type (ext) for %s'),
- Format::htmlchars($file['name']));
- }
- elseif (@$a['data'] instanceof TnefAttachment) {
+ if (@$a['data'] instanceof TnefAttachment) {
$file['data'] = $a['data']->getData();
}
else {
@@ -701,6 +703,15 @@ class MailFetcher {
}
// Include the Content-Id if specified (for inline images)
$file['cid'] = isset($a['cid']) ? $a['cid'] : false;
+
+ // Validate and save immediately
+ try {
+ $file['id'] = $fileField->uploadAttachment($file);
+ }
+ catch (FileUploadError $ex) {
+ $file['error'] = $file['name'] . ': ' . $ex->getMessage();
+ }
+
$vars['attachments'][] = $file;
}
}
diff --git a/include/class.osticket.php b/include/class.osticket.php
index e3901f25d34b1748db66f4a2fa416e9c786c5a87..59dc66c6c83de78b4191fb2178ebf78b141ba9f7 100644
--- a/include/class.osticket.php
+++ b/include/class.osticket.php
@@ -133,24 +133,6 @@ class osTicket {
return ($token && !strcasecmp($token, $this->getLinkToken()));
}
- function isFileTypeAllowed($file, $mimeType='') {
-
- if(!$file || !($allowedFileTypes=$this->getConfig()->getAllowedFileTypes()))
- return false;
-
- //Return true if all file types are allowed (.*)
- if(trim($allowedFileTypes)=='.*') return true;
-
- $allowed = array_map('trim', explode(',', strtolower($allowedFileTypes)));
- $filename = is_array($file)?$file['name']:$file;
-
- $ext = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
-
- //TODO: Check MIME type - file ext. shouldn't be solely trusted.
-
- return ($ext && is_array($allowed) && in_array(".$ext", $allowed));
- }
-
/* Replace Template Variables */
function replaceTemplateVariables($input, $vars=array()) {
diff --git a/include/class.thread.php b/include/class.thread.php
index f99c0f45f3e22e96f20331c1c9dc27e09d285712..f7751f78e7f9d6b2a66fee694cdeb943f321ee86 100644
--- a/include/class.thread.php
+++ b/include/class.thread.php
@@ -539,7 +539,11 @@ Class ThreadEntry {
*/
function saveAttachment(&$file) {
- if(!($fileId=is_numeric($file)?$file:AttachmentFile::save($file)))
+ if (is_numeric($file))
+ $fileId = $file;
+ elseif (is_array($file) && isset($file['id']))
+ $fileId = $file['id'];
+ elseif (!($fileId = AttachmentFile::save($file)))
return 0;
$inline = is_array($file) && @$file['inline'];