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

diff --git a/blog/edit.php b/blog/edit.php
index a87591f942aff2fbe902abb3760331b96390367d..38bb661779e0a98f2c4da4447198fbc526ede348 100755 (executable)
--- a/blog/edit.php
+++ b/blog/edit.php
@@ -73,22 +73,22 @@ $blogeditform = new blog_edit_form(null, compact('existing', 'sitecontext'));
 
 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

diff --git a/blog/edit_form.php b/blog/edit_form.php
index cef59f4e99236b6999b2149aa4be59698345e2eb..1daddab321b976680c6c204174d67e5e5c97e7ab 100644 (file)
--- a/blog/edit_form.php
+++ b/blog/edit_form.php
@@ -12,9 +12,8 @@ class blog_edit_form extends moodleform {
                $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"');

diff --git a/blog/lib.php b/blog/lib.php
index 55fbe953922973a9a06a8a1e76ef07c25e7e3dd3..655e14bbb24562992511a0eb9d98cc4a39634d91 100755 (executable)
--- a/blog/lib.php
+++ b/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 @@
                 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 @@
                         $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 @@
 
         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

diff --git a/course/import/groups/import_form.php b/course/import/groups/import_form.php
index a6716b6eb645ddba9526a5dccb5042470f68dd7e..10c1b07a4196cceceae4da26856ddb882c106161 100755 (executable)
--- a/course/import/groups/import_form.php
+++ b/course/import/groups/import_form.php
@@ -11,7 +11,7 @@ class course_import_groups_form extends moodleform {
                $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

diff --git a/file.php b/file.php
index 6c78f789f972e8d61709547ccab88810d4409927..3191ac715c8fc967d0262b3332f22ae869b80d3b 100644 (file)
--- a/file.php
+++ b/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
     // ========================================

diff --git a/lib/formslib.php b/lib/formslib.php
index d4be25ca64124416ecb303088d393b4f0f5be415..3741cf0d37e7e542232938ec9597ab77f95673da 100644 (file)
--- a/lib/formslib.php
+++ b/lib/formslib.php
@@ -113,6 +113,7 @@ class moodleform {
         $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 @@ class moodleform {
         $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 @@ class moodleform {
                     $errors[$elname] = $this->_upload_manager->files[$elname]['uploadlog'];
                 }
             } else {
-                error('Incorrect upload attemp!');
+                error('Incorrect upload attempt!');
             }
         }
 
@@ -248,21 +244,18 @@ class moodleform {
     }
 
     /**
-     * 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 @@ class moodleform {
      * @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.
      */

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 48a3f8933d344d63b6e03c9050b91c6a054541af..2c06dcc38221ae7a3cf7794f19e612570efb2cd5 100644 (file)
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -235,6 +235,16 @@ define ('DEBUG_ALL', 2047);
 /** 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 ////////////////////////////////////////////////////
 
 /**

diff --git a/mod/forum/lib.php b/mod/forum/lib.php
index 202b7b252ba24a81046f9778b83f5201c4641fa3..f8077dc589b5ed6cabeb6908e4a23b676a5b5c60 100644 (file)
--- a/mod/forum/lib.php
+++ b/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) {

diff --git a/mod/forum/post_form.php b/mod/forum/post_form.php
index c6e78625e927dce5460c9accd5dde9bd279cf10a..89f86d2ec2d3ee4c778ad161a0c92b91a5f49ac4 100644 (file)
--- a/mod/forum/post_form.php
+++ b/mod/forum/post_form.php
@@ -17,8 +17,7 @@ class mod_forum_post_form extends moodleform {
 
 
         // 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, $forum->maxbytes, true, true);
-        $this->set_max_file_size($course, $forum->maxbytes);
+        $this->set_upload_manager(new upload_manager('attachment', true, false, $course, false, $forum->maxbytes, true, true));
 
         $mform->addElement('header', 'general', '');//fill in the data depending on page params
                                                     //later using set_defaults

diff --git a/mod/glossary/edit.php b/mod/glossary/edit.php
index dc4070a74062324eaca316ee78a0e8f018f9bf44..54cd0dc317e49c10b5adfc7d08197edb818655f6 100644 (file)
--- a/mod/glossary/edit.php
+++ b/mod/glossary/edit.php
@@ -79,15 +79,12 @@ if ($mform->is_cancelled()){
     }
 
     if ($e) {
-        /* TODO process file uploads
-        $todb->attachment = $_FILES["attachment"];
-        if ($newfilename = glossary_add_attachment($todb, 'attachment')) {
-            $todb->attachment = $newfilename;
-        } else {
-            unset($todb->attachment);
-        }*/
         $todb->id = $e;
-        print_object($todb);
+        $dir = glossary_file_area_name($todb);
+        if ($mform->save_files($dir) and $newfilename = $mform->get_new_filename()) {
+            $todb->attachment = $newfilename;
+        }
+
         if (update_record('glossary_entries', $todb)) {
             add_to_log($course->id, "glossary", "update entry",
                        "view.php?id=$cm->id&mode=entry&hook=$todb->id",
@@ -105,14 +102,10 @@ if ($mform->is_cancelled()){
 
         if ($todb->id = insert_record("glossary_entries", $todb)) {
             $e = $todb->id;
-            /* TODO process file uploads
-            $todb->attachment = $_FILES["attachment"];
-            if ($newfilename = glossary_add_attachment($todb, 'attachment')) {
-                $todb->attachment = $newfilename;
-            } else {
-                 unset($todb->attachment);
+            $dir = glossary_file_area_name($todb);
+            if ($mform->save_files($dir) and $newfilename = $mform->get_new_filename()) {
+                set_field("glossary_entries", "attachment", $newfilename, "id", $todb->id);
             }
-            set_field("glossary_entries", "attachment", $newfilename, "id", $todb->id);*/
             add_to_log($course->id, "glossary", "add entry",
                        "view.php?id=$cm->id&mode=entry&hook=$todb->id", $todb->id,$cm->id);
         } else {

diff --git a/mod/glossary/edit_form.php b/mod/glossary/edit_form.php
index a40703090e37509950ab140ff2c3f380dbc44cda..cb4ee15f2fa3373a8261294057048e006b5432a9 100644 (file)
--- a/mod/glossary/edit_form.php
+++ b/mod/glossary/edit_form.php
@@ -9,10 +9,10 @@ class mod_glossary_entry_form extends moodleform {
                $mform    =& $this->_form;
 
                $glossary =& $this->_customdata['glossary'];
-               $mode =& $this->_customdata['mode'];
-               $cm =& $this->_customdata['cm'];
-               $hook =& $this->_customdata['hook'];
-               $e =& $this->_customdata['e'];
+               $mode     =& $this->_customdata['mode'];
+               $cm       =& $this->_customdata['cm'];
+               $hook     =& $this->_customdata['hook'];
+               $e        =& $this->_customdata['e'];
 
 //-------------------------------------------------------------------------------
         $mform->addElement('header', 'general', get_string('general', 'form'));
@@ -42,8 +42,7 @@ class mod_glossary_entry_form extends moodleform {
                $mform->setType('aliases', PARAM_TEXT);
         $mform->setHelpButton('aliases', array('aliases2', strip_tags(get_string('aliases', 'glossary')), 'glossary'));
 
-        $this->set_max_file_size();
-        $this->_upload_manager = new upload_manager('attachment', true, false, $COURSE, false, 0, true, true);
+        $this->set_upload_manager(new upload_manager('attachment', true, false, $COURSE, false, 0, true, true, false));
                $mform->addElement('file', 'attachment', get_string('attachment', 'forum'));
         $mform->setHelpButton('attachment', array('attachment', get_string('attachment', 'glossary'), 'glossary'));
 

diff --git a/mod/glossary/lib.php b/mod/glossary/lib.php
index a06097a5df545daff628abade112c49b561b8147..b208dc1fe029d6556ae4f36c6819c669b3ba0f14 100644 (file)
--- a/mod/glossary/lib.php
+++ b/mod/glossary/lib.php
@@ -1150,31 +1150,6 @@ function glossary_move_attachments($entry, $glossaryid) {
     return $return;
 }
 
-function glossary_add_attachment($entry, $inputname) {
-// $entry is a full entry record, including course and glossary
-// $newfile is a full upload array from $_FILES
-// If successful, this function returns the name of the file
-
-    global $CFG;
-
-    if (!$glossary = get_record("glossary","id",$entry->glossaryid)) {
-        return false;
-    }
-
-    if (!$course = get_record("course","id",$glossary->course)) {
-        return false;
-    }
-
-    require_once($CFG->dirroot.'/lib/uploadlib.php');
-    $um = new upload_manager($inputname,true,false,$course,false,0,false,true);
-    $dir = glossary_file_area_name($entry);
-
-    if ($um->process_file_uploads($dir)) {
-        return $um->get_new_filename();
-    }
-    // upload manager will take care of errors.
-}
-
 function glossary_print_attachments($entry, $return=NULL, $align="left") {
 // if return=html, then return a html string.
 // if return=text, then return a text-only string.