MDL-8015 improved file uploading

- changed file upload api in formslib
- fixed blog attachments and related code in file.php
- fixed glossary attachments
- fixed embedded images in forum posts and blogs - only gif, png and jpeg; the problme was that svg were embedded using img tag which was wrong, the same applied to other picture formats unsupported by browsers (please note that student submitted svg should be never embedded in moodle page for security reasons)
- other minor fixes

blog/edit.php

@@ -73,22 +73,22 @@
 
 if ($blogeditform->is_cancelled()){
     redirect($returnurl);
-} elseif ($blogeditform->no_submit_button_pressed()) {
+} else if ($blogeditform->no_submit_button_pressed()) {
     no_submit_button_actions($blogeditform, $sitecontext);
 
 
-} elseif ($fromform = $blogeditform->data_submitted()){
+} else if ($fromform = $blogeditform->data_submitted()){
     //save stuff in db
     switch ($action) {
         case 'add':
-            do_add($fromform);
+            do_add($fromform, $blogeditform);
         break;
 
         case 'edit':
             if (!$existing) {
                 error('Incorrect blog post id');
             }
-            do_edit($fromform);
+            do_edit($fromform, $blogeditform);
         break;
         default :
             error('Unknown action!');
@@ -178,6 +178,7 @@ function no_submit_button_actions(&$blogeditform, $sitecontext){
     }
     $blogeditform->otags_select_setup();
 }
+
 function delete_otags($tagids, $sitecontext){
     foreach ($tagids as $tagid) {
 
@@ -208,6 +209,7 @@ function delete_otags($tagids, $sitecontext){
 
     }
 }
+
 function add_otag($otag){
     global $USER;
     $error = '';
@@ -233,6 +235,7 @@ function add_otag($otag){
     }
     return $error;
 }
+
 /*
 * Delete blog post from database
 */
@@ -252,24 +255,9 @@ function do_delete($post) {
 /**
  * Write a new blog entry into database
  */
-function do_add($post) {
+function do_add($post, $blogeditform) {
     global $CFG, $USER, $returnurl;
 
-    if ($post->summary == '<br />') {
-        $post->summary = '';
-    }
-
-    if ($post->subject == '') {
-        $errors['subject'] = get_string('emptytitle', 'blog');
-    }
-    if ($post->summary == '') {
-        $errors['summary'] = get_string('emptybody', 'blog');
-    }
-
-    if (!empty($errors)) {
-        return; // no saving
-    }
-
     $post->module       = 'blog';
     $post->userid       = $USER->id;
     $post->lastmodified = time();
@@ -279,8 +267,9 @@ function do_add($post) {
     if ($id = insert_record('post', $post)) {
         $post->id = $id;
         // add blog attachment
-        if ($post->attachment = blog_add_attachment($post, 'attachment',$message)) {
-            set_field("post", "attachment", $post->attachment, "id", $post->id);
+        $dir = blog_file_area_name($post);
+        if ($blogeditform->save_files($dir) and $newfilename = $blogeditform->get_new_filename()) {
+            set_field("post", "attachment", $newfilename, "id", $post->id);
         }
         add_tags_info($post->id);
         add_to_log(SITEID, 'blog', 'add', 'index.php?userid='.$post->userid.'&postid='.$post->id, $post->subject);
@@ -296,19 +285,18 @@ function do_add($post) {
  * @param . $bloginfo_arg argument is reference to a blogInfo object.
  * @todo complete documenting this function. enable trackback and pingback between entries on the same server
  */
-function do_edit($post) {
+function do_edit($post, $blogeditform) {
 
     global $CFG, $USER, $returnurl;
 
 
     $post->lastmodified = time();
 
-/*  TODO add attachment processing
-    if ($newfilename = blog_add_attachment($post, 'attachment',$message)) {
+    $dir = blog_file_area_name($post);
+    if ($blogeditform->save_files($dir) and $newfilename = $blogeditform->get_new_filename()) {
         $post->attachment = $newfilename;
-    } else {
-        unset($post->attachment);
-    }*/
+    }
+
     // update record
     if (update_record('post', $post)) {
         // delete all tags associated with this entry

blog/edit_form.php

@@ -12,9 +12,8 @@ function definition() {
 		$post = $this->_customdata['existing'];
         $sitecontext = $this->_customdata['sitecontext'];
 
-        // the upload manager is used directly in post precessing, moodleform::save_files() is not used yet
-        $this->_upload_manager = new upload_manager('attachment', true, false, $COURSE, false, 0, true, true);
-        $this->set_max_file_size($COURSE);
+        // the upload manager is used directly in entry processing, moodleform::save_files() is not used yet
+        $this->set_upload_manager(new upload_manager('attachment', true, false, $COURSE, false, 0, true, true, false));
 
         $mform->addElement('header', 'general', get_string('general', 'form'));
 		$mform->addElement('text', 'subject', get_string('entrytitle', 'blog'), 'size="60"');

blog/lib.php

@@ -9,16 +9,6 @@
     require_once($CFG->dirroot .'/blog/blogpage.php');
 
 
-    /**
-     * Blog access level constant declaration
-     */
-    define ('BLOG_USER_LEVEL', 1);
-    define ('BLOG_GROUP_LEVEL', 2);
-    define ('BLOG_COURSE_LEVEL', 3);
-    define ('BLOG_SITE_LEVEL', 4);
-    define ('BLOG_GLOBAL_LEVEL', 5);
-
-
     /**
      * Definition of blogcourse page type (blog page with course id present).
      */
@@ -292,6 +282,7 @@ function blog_print_attachments($blogentry, $return=NULL) {
                 foreach ($files as $file) {
                     include_once($CFG->libdir.'/filelib.php');
                     $icon = mimeinfo("icon", $file);
+                    $type = mimeinfo("type", $file);
                     if ($CFG->slasharguments) {
                         $ffurl = "$CFG->wwwroot/file.php/$filearea/$file";
                     } else {
@@ -307,7 +298,7 @@ function blog_print_attachments($blogentry, $return=NULL) {
                         $output .= "$strattachment $file:\n$ffurl\n";
 
                     } else {
-                        if ($icon == "image.gif") {    // Image attachments don't get printed as links
+                        if (in_array($type, array('image/gif', 'image/jpeg', 'image/png'))) {    // Image attachments don't get printed as links
                             $imagereturn .= "<br /><img src=\"$ffurl\" alt=\"\" />";
                         } else {
                             echo "<a href=\"$ffurl\">$image</a> ";
@@ -324,28 +315,7 @@ function blog_print_attachments($blogentry, $return=NULL) {
 
         return $imagereturn;
     }
-
-    /**
-    * If successful, this function returns the name of the file
-    * @param $post is a full post record, including course and forum
-    * @param $newfile is a full upload array from $_FILES
-    * @param $message is a string to hold the messages.
-    */
 
-    function blog_add_attachment($blogentry, $inputname, &$message) {
-
-        global $CFG;
-
-        require_once($CFG->dirroot.'/lib/uploadlib.php');
-        $um = new upload_manager($inputname,true,false,null,false,$CFG->maxbytes,true,true);
-        $dir = blog_file_area_name($blogentry);
-        if ($um->process_file_uploads($dir)) {
-            $message .= $um->get_errors();
-            return $um->get_new_filename();
-        }
-        $message .= $um->get_errors();
-        echo $message;
-    }
 
     /**
      * Use this function to retrieve a list of publish states available for

course/import/groups/import_form.php

@@ -11,7 +11,7 @@ function definition() {
 		$maxuploadsize = $this->_customdata['maxuploadsize'];
 		$strimportgroups = get_string("importgroups");
 
-        $this->_upload_manager = new upload_manager('userfile', true, false, '', false, $maxuploadsize, true, true);
+        $this->set_upload_manager(new upload_manager('userfile', true, false, '', false, $maxuploadsize, true, true));
         $this->set_max_file_size('', $maxuploadsize);
 
         $mform->addElement('header', 'general', '');//fill in the data depending on page params

file.php

@@ -6,6 +6,11 @@
       // Workaround:  file.php?file=/courseid/dir/dir/dir/filename.ext
       // Test:        file.php/testslasharguments
 
+
+      //TODO: Blog attachments do not have access control implemented - anybody can read them!
+      //      It might be better to move the code to separate file because the access
+      //      control is quite complex - see bolg/index.php 
+
     require_once('config.php');
     require_once('lib/filelib.php');
 
@@ -37,19 +42,26 @@
     }
 
     // security: limit access to existing course subdirectories
-    // hack for blogs, needs proper security check too
-    if ((!$course = get_record_sql("SELECT * FROM {$CFG->prefix}course WHERE id='".(int)$args[0]."'")) && $args[0]!='blog') {
+    if (($args[0]!='blog') and (!$course = get_record_sql("SELECT * FROM {$CFG->prefix}course WHERE id='".(int)$args[0]."'"))) {
         error('Invalid course ID');
     }
 
     // security: prevent access to "000" or "1 something" directories
     // hack for blogs, needs proper security check too
-    if ($args[0] != $course->id && $args[0]!='blog') {
+    if (($args[0] != 'blog') and ($args[0] != $course->id)) {
         error('Invalid course ID');
     }
 
     // security: login to course if necessary
-    if ($course->id != SITEID) {
+    if ($args[0] == 'blog') {
+        if (empty($CFG->bloglevel)) {
+            error('Blogging is disabled!');
+        } else if ($CFG->bloglevel < BLOG_GLOBAL_LEVEL) {
+            require_login();
+        } else if ($CFG->forcelogin) {
+            require_login();
+        }
+    } else if ($course->id != SITEID) {
         require_login($course->id);
     } else if ($CFG->forcelogin) {
         require_login();
@@ -105,6 +117,9 @@
             )) {
         $forcedownload  = 1; // force download of all attachments
     }
+    if ($args[0] == 'blog') {
+        $forcedownload  = 1; // force download of all attachments
+    }    
 
     // security: some protection of hidden resource files
     // warning: it may break backwards compatibility
@@ -138,15 +153,6 @@
         not_found($course->id);
     }
 
-    // extra security: keep symbolic links inside dataroot/courseid if required
-    /*if (!empty($CFG->checksymlinks)) {
-        $realpath = realpath($pathname);
-        $realdataroot = realpath($CFG->dataroot.'/'.$course->id);
-        if (strpos($realpath, $realdataroot) !== 0) {
-            not_found($course->id);
-        }
-    }*/
-
     // ========================================
     // finally send the file
     // ========================================

lib/formslib.php

@@ -113,6 +113,7 @@ function moodleform($action=null, $customdata=null, $method='post', $target='',
         $this->_formname = preg_replace('/_form$/', '', get_class($this), 1);
         $this->_customdata = $customdata;
         $this->_form =& new MoodleQuickForm($this->_formname, $method, $action, $target, $attributes);
+        $this->set_upload_manager(new upload_manager());
 
         $this->definition();
 
@@ -197,11 +198,6 @@ function _validate_files() {
         $errors = array();
         $mform =& $this->_form;
 
-        // create default upload manager if not already created
-        if (empty($this->_upload_manager)) {
-            $this->_upload_manager = new upload_manager();
-        }
-
         // check the files
         $status = $this->_upload_manager->preprocess_files();
 
@@ -217,7 +213,7 @@ function _validate_files() {
                     $errors[$elname] = $this->_upload_manager->files[$elname]['uploadlog'];
                 }
             } else {
-                error('Incorrect upload attemp!');
+                error('Incorrect upload attempt!');
             }
         }
 
@@ -248,21 +244,18 @@ function set_defaults($default_values, $slashed=false) {
     }
 
     /**
-     * Set maximum allowed uploaded file size.
+     * Set custom upload manager.
      * Must be used BEFORE creating of file element!
      *
-     * @param object $course
-     * @param object $modbytes - max size limit defined in module
+     * @param object $um - custom upload manager
      */
-    function set_max_file_size($course=null, $modbytes=0) {
-        global $CFG, $COURSE;
-
-        if (empty($course->id)) {
-            $course = $COURSE;
+    function set_upload_manager($um=false) {
+        if ($um === false) {
+            $um = new upload_manager();
         }
+        $this->_upload_manager = $um;
 
-        $maxbytes = get_max_upload_file_size($CFG->maxbytes, $course->maxbytes, $modbytes);
-        $this->_form->setMaxFileSize($maxbytes);
+        $this->_form->setMaxFileSize($um->config->maxbytes);
     }
 
     /**
@@ -383,15 +376,21 @@ function data_submitted($slashed=true) {
      * @return bool success
      */
     function save_files($destination) {
-        if (empty($this->_upload_manager)) {
-            return false;
-        }
         if ($this->is_submitted() and $this->is_validated()) {
             return $this->_upload_manager->save_files($destination);
         }
         return false;
     }
 
+    /**
+     * If we're only handling one file (if inputname was given in the constructor)
+     * this will return the (possibly changed) filename of the file.
+     * @return mixed false in case of failure, string if ok
+     */
+    function get_new_filename() {
+        return $this->_upload_manager->get_new_filename();
+    }
+
     /**
      * Print html form.
      */

lib/moodlelib.php

@@ -235,6 +235,16 @@
 /** DEBUG_ALL with extra Moodle debug messages - (DEBUG_ALL | 32768) */
 define ('DEBUG_DEVELOPER', 34815);
 
+/**
+ * Blog access level constant declaration
+ */
+define ('BLOG_USER_LEVEL', 1);
+define ('BLOG_GROUP_LEVEL', 2);
+define ('BLOG_COURSE_LEVEL', 3);
+define ('BLOG_SITE_LEVEL', 4);
+define ('BLOG_GLOBAL_LEVEL', 5);
+
+
 /// PARAMETER HANDLING ////////////////////////////////////////////////////
 
 /**

mod/forum/lib.php

@@ -2575,6 +2575,7 @@ function forum_print_attachments($post, $return=NULL) {
             $strattachment = get_string("attachment", "forum");
             foreach ($files as $file) {
                 $icon = mimeinfo("icon", $file);
+                $type = mimeinfo("type", $file);
                 if ($CFG->slasharguments) {
                     $ffurl = "$CFG->wwwroot/file.php/$filearea/$file";
                 } else {
@@ -2590,7 +2591,7 @@ function forum_print_attachments($post, $return=NULL) {
                     $output .= "$strattachment $file:\n$ffurl\n";
 
                 } else {
-                    if ($icon == "image.gif") {    // Image attachments don't get printed as links
+                    if (in_array($type, array('image/gif', 'image/jpeg', 'image/png'))) {    // Image attachments don't get printed as links
                         $imagereturn .= "<br /><img src=\"$ffurl\" alt=\"\" />";
                     } else {
                         echo "<a href=\"$ffurl\">$image</a> ";
@@ -2634,6 +2635,7 @@ function forum_add_attachment($post, $inputname,&$message) {
         return $um->get_new_filename();
     }
     $message .= $um->get_errors();
+    return null;
 }
 
 function forum_add_new_post($post,&$message) {