From f212f6a794c4aff96446b99e4824a9e2c8cfb259 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Thu, 14 Feb 2013 23:42:20 +0100
Subject: #2003 - Add admin/movies screen. Added admin/movies screen analogous
 to the admin/graphics screen so the user can: - see how FFmpeg is configured
 (path and version, similar to toolkits in admin/graphics) - get some
 instructions on how to install FFmpeg if not found - change the
 movie_allow_uploads setting - ask Gallery to rebuild their movie thumbs
 Specifics: - admin_movies, admin_movies.html (new) - new Movies admin screen
 - ffmpeg.png (new) - logo for admin screen - movie::get_ffmpeg_version (new)
 - return version number and date of FFmpeg - form_uploadify.html - change
 admin message if movie uploads are disabled - gallery_event::admin_menu -
 added Movies link to Settings - xss_data.txt - updated golden file for unit
 tests

---
 modules/gallery/controllers/admin_movies.php | 72 ++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 modules/gallery/controllers/admin_movies.php

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/admin_movies.php b/modules/gallery/controllers/admin_movies.php
new file mode 100644
index 00000000..38fa44a5
--- /dev/null
+++ b/modules/gallery/controllers/admin_movies.php
@@ -0,0 +1,72 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2013 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class Admin_Movies_Controller extends Admin_Controller {
+  public function index() {
+    // Print screen from new form.
+    $form = $this->_get_admin_form();
+    $this->_print_view($form);
+  }
+
+  public function save() {
+    access::verify_csrf();
+    $form = $this->_get_admin_form();
+    if ($form->validate()) {
+      module::set_var("gallery", "movie_allow_uploads", $form->settings->allow_uploads->value);
+      if ($form->settings->rebuild_thumbs->value) {
+        graphics::mark_dirty(true, false, "movie");
+      }
+      // All done - redirect with message.
+      message::success(t("Movies settings updated successfully"));
+      url::redirect("admin/movies");
+    }
+    // Something went wrong - print view from existing form.
+    $this->_print_view($form);
+  }
+
+  private function _print_view($form) {
+    list ($ffmpeg_version, $ffmpeg_date) = movie::get_ffmpeg_version();
+    $ffmpeg_version = $ffmpeg_date ? "{$ffmpeg_version} ({$ffmpeg_date})" : $ffmpeg_version;
+    $ffmpeg_path = movie::find_ffmpeg();
+    $ffmpeg_dir = substr($ffmpeg_path, 0, strrpos($ffmpeg_path, "/"));
+
+    $view = new Admin_View("admin.html");
+    $view->page_title = t("Movies settings");
+    $view->content = new View("admin_movies.html");
+    $view->content->form = $form;
+    $view->content->ffmpeg_dir = $ffmpeg_dir;
+    $view->content->ffmpeg_version = $ffmpeg_version;
+    print $view;
+  }
+
+  private function _get_admin_form() {
+    $form = new Forge("admin/movies/save", "", "post", array("id" => "g-movies-admin-form"));
+    $group = $form->group("settings")->label(t("Settings"));
+    $group->dropdown("allow_uploads")
+      ->label(t("Allow movie uploads into Gallery (does not affect existing movies)"))
+      ->options(array("autodetect"=>t("only if FFmpeg is detected (default)"),
+                      "always"=>t("always"), "never"=>t("never")))
+      ->selected(module::get_var("gallery", "movie_allow_uploads", "autodetect"));
+    $group->checkbox("rebuild_thumbs")
+      ->label(t("Rebuild all movie thumbnails (once FFmpeg is installed, use this to update existing movie thumbnails)"))
+      ->checked(false);  // always set as false
+    $form->submit("save")->value(t("Save"));
+    return $form;
+  }
+}
-- 
cgit v1.2.3


From fd0051dab7258c817ff01877bd4324530ce79398 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sun, 17 Feb 2013 23:19:35 +0100
Subject: #2006 - Add system::mark_file_for_delete API to delete files at
 shutdown. - added system::mark_file_for_delete to be called to mark a file -
 added system::delete_marked_files to be called at shutdown to delete the list
 - amended system::temp_filename to, by default, add the temp name to the list
 - updated a few other places in code where this should be used

---
 modules/gallery/controllers/quick.php              |  1 -
 modules/gallery/controllers/uploader.php           |  2 +-
 modules/gallery/helpers/gallery_event.php          |  4 ++-
 modules/gallery/helpers/system.php                 | 32 ++++++++++++++++++++--
 modules/rest/controllers/rest.php                  |  2 +-
 modules/watermark/controllers/admin_watermarks.php |  6 ++--
 6 files changed, 38 insertions(+), 9 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php
index 2ddf2a4b..4b21d9ee 100644
--- a/modules/gallery/controllers/quick.php
+++ b/modules/gallery/controllers/quick.php
@@ -41,7 +41,6 @@ class Quick_Controller extends Controller {
       gallery_graphics::rotate($item->file_path(), $tmpfile, array("degrees" => $degrees), $item);
       $item->set_data_file($tmpfile);
       $item->save();
-      unlink($tmpfile);
     }
 
     if (Input::instance()->get("page_type") == "collection") {
diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php
index 78437071..8e09dbed 100644
--- a/modules/gallery/controllers/uploader.php
+++ b/modules/gallery/controllers/uploader.php
@@ -55,7 +55,7 @@ class Uploader_Controller extends Controller {
 
     if ($form->validate() && $file_validation->validate()) {
       $temp_filename = upload::save("Filedata");
-      Event::add("system.shutdown", create_function("", "unlink(\"$temp_filename\");"));
+      system::delete_later($temp_filename);
       try {
         $item = ORM::factory("item");
         $item->name = substr(basename($temp_filename), 10);  // Skip unique identifier Kohana adds
diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php
index 26432ef5..a319b9c6 100644
--- a/modules/gallery/helpers/gallery_event.php
+++ b/modules/gallery/helpers/gallery_event.php
@@ -44,7 +44,7 @@ class gallery_event_Core {
     // var/tmp might be stickier because theoretically we could wind up spamming that
     // dir with a lot of files.  But let's start with this and refine as we go.
     if (!(rand() % 500)) {
-      // Note that this code is roughly duplicated in gallery_event::gallery_shutdown
+      // Note that this code is roughly duplicated in gallery_task::file_cleanup
       $threshold = time() - 1209600; // older than 2 weeks
       foreach(array("logs", "tmp") as $dir) {
         $dir = VARPATH . $dir;
@@ -67,6 +67,8 @@ class gallery_event_Core {
         }
       }
     }
+    // Delete all files marked using system::delete_later.
+    system::delete_marked_files();
   }
 
   static function user_deleted($user) {
diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php
index e1398103..7d56466e 100644
--- a/modules/gallery/helpers/system.php
+++ b/modules/gallery/helpers/system.php
@@ -18,6 +18,8 @@
  * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
  */
 class system_Core {
+  private static $files_marked_for_deletion = array();
+
   /**
    * Return the path to an executable version of the named binary, or null.
    * The paths are traversed in the following order:
@@ -66,8 +68,10 @@ class system_Core {
    * This helper is similar to the built-in tempnam.
    * It allows the caller to specify a prefix and an extension.
    * It always places the file in TMPPATH.
+   * Unless specified with the $delete_later argument, it will be marked
+   * for deletion at shutdown using system::delete_later.
    */
-  static function temp_filename($prefix="", $extension="") {
+  static function temp_filename($prefix="", $extension="", $delete_later=true) {
     do {
       $basename = tempnam(TMPPATH, $prefix);
       if (!$basename) {
@@ -79,6 +83,30 @@ class system_Core {
         @unlink($basename);
       }
     } while (!$success);
+
+    if ($delete_later) {
+      system::delete_later($filename);
+    }
+
     return $filename;
   }
-}
\ No newline at end of file
+
+  /**
+   * Mark a file for deletion at shutdown time.  This is useful for temp files, where we can delay
+   * the deletion time until shutdown to keep page load time quick.
+   */
+  static function delete_later($filename) {
+    self::$files_marked_for_deletion[] = $filename;
+  }
+
+  /**
+   * Delete all files marked using system::delete_later.  This is called at gallery shutdown.
+   */
+  static function delete_marked_files() {
+    foreach (self::$files_marked_for_deletion as $filename) {
+      // We want to suppress all errors, as it's possible that some of these
+      // files may have been deleted/moved before we got here.
+      @unlink($filename);
+    }
+  }
+}
diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php
index bd03b334..54ca6fe9 100644
--- a/modules/rest/controllers/rest.php
+++ b/modules/rest/controllers/rest.php
@@ -69,7 +69,7 @@ class Rest_Controller extends Controller {
         $request->params = (object) $input->post();
         if (isset($_FILES["file"])) {
           $request->file = upload::save("file");
-          Event::add("system.shutdown", create_function("", "unlink(\"{$request->file}\");"));
+          system::delete_later($request->file);
         }
         break;
       }
diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php
index b058d6a5..222279e8 100644
--- a/modules/watermark/controllers/admin_watermarks.php
+++ b/modules/watermark/controllers/admin_watermarks.php
@@ -67,7 +67,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
     $form = watermark::get_delete_form();
     if ($form->validate()) {
       if ($name = basename(module::get_var("watermark", "name"))) {
-        @unlink(VARPATH . "modules/watermark/$name");
+        system::delete_later(VARPATH . "modules/watermark/$name");
 
         module::clear_var("watermark", "name");
         module::clear_var("watermark", "width");
@@ -108,7 +108,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
         $name = legal_file::sanitize_filename($name, $extension, "photo");
       } catch (Exception $e) {
         message::error(t("Invalid or unidentifiable image file"));
-        @unlink($file);
+        system::delete_later($file);
         return;
       }
 
@@ -120,7 +120,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
       module::set_var("watermark", "position", $form->add_watermark->position->value);
       module::set_var("watermark", "transparency", $form->add_watermark->transparency->value);
       $this->_update_graphics_rules();
-      @unlink($file);
+      system::delete_later($file);
 
       message::success(t("Watermark saved"));
       log::success("watermark", t("Watermark saved"));
-- 
cgit v1.2.3


From 94aadf03dadbfa01ba1744df60c97b6f3094ae88 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Wed, 20 Feb 2013 17:30:27 +0100
Subject: #2008 - Add warnings if some active modules are obsolete. - added
 module::get_obsolete_modules_message function - put message on
 admin/dashboard - put message on admin/modules - put message on upgrader -
 updated unit test golden file xss_data

---
 modules/gallery/controllers/admin_dashboard.php |  1 +
 modules/gallery/controllers/admin_modules.php   |  1 +
 modules/gallery/controllers/upgrader.php        |  1 +
 modules/gallery/helpers/module.php              | 30 +++++++++++++++++++
 modules/gallery/tests/xss_data.txt              | 39 +++++++++++++------------
 modules/gallery/views/admin_dashboard.html.php  |  7 +++++
 modules/gallery/views/admin_modules.html.php    |  6 ++++
 modules/gallery/views/upgrader.html.php         |  9 ++++++
 8 files changed, 76 insertions(+), 18 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/admin_dashboard.php b/modules/gallery/controllers/admin_dashboard.php
index 6bd36b07..53172109 100644
--- a/modules/gallery/controllers/admin_dashboard.php
+++ b/modules/gallery/controllers/admin_dashboard.php
@@ -26,6 +26,7 @@ class Admin_Dashboard_Controller extends Admin_Controller {
     $view->sidebar = "<div id=\"g-admin-dashboard-sidebar\">" .
       block_manager::get_html("dashboard_sidebar") .
       "</div>";
+    $view->content->obsolete_modules_message = module::get_obsolete_modules_message();
     print $view;
   }
 
diff --git a/modules/gallery/controllers/admin_modules.php b/modules/gallery/controllers/admin_modules.php
index d13ec1c6..177a925d 100644
--- a/modules/gallery/controllers/admin_modules.php
+++ b/modules/gallery/controllers/admin_modules.php
@@ -26,6 +26,7 @@ class Admin_Modules_Controller extends Admin_Controller {
     $view->page_title = t("Modules");
     $view->content = new View("admin_modules.html");
     $view->content->available = module::available();
+    $view->content->obsolete_modules_message = module::get_obsolete_modules_message();
     print $view;
   }
 
diff --git a/modules/gallery/controllers/upgrader.php b/modules/gallery/controllers/upgrader.php
index d3c6e2ec..6b3a9ef6 100644
--- a/modules/gallery/controllers/upgrader.php
+++ b/modules/gallery/controllers/upgrader.php
@@ -46,6 +46,7 @@ class Upgrader_Controller extends Controller {
     $view->available = module::available();
     $view->failed = $failed ? explode(",", $failed) : array();
     $view->done = $available_upgrades == 0;
+    $view->obsolete_modules_message = module::get_obsolete_modules_message();
     print $view;
   }
 
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index df258e87..d7429121 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -541,4 +541,34 @@ class module_Core {
   static function get_version($module_name) {
     return module::get($module_name)->version;
   }
+
+  /**
+   * Check if obsolete modules are active and, if so, return a warning message.
+   * If none are found, return null.
+   */
+  static function get_obsolete_modules_message() {
+    // This is the obsolete modules list.  Any active module that's on the list
+    // with version number at or below the one given will be considered obsolete.
+    // It is hard-coded here, and may be updated with future releases of Gallery.
+    $obsolete_modules = array("videos" => 4, "noffmpeg" => 1, "videodimensions" => 1,
+                              "digibug" => 2);
+
+    $modules_found = array();
+    foreach ($obsolete_modules as $module => $version) {
+      if (module::is_active($module) && (module::get_version($module) <= $version)) {
+        $modules_found[] = $module;
+      }
+    }
+
+    if ($modules_found) {
+      // Need this to be on one super-long line or else the localization scanner may not work.
+      // (ref: http://sourceforge.net/apps/trac/gallery/ticket/1321)
+      return t("Recent upgrades to Gallery have made the following modules obsolete: %modules. We recommend that you <a href=\"%url_mod\">deactivate</a> the module(s). For more information, please see the <a href=\"%url_doc\">documentation page</a>.",
+               array("modules" => implode(", ", $modules_found),
+                     "url_mod" => url::site("admin/modules"),
+                     "url_doc" => "http://codex.galleryproject.org/Gallery3:User_guide:Obsolete_modules"));
+    }
+
+    return null;
+  }
 }
diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 457c157f..0028ac87 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -56,7 +56,8 @@ modules/gallery/views/admin_block_photo_stream.html.php      5   DIRTY_JS $photo
 modules/gallery/views/admin_block_photo_stream.html.php      6   DIRTY    photo::img_dimensions($photo->width,$photo->height,72)
 modules/gallery/views/admin_block_photo_stream.html.php      7   DIRTY_ATTR $photo->thumb_url()
 modules/gallery/views/admin_dashboard.html.php               5   DIRTY_JS $csrf
-modules/gallery/views/admin_dashboard.html.php               35  DIRTY    $blocks
+modules/gallery/views/admin_dashboard.html.php               37  DIRTY    $obsolete_modules_message
+modules/gallery/views/admin_dashboard.html.php               42  DIRTY    $blocks
 modules/gallery/views/admin_graphics.html.php                25  DIRTY    newView("admin_graphics_none.html")
 modules/gallery/views/admin_graphics.html.php                27  DIRTY    newView("admin_graphics_$active.html",array("tk"=>$tk->$active,"is_active"=>true))
 modules/gallery/views/admin_graphics.html.php                34  DIRTY    newView("admin_graphics_$id.html",array("tk"=>$tk->$id,"is_active"=>false))
@@ -96,15 +97,16 @@ modules/gallery/views/admin_maintenance.html.php             181 DIRTY    $task-
 modules/gallery/views/admin_maintenance_show_log.html.php    8   DIRTY_JS url::site("admin/maintenance/save_log/$task->id?csrf=$csrf")
 modules/gallery/views/admin_maintenance_show_log.html.php    13  DIRTY    $task->name
 modules/gallery/views/admin_maintenance_task.html.php        75  DIRTY    $task->name
-modules/gallery/views/admin_modules.html.php                 51  DIRTY    access::csrf_form_field()
-modules/gallery/views/admin_modules.html.php                 61  DIRTY_ATTR text::alternate("g-odd","g-even")
-modules/gallery/views/admin_modules.html.php                 64  DIRTY    form::checkbox($data,'1',module::is_active($module_name))
-modules/gallery/views/admin_modules.html.php                 66  DIRTY    $module_info->version
-modules/gallery/views/admin_modules.html.php                 74  DIRTY_JS $module_info->author_url
-modules/gallery/views/admin_modules.html.php                 81  DIRTY_ATTR $module_info->author_name
-modules/gallery/views/admin_modules.html.php                 85  DIRTY    $module_info->author_name
-modules/gallery/views/admin_modules.html.php                 93  DIRTY_JS $module_info->info_url
-modules/gallery/views/admin_modules.html.php                 106 DIRTY_JS $module_info->discuss_url
+modules/gallery/views/admin_modules.html.php                 51  DIRTY    $obsolete_modules_message
+modules/gallery/views/admin_modules.html.php                 57  DIRTY    access::csrf_form_field()
+modules/gallery/views/admin_modules.html.php                 67  DIRTY_ATTR text::alternate("g-odd","g-even")
+modules/gallery/views/admin_modules.html.php                 70  DIRTY    form::checkbox($data,'1',module::is_active($module_name))
+modules/gallery/views/admin_modules.html.php                 72  DIRTY    $module_info->version
+modules/gallery/views/admin_modules.html.php                 80  DIRTY_JS $module_info->author_url
+modules/gallery/views/admin_modules.html.php                 87  DIRTY_ATTR $module_info->author_name
+modules/gallery/views/admin_modules.html.php                 91  DIRTY    $module_info->author_name
+modules/gallery/views/admin_modules.html.php                 99  DIRTY_JS $module_info->info_url
+modules/gallery/views/admin_modules.html.php                 112 DIRTY_JS $module_info->discuss_url
 modules/gallery/views/admin_modules_confirm.html.php         11  DIRTY_ATTR $css_class
 modules/gallery/views/admin_modules_confirm.html.php         11  DIRTY    $message
 modules/gallery/views/admin_modules_confirm.html.php         16  DIRTY    access::csrf_form_field()
@@ -264,14 +266,15 @@ modules/gallery/views/quick_delete_confirm.html.php          11  DIRTY    $form
 modules/gallery/views/reauthenticate.html.php                9   DIRTY    $form
 modules/gallery/views/upgrade_checker_block.html.php         19  DIRTY    $new_version
 modules/gallery/views/upgrader.html.php                      76  DIRTY_ATTR $done?"muted":""
-modules/gallery/views/upgrader.html.php                      94  DIRTY_ATTR $done?"muted":""
-modules/gallery/views/upgrader.html.php                      102 DIRTY_ATTR $module->version==$module->code_version?"current":"upgradeable"
-modules/gallery/views/upgrader.html.php                      102 DIRTY_ATTR in_array($id,$failed)?"failed":""
-modules/gallery/views/upgrader.html.php                      103 DIRTY_ATTR $id
-modules/gallery/views/upgrader.html.php                      107 DIRTY    $module->version
-modules/gallery/views/upgrader.html.php                      110 DIRTY    $module->code_version
-modules/gallery/views/upgrader.html.php                      120 DIRTY_ATTR $done?"muted":""
-modules/gallery/views/upgrader.html.php                      123 DIRTY_ATTR $done?"muted":""
+modules/gallery/views/upgrader.html.php                      97  DIRTY    $obsolete_modules_message
+modules/gallery/views/upgrader.html.php                      103 DIRTY_ATTR $done?"muted":""
+modules/gallery/views/upgrader.html.php                      111 DIRTY_ATTR $module->version==$module->code_version?"current":"upgradeable"
+modules/gallery/views/upgrader.html.php                      111 DIRTY_ATTR in_array($id,$failed)?"failed":""
+modules/gallery/views/upgrader.html.php                      112 DIRTY_ATTR $id
+modules/gallery/views/upgrader.html.php                      116 DIRTY    $module->version
+modules/gallery/views/upgrader.html.php                      119 DIRTY    $module->code_version
+modules/gallery/views/upgrader.html.php                      129 DIRTY_ATTR $done?"muted":""
+modules/gallery/views/upgrader.html.php                      132 DIRTY_ATTR $done?"muted":""
 modules/gallery/views/user_languages_block.html.php          2   DIRTY    form::dropdown("g-select-session-locale",$installed_locales,$selected)
 modules/gallery/views/user_profile.html.php                  34  DIRTY_ATTR $user->avatar_url(40,$theme->url(,true))
 modules/gallery/views/user_profile.html.php                  43  DIRTY    $info->view
diff --git a/modules/gallery/views/admin_dashboard.html.php b/modules/gallery/views/admin_dashboard.html.php
index f391547e..cf90ef28 100644
--- a/modules/gallery/views/admin_dashboard.html.php
+++ b/modules/gallery/views/admin_dashboard.html.php
@@ -31,6 +31,13 @@
     });
   });
 </script>
+<div>
+  <? if ($obsolete_modules_message): ?>
+  <p class="g-warning">
+    <?= $obsolete_modules_message ?>
+  </p>
+  <? endif ?>
+</div>
 <div id="g-admin-dashboard">
   <?= $blocks ?>
 </div>
diff --git a/modules/gallery/views/admin_modules.html.php b/modules/gallery/views/admin_modules.html.php
index 5a7f7b6c..96576ae4 100644
--- a/modules/gallery/views/admin_modules.html.php
+++ b/modules/gallery/views/admin_modules.html.php
@@ -46,6 +46,12 @@
     <?= t("Power up your Gallery by <a href=\"%url\">adding more modules</a>! Each module provides new cool features.", array("url" => "http://codex.galleryproject.org/Category:Gallery_3:Modules")) ?>
   </p>
 
+  <? if ($obsolete_modules_message): ?>
+  <p class="g-warning">
+    <?= $obsolete_modules_message ?>
+  </p>
+  <? endif ?>
+
   <div class="g-block-content">
     <form id="g-module-update-form" method="post" action="<?= url::site("admin/modules/confirm") ?>">
       <?= access::csrf_form_field() ?>
diff --git a/modules/gallery/views/upgrader.html.php b/modules/gallery/views/upgrader.html.php
index edfaf720..4c611f7e 100644
--- a/modules/gallery/views/upgrader.html.php
+++ b/modules/gallery/views/upgrader.html.php
@@ -90,6 +90,15 @@
         </div>
         <? endif ?>
 
+        <? if ($obsolete_modules_message): ?>
+        <div id="obsolete_modules_message">
+          <p>
+            <span class="failed"><?= t("Warning!") ?></span>
+            <?= $obsolete_modules_message ?>
+          </p>
+        </div>
+        <? endif ?>
+
         <table>
           <tr class="<?= $done ? "muted" : "" ?>">
             <th class="name"> <?= t("Module name") ?> </th>
-- 
cgit v1.2.3


From 251e9d5c8f727b886676e010481a6090ddac028c Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Tue, 26 Feb 2013 18:39:59 +0100
Subject: #2010 - Revise item::find_by_path to search for jpg-converted items.
 - added extra $var_subdir argument to item::find_by_path. - changed
 item::find_by_path to use $var_subdir to detect if we should look for a
 jpg-converted item or not (e.g. movie thumbs) - moved the album thumb
 detection to item::find_by_path to ensure it knows to look for an exact album
 match. - added more sanity checks to item::find_by_path (now has fewer false
 positive possibilities).

- updated file_proxy to remove the need to guess different movie files.
- updated File_Proxy_Controller - new sanity checks catch previously undetected bug.
- added additional unit tests for item::find_by_path.
---
 modules/gallery/controllers/file_proxy.php         |  20 +--
 modules/gallery/helpers/item.php                   |  81 +++++++++--
 .../gallery/tests/File_Proxy_Controller_Test.php   |   2 +-
 modules/gallery/tests/Item_Helper_Test.php         | 159 +++++++++++++++++++--
 4 files changed, 220 insertions(+), 42 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php
index 7e5d0038..ac558a71 100644
--- a/modules/gallery/controllers/file_proxy.php
+++ b/modules/gallery/controllers/file_proxy.php
@@ -66,24 +66,8 @@ class File_Proxy_Controller extends Controller {
       throw $e;
     }
 
-    // If the last element is .album.jpg, pop that off since it's not a real item
-    $path = preg_replace("|/.album.jpg$|", "", $path);
-
-    $item = item::find_by_path($path);
-    if (!$item->loaded()) {
-      // We didn't turn it up. If we're looking for a .jpg then it's it's possible that we're
-      // requesting the thumbnail for a movie.  In that case, the movie file would
-      // have been converted to a .jpg. So try some alternate types:
-      if (preg_match('/.jpg$/', $path)) {
-        foreach (legal_file::get_movie_extensions() as $ext) {
-          $movie_path = preg_replace('/.jpg$/', ".$ext", $path);
-          $item = item::find_by_path($movie_path);
-          if ($item->loaded()) {
-            break;
-          }
-        }
-      }
-    }
+    // Get the item model using the path and type (which corresponds to a var subdir)
+    $item = item::find_by_path($path, $type);
 
     if (!$item->loaded()) {
       $e = new Kohana_404_Exception();
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 9882a9c5..bbbc81d6 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -203,10 +203,18 @@ class item_Core {
   /**
    * Find an item by its path.  If there's no match, return an empty Item_Model.
    * NOTE: the caller is responsible for performing security checks on the resulting item.
+   *
+   * In addition to $path, $var_subdir can be specified ("albums", "resizes", or "thumbs").  This
+   * corresponds to the file's directory in var, which is what's used in file_proxy.  By specifying
+   * this, we can be smarter about items whose formats get converted (e.g. movies that get jpg
+   * thumbs).  If omitted, it defaults to "albums" which looks for identical matches between $path
+   * and the item name, just like pre-v3.1 behavior.
+   *
    * @param string $path
+   * @param string $var_subdir
    * @return object Item_Model
    */
-  static function find_by_path($path) {
+  static function find_by_path($path, $var_subdir="albums") {
     $path = trim($path, "/");
 
     // The root path name is NULL not "", hence this workaround.
@@ -214,35 +222,80 @@ class item_Core {
       return item::root();
     }
 
+    $search_full_name = true;
+    $album_thumb = false;
+    if (($var_subdir == "thumbs") && preg_match("|^(.*)/\.album\.jpg$|", $path, $matches)) {
+      // It's an album thumb - remove "/.album.jpg" from the path.
+      $path = $matches[1];
+      $album_thumb = true;
+    } else if (($var_subdir != "albums") && preg_match("/^(.*)\.jpg$/", $path, $matches)) {
+      // Item itself could be non-jpg (e.g. movies) - remove .jpg from path, don't search full name.
+      $path = $matches[1];
+      $search_full_name = false;
+    }
+
     // Check to see if there's an item in the database with a matching relative_path_cache value.
-    // Since that field is urlencoded, we must urlencoded the components of the path.
+    // Since that field is urlencoded, we must urlencode the components of the path.
     foreach (explode("/", $path) as $part) {
       $encoded_array[] = rawurlencode($part);
     }
     $encoded_path = join("/", $encoded_array);
-    $item = ORM::factory("item")
-      ->where("relative_path_cache", "=", $encoded_path)
-      ->find();
-    if ($item->loaded()) {
-      return $item;
+    if ($search_full_name) {
+      $item = ORM::factory("item")
+        ->where("relative_path_cache", "=", $encoded_path)
+        ->find();
+      // See if the item was found and if it should have been found.
+      if ($item->loaded() &&
+          (($var_subdir == "albums") || $item->is_photo() || $album_thumb)) {
+        return $item;
+      }
+    } else {
+      // Note that the below query uses LIKE with wildcard % at end, which is still sargable and
+      // therefore still takes advantage of the indexed relative_path_cache (i.e. still quick).
+      $item = ORM::factory("item")
+        ->where("relative_path_cache", "LIKE", Database::escape_for_like($encoded_path) . ".%")
+        ->find();
+      // See if the item was found and should be a jpg.
+      if ($item->loaded() &&
+          (($item->is_movie() && ($var_subdir == "thumbs")) ||
+           ($item->is_photo() && (preg_match("/^(.*)\.jpg$/", $item->name))))) {
+        return $item;
+      }
     }
 
     // Since the relative_path_cache field is a cache, it can be unavailable.  If we don't find
     // anything, fall back to checking the path the hard way.
     $paths = explode("/", $path);
-    foreach (ORM::factory("item")
-             ->where("name", "=", end($paths))
-             ->where("level", "=", count($paths) + 1)
-             ->find_all() as $item) {
-      if (urldecode($item->relative_path()) == $path) {
-        return $item;
+    if ($search_full_name) {
+      foreach (ORM::factory("item")
+               ->where("name", "=", end($paths))
+               ->where("level", "=", count($paths) + 1)
+               ->find_all() as $item) {
+        // See if the item was found and if it should have been found.
+        if ((urldecode($item->relative_path()) == $path) &&
+            (($var_subdir == "albums") || $item->is_photo() || $album_thumb)) {
+          return $item;
+        }
+      }
+    } else {
+      foreach (ORM::factory("item")
+               ->where("name", "LIKE", Database::escape_for_like(end($paths)) . ".%")
+               ->where("level", "=", count($paths) + 1)
+               ->find_all() as $item) {
+        // Compare relative_path without extension (regexp same as legal_file::change_extension),
+        // see if it should be a jpg.
+        if ((preg_replace("/\.[^\.\/]*?$/", "", urldecode($item->relative_path())) == $path) &&
+            (($item->is_movie() && ($var_subdir == "thumbs")) ||
+             ($item->is_photo() && (preg_match("/^(.*)\.jpg$/", $item->name))))) {
+          return $item;
+        }
       }
     }
 
+    // Nothing found - return an empty item model.
     return new Item_Model();
   }
 
-
   /**
    * Locate an item using the URL.  We assume that the url is in the form /a/b/c where each
    * component matches up with an item slug.  If there's no match, return an empty Item_Model
diff --git a/modules/gallery/tests/File_Proxy_Controller_Test.php b/modules/gallery/tests/File_Proxy_Controller_Test.php
index 562100e4..06068d62 100644
--- a/modules/gallery/tests/File_Proxy_Controller_Test.php
+++ b/modules/gallery/tests/File_Proxy_Controller_Test.php
@@ -66,7 +66,7 @@ class File_Proxy_Controller_Test extends Gallery_Unit_Test_Case {
   public function movie_thumbnails_are_jpgs_test() {
     $movie = test::random_movie();
     $name = legal_file::change_extension($movie->name, "jpg");
-    $_SERVER["REQUEST_URI"] = url::file("var/thumbs/{$movie->name}");
+    $_SERVER["REQUEST_URI"] = url::file("var/thumbs/$name");
     $controller = new File_Proxy_Controller();
     $this->assert_same($movie->thumb_path(), $controller->__call("", array()));
   }
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index f5b99bec..f4995c53 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -164,11 +164,9 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_same(item::root()->id, item::find_by_path("")->id);
 
     // Verify that we don't get confused by the part names, using the fallback code.
-    db::build()
-      ->update("items")
-      ->set(array("relative_path_cache" => null))
-      ->where("id", "IN", array($level3->id, $level3b->id))
-      ->execute();
+    self::_remove_relative_path_caches();
+    self::_remove_relative_path_caches();
+
     $this->assert_same(
       $level3->id,
       item::find_by_path("{$level1->name}/{$level2->name}/{$level3->name}")->id);
@@ -180,11 +178,154 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     // Verify that we don't get false positives
     $this->assert_false(
       item::find_by_path("foo/bar/baz")->loaded());
+  }
 
-    // Verify that the fallback code works
-    $this->assert_same(
-      $level3b->id,
-      item::find_by_path("{$level1->name}/{$level2b->name}/{$level3b->name}")->id);
+  public function find_by_path_with_jpg_test() {
+    $parent = test::random_album();
+    $jpg = test::random_photo($parent);
+
+    $jpg_path = "{$parent->name}/{$jpg->name}";
+    $flv_path = legal_file::change_extension($jpg_path, "flv");
+
+    // Check normal operation.
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "albums")->id);
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "resizes")->id);
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "thumbs")->id);
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path)->id);
+
+    // Check that we don't get false positives.
+    $this->assert_equal(null, item::find_by_path($flv_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "resizes")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path)->id);
+
+    // Check normal operation without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "albums")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "resizes")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path, "thumbs")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($jpg->id, item::find_by_path($jpg_path)->id);
+
+    // Check that we don't get false positives without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal(null, item::find_by_path($flv_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "resizes")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path)->id);
+  }
+
+  public function find_by_path_with_png_test() {
+    $parent = test::random_album();
+    $png = test::random_photo_unsaved($parent);
+    $png->set_data_file(MODPATH . "gallery/images/graphicsmagick.png");
+    $png->save();
+
+    $png_path = "{$parent->name}/{$png->name}";
+    $jpg_path = legal_file::change_extension($png_path, "jpg");
+
+    // Check normal operation.
+    $this->assert_equal($png->id, item::find_by_path($png_path, "albums")->id);
+    $this->assert_equal($png->id, item::find_by_path($png_path, "resizes")->id);
+    $this->assert_equal($png->id, item::find_by_path($png_path, "thumbs")->id);
+    $this->assert_equal($png->id, item::find_by_path($png_path)->id);
+
+    // Check that we don't get false positives.
+    $this->assert_equal(null, item::find_by_path($jpg_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path, "resizes")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path)->id);
+
+    // Check normal operation without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal($png->id, item::find_by_path($png_path, "albums")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($png->id, item::find_by_path($png_path, "resizes")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($png->id, item::find_by_path($png_path, "thumbs")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($png->id, item::find_by_path($png_path)->id);
+
+    // Check that we don't get false positives without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal(null, item::find_by_path($jpg_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path, "resizes")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path)->id);
+  }
+
+  public function find_by_path_with_flv_test() {
+    $parent = test::random_album();
+    $flv = test::random_movie($parent);
+
+    $flv_path = "{$parent->name}/{$flv->name}";
+    $jpg_path = legal_file::change_extension($flv_path, "jpg");
+
+    // Check normal operation.
+    $this->assert_equal($flv->id, item::find_by_path($flv_path, "albums")->id);
+    $this->assert_equal($flv->id, item::find_by_path($jpg_path, "thumbs")->id);
+    $this->assert_equal($flv->id, item::find_by_path($flv_path)->id);
+
+    // Check that we don't get false positives.
+    $this->assert_equal(null, item::find_by_path($jpg_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path)->id);
+
+    // Check normal operation without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal($flv->id, item::find_by_path($flv_path, "albums")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($flv->id, item::find_by_path($jpg_path, "thumbs")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($flv->id, item::find_by_path($flv_path)->id);
+
+    // Check that we don't get false positives without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal(null, item::find_by_path($jpg_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($flv_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($jpg_path)->id);
+  }
+
+  public function find_by_path_with_album_test() {
+    $parent = test::random_album();
+    $album = test::random_movie($parent);
+
+    $album_path = "{$parent->name}/{$album->name}";
+    $thumb_path = "{$album_path}/.album.jpg";
+
+    // Check normal operation.
+    $this->assert_equal($album->id, item::find_by_path($album_path, "albums")->id);
+    $this->assert_equal($album->id, item::find_by_path($thumb_path, "thumbs")->id);
+    $this->assert_equal($album->id, item::find_by_path($album_path)->id);
+
+    // Check that we don't get false positives.
+    $this->assert_equal(null, item::find_by_path($thumb_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($album_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($thumb_path)->id);
+
+    // Check normal operation without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal($album->id, item::find_by_path($album_path, "albums")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($album->id, item::find_by_path($thumb_path, "thumbs")->id);
+    self::_remove_relative_path_caches();
+    $this->assert_equal($album->id, item::find_by_path($album_path)->id);
+
+    // Check that we don't get false positives without relative path cache.
+    self::_remove_relative_path_caches();
+    $this->assert_equal(null, item::find_by_path($thumb_path, "albums")->id);
+    $this->assert_equal(null, item::find_by_path($album_path, "thumbs")->id);
+    $this->assert_equal(null, item::find_by_path($thumb_path)->id);
+  }
+
+  private function _remove_relative_path_caches() {
+    // This gets used *many* times in the find_by_path tests above to check the fallback code.
+    db::build()
+      ->update("items")
+      ->set("relative_path_cache", null)
+      ->execute();
   }
 
   public function find_by_relative_url_test() {
-- 
cgit v1.2.3


From 891a24151ee7dd197c623b4f482f260009d67096 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Wed, 27 Feb 2013 07:11:52 +0100
Subject: #2021 - Cleanup thumb_dirty and resize_dirty instances. - g2_import:
 changed "false" assignment to "0" assignment for consistency. -
 admin_theme_options: removed unused variables (they're called/used nowhere
 else). - graphics: removed stanza that clears thumb_dirty and resize_dirty
 and returns if we have no ops.  This has no effect on Gallery currently (for
 one, graphics::generate doesn't normally get called on an item with no dirty
 flags), but can inconsistently set resize_dirty of albums and movies to 0
 where it's otherwise left at 1.  Going forward, it may be useful to be
 consistent here. - gallery_installer: added v57 stanza to correct any
 resize_dirty flags of movies/albums that were previously reset to 0. -
 module.info, install.sql: update to v57

---
 installer/install.sql                               |  2 +-
 modules/g2_import/helpers/g2_import.php             |  4 ++--
 modules/gallery/controllers/admin_theme_options.php |  2 --
 modules/gallery/helpers/gallery_installer.php       | 12 ++++++++++++
 modules/gallery/helpers/graphics.php                |  6 ------
 modules/gallery/module.info                         |  2 +-
 6 files changed, 16 insertions(+), 12 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/installer/install.sql b/installer/install.sql
index b89d6b9b..f4938f6f 100644
--- a/installer/install.sql
+++ b/installer/install.sql
@@ -245,7 +245,7 @@ CREATE TABLE {modules} (
   KEY `weight` (`weight`)
 ) AUTO_INCREMENT=10 DEFAULT CHARSET=utf8;
 /*!40101 SET character_set_client = @saved_cs_client */;
-INSERT INTO {modules} VALUES (1,1,'gallery',56,1);
+INSERT INTO {modules} VALUES (1,1,'gallery',57,1);
 INSERT INTO {modules} VALUES (2,1,'user',4,2);
 INSERT INTO {modules} VALUES (3,1,'comment',7,3);
 INSERT INTO {modules} VALUES (4,1,'organize',4,4);
diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php
index 70aac747..b155a88a 100644
--- a/modules/g2_import/helpers/g2_import.php
+++ b/modules/g2_import/helpers/g2_import.php
@@ -1055,7 +1055,7 @@ class g2_import_Core {
           if (@copy(g2($derivative->fetchPath()), $item->thumb_path())) {
             $item->thumb_height = $derivative->getHeight();
             $item->thumb_width = $derivative->getWidth();
-            $item->thumb_dirty = false;
+            $item->thumb_dirty = 0;
           }
         }
 
@@ -1066,7 +1066,7 @@ class g2_import_Core {
           if (@copy(g2($derivative->fetchPath()), $item->resize_path())) {
             $item->resize_height = $derivative->getHeight();
             $item->resize_width = $derivative->getWidth();
-            $item->resize_dirty = false;
+            $item->resize_dirty = 0;
           }
         }
       }
diff --git a/modules/gallery/controllers/admin_theme_options.php b/modules/gallery/controllers/admin_theme_options.php
index aead8bae..38d2b0a8 100644
--- a/modules/gallery/controllers/admin_theme_options.php
+++ b/modules/gallery/controllers/admin_theme_options.php
@@ -34,7 +34,6 @@ class Admin_Theme_Options_Controller extends Admin_Controller {
       module::set_var("gallery", "page_size", $form->edit_theme->page_size->value);
 
       $thumb_size = $form->edit_theme->thumb_size->value;
-      $thumb_dirty = false;
       if (module::get_var("gallery", "thumb_size") != $thumb_size) {
         graphics::remove_rule("gallery", "thumb", "gallery_graphics::resize");
         graphics::add_rule(
@@ -45,7 +44,6 @@ class Admin_Theme_Options_Controller extends Admin_Controller {
       }
 
       $resize_size = $form->edit_theme->resize_size->value;
-      $resize_dirty = false;
       if (module::get_var("gallery", "resize_size") != $resize_size) {
         graphics::remove_rule("gallery", "resize", "gallery_graphics::resize");
         graphics::add_rule(
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 051a66cf..d49be83f 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -797,6 +797,18 @@ class gallery_installer {
       module::set_var("gallery", "movie_allow_uploads", "autodetect");
       module::set_version("gallery", $version = 56);
     }
+
+    if ($version == 56) {
+      // Cleanup possible instances where resize_dirty of albums or movies was set to 0.  This is
+      // unlikely to have occurred, and doesn't currently matter much since albums and movies don't
+      // have resize images anyway.  However, it may be useful to be consistent here going forward.
+      db::build()
+        ->update("items")
+        ->set("resize_dirty", 1)
+        ->where("type", "<>", "photo")
+        ->execute();
+      module::set_version("gallery", $version = 57);
+    }
   }
 
   static function uninstall() {
diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index e66908c4..459784c9 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -121,12 +121,6 @@ class graphics_Core {
     if ($item->resize_dirty && $item->is_photo()) {
       $ops["resize"] = $item->resize_path();
     }
-    if (empty($ops)) {
-      $item->thumb_dirty = 0;
-      $item->resize_dirty = 0;
-      $item->save();
-      return;
-    }
 
     try {
       foreach ($ops as $target => $output_file) {
diff --git a/modules/gallery/module.info b/modules/gallery/module.info
index 2383ec3c..7f49b72e 100644
--- a/modules/gallery/module.info
+++ b/modules/gallery/module.info
@@ -1,6 +1,6 @@
 name = "Gallery 3"
 description = "Gallery core application"
-version = 56
+version = 57
 author_name = "Gallery Team"
 author_url = "http://codex.galleryproject.org/Gallery:Team"
 info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery"
-- 
cgit v1.2.3


From 9b9f1a7b07daecf2251770e4f49838f22cb58a2a Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sat, 2 Mar 2013 13:25:10 +0100
Subject: #2031 - Add class_exists() before method_exists() if class existence
 is unknown. - fixed all instances of this in core code - deleted previous
 Zend Guard Loader workaround in MY_Kohana.php - updated Bootstrap.php to
 reflect deleted MY_Kohana.php

---
 application/Bootstrap.php                      |  2 +-
 modules/gallery/controllers/admin.php          |  2 +-
 modules/gallery/helpers/block_manager.php      | 10 +++---
 modules/gallery/helpers/module.php             | 18 +++++------
 modules/gallery/helpers/task.php               |  2 +-
 modules/gallery/libraries/Admin_View.php       |  2 +-
 modules/gallery/libraries/IdentityProvider.php |  3 +-
 modules/gallery/libraries/MY_Kohana.php        | 45 --------------------------
 modules/gallery/libraries/SafeString.php       |  2 +-
 modules/gallery/libraries/Theme_View.php       |  4 +--
 modules/gallery/tests/Html_Helper_Test.php     |  2 +-
 modules/gallery/tests/SafeString_Test.php      |  4 +--
 modules/rest/controllers/rest.php              |  2 +-
 modules/rest/helpers/rest.php                  |  6 ++--
 modules/rss/controllers/rss.php                |  2 +-
 modules/rss/helpers/rss_block.php              |  2 +-
 16 files changed, 32 insertions(+), 76 deletions(-)
 delete mode 100644 modules/gallery/libraries/MY_Kohana.php

(limited to 'modules/gallery/controllers')

diff --git a/application/Bootstrap.php b/application/Bootstrap.php
index a79ccba4..93353b47 100644
--- a/application/Bootstrap.php
+++ b/application/Bootstrap.php
@@ -35,7 +35,7 @@ require SYSPATH.'core/Event'.EXT;
 final class Event extends Event_Core {}
 
 require SYSPATH.'core/Kohana'.EXT;
-require MODPATH.'gallery/libraries/MY_Kohana'.EXT;
+final class Kohana extends Kohana_Core {}
 
 require SYSPATH.'core/Kohana_Exception'.EXT;
 require MODPATH.'gallery/libraries/MY_Kohana_Exception'.EXT;
diff --git a/modules/gallery/controllers/admin.php b/modules/gallery/controllers/admin.php
index c9d944cc..b35a9299 100644
--- a/modules/gallery/controllers/admin.php
+++ b/modules/gallery/controllers/admin.php
@@ -55,7 +55,7 @@ class Admin_Controller extends Controller {
       $method = "index";
     }
 
-    if (!method_exists($controller_name, $method)) {
+    if (!class_exists($controller_name) || !method_exists($controller_name, $method)) {
       throw new Kohana_404_Exception();
     }
 
diff --git a/modules/gallery/helpers/block_manager.php b/modules/gallery/helpers/block_manager.php
index bd6ca1c8..a2279468 100644
--- a/modules/gallery/helpers/block_manager.php
+++ b/modules/gallery/helpers/block_manager.php
@@ -35,7 +35,7 @@ class block_manager_Core {
 
   static function activate_blocks($module_name) {
     $block_class = "{$module_name}_block";
-    if (method_exists($block_class, "get_site_list")) {
+    if (class_exists($block_class) && method_exists($block_class, "get_site_list")) {
       $blocks = call_user_func(array($block_class, "get_site_list"));
       foreach (array_keys($blocks) as $block_id) {
         block_manager::add("site_sidebar", $module_name, $block_id);
@@ -61,14 +61,14 @@ class block_manager_Core {
 
   static function deactivate_blocks($module_name) {
     $block_class = "{$module_name}_block";
-    if (method_exists($block_class, "get_site_list")) {
+    if (class_exists($block_class) && method_exists($block_class, "get_site_list")) {
       $blocks = call_user_func(array($block_class, "get_site_list"));
       foreach  (array_keys($blocks) as $block_id) {
         block_manager::remove_blocks_for_module("site_sidebar", $module_name);
       }
     }
 
-    if (method_exists($block_class, "get_admin_list")) {
+    if (class_exists($block_class) && method_exists($block_class, "get_admin_list")) {
       $blocks = call_user_func(array($block_class, "get_admin_list"));
       foreach (array("dashboard_sidebar", "dashboard_center") as $location) {
         block_manager::remove_blocks_for_module($location, $module_name);
@@ -89,7 +89,7 @@ class block_manager_Core {
 
     foreach (module::active() as $module) {
       $class_name = "{$module->name}_block";
-      if (method_exists($class_name, $function)) {
+      if (class_exists($class_name) && method_exists($class_name, $function)) {
         foreach (call_user_func(array($class_name, $function)) as $id => $title) {
           $blocks["{$module->name}:$id"] = $title;
         }
@@ -102,7 +102,7 @@ class block_manager_Core {
     $active = block_manager::get_active($location);
     $result = "";
     foreach ($active as $id => $desc) {
-      if (method_exists("$desc[0]_block", "get")) {
+      if (class_exists("$desc[0]_block") && method_exists("$desc[0]_block", "get")) {
         $block = call_user_func(array("$desc[0]_block", "get"), $desc[1], $theme);
         if (!empty($block)) {
           $block->id = $id;
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index d7429121..da201d20 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -141,7 +141,7 @@ class module_Core {
     $messages = array();
 
     $installer_class = "{$module_name}_installer";
-    if (method_exists($installer_class, "can_activate")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "can_activate")) {
       $messages = call_user_func(array($installer_class, "can_activate"));
     }
 
@@ -173,7 +173,7 @@ class module_Core {
     module::_add_to_path($module_name);
 
     $installer_class = "{$module_name}_installer";
-    if (method_exists($installer_class, "install")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "install")) {
       call_user_func_array(array($installer_class, "install"), array());
     }
     module::set_version($module_name, module::available()->$module_name->code_version);
@@ -226,7 +226,7 @@ class module_Core {
     $version_before = module::get_version($module_name);
     $installer_class = "{$module_name}_installer";
     $available = module::available();
-    if (method_exists($installer_class, "upgrade")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "upgrade")) {
       call_user_func_array(array($installer_class, "upgrade"), array($version_before));
     } else {
       if (isset($available->$module_name->code_version)) {
@@ -261,7 +261,7 @@ class module_Core {
     module::_add_to_path($module_name);
 
     $installer_class = "{$module_name}_installer";
-    if (method_exists($installer_class, "activate")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "activate")) {
       call_user_func_array(array($installer_class, "activate"), array());
     }
 
@@ -288,7 +288,7 @@ class module_Core {
    */
   static function deactivate($module_name) {
     $installer_class = "{$module_name}_installer";
-    if (method_exists($installer_class, "deactivate")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "deactivate")) {
       call_user_func_array(array($installer_class, "deactivate"), array());
     }
 
@@ -314,7 +314,7 @@ class module_Core {
    */
   static function uninstall($module_name) {
     $installer_class = "{$module_name}_installer";
-    if (method_exists($installer_class, "uninstall")) {
+    if (class_exists($installer_class) && method_exists($installer_class, "uninstall")) {
       call_user_func(array($installer_class, "uninstall"));
     }
 
@@ -403,7 +403,7 @@ class module_Core {
         continue;
       }
       $class = "{$module->name}_event";
-      if (method_exists($class, $function)) {
+      if (class_exists($class) && method_exists($class, $function)) {
         call_user_func_array(array($class, $function), $args);
       }
     }
@@ -411,7 +411,7 @@ class module_Core {
     // Give the admin theme a chance to respond, if we're in admin mode.
     if (theme::$is_admin) {
       $class = theme::$admin_theme_name . "_event";
-      if (method_exists($class, $function)) {
+      if (class_exists($class) && method_exists($class, $function)) {
         call_user_func_array(array($class, $function), $args);
       }
     }
@@ -419,7 +419,7 @@ class module_Core {
     // Give the site theme a chance to respond as well.  It gets a chance even in admin mode, as
     // long as the theme has an admin subdir.
     $class = theme::$site_theme_name . "_event";
-    if (method_exists($class, $function)) {
+    if (class_exists($class) && method_exists($class, $function)) {
       call_user_func_array(array($class, $function), $args);
     }
   }
diff --git a/modules/gallery/helpers/task.php b/modules/gallery/helpers/task.php
index 32fd9739..5638faf4 100644
--- a/modules/gallery/helpers/task.php
+++ b/modules/gallery/helpers/task.php
@@ -25,7 +25,7 @@ class task_Core {
     $tasks = array();
     foreach (module::active() as $module) {
       $class_name = "{$module->name}_task";
-      if (method_exists($class_name, "available_tasks")) {
+      if (class_exists($class_name) && method_exists($class_name, "available_tasks")) {
         foreach (call_user_func(array($class_name, "available_tasks")) as $task) {
           $tasks[$task->callback] = $task;
         }
diff --git a/modules/gallery/libraries/Admin_View.php b/modules/gallery/libraries/Admin_View.php
index 83163868..62645d18 100644
--- a/modules/gallery/libraries/Admin_View.php
+++ b/modules/gallery/libraries/Admin_View.php
@@ -95,7 +95,7 @@ class Admin_View_Core extends Gallery_View {
       $blocks = array();
       foreach (module::active() as $module) {
         $helper_class = "{$module->name}_theme";
-        if (method_exists($helper_class, $function)) {
+        if (class_exists($helper_class) && method_exists($helper_class, $function)) {
           $blocks[] = call_user_func_array(
             array($helper_class, $function),
             array_merge(array($this), $args));
diff --git a/modules/gallery/libraries/IdentityProvider.php b/modules/gallery/libraries/IdentityProvider.php
index 23368a6a..525e1695 100644
--- a/modules/gallery/libraries/IdentityProvider.php
+++ b/modules/gallery/libraries/IdentityProvider.php
@@ -81,7 +81,8 @@ class IdentityProvider_Core {
 
       module::set_var("gallery", "identity_provider", $new_provider);
 
-      if (method_exists("{$new_provider}_installer", "initialize")) {
+      if (class_exists("{$new_provider}_installer") &&
+          method_exists("{$new_provider}_installer", "initialize")) {
         call_user_func("{$new_provider}_installer::initialize");
       }
 
diff --git a/modules/gallery/libraries/MY_Kohana.php b/modules/gallery/libraries/MY_Kohana.php
deleted file mode 100644
index d344c8ed..00000000
--- a/modules/gallery/libraries/MY_Kohana.php
+++ /dev/null
@@ -1,45 +0,0 @@
-<?php defined("SYSPATH") or die("No direct script access.");
-/**
- * Gallery - a web based photo album viewer and editor
- * Copyright (C) 2000-2013 Bharat Mediratta
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or (at
- * your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
- */
-final class Kohana extends Kohana_Core {
-  /**
-   * Wrapper function for Kohana::auto_load that provides compatibility with Zend Guard Loader's
-   * code obfuscation.  Zend Guard is enabled by default on many PHP 5.3+ installations and can
-   * cause problems with Kohana 2.4.  When a class is not found, Zend Guard Loader may continue to
-   * try and load the class, eventually leading to a seg fault.
-   *
-   * Instead, if we can't find the class and we can see that code obfuscation is at level 3+, let's
-   * load a dummy class.  This does not change the return value, so Kohana still knows that
-   * there is no class.
-   *
-   * This is based on the patch described here: http://blog.teatime.com.tw/1/post/403
-   */
-  public static function auto_load($class) {
-    $found = parent::auto_load($class);
-
-    if (!$found && function_exists("zend_current_obfuscation_level") &&
-        (zend_current_obfuscation_level() >= 3)) {
-      // Load a dummy class instead.
-      eval("class $class {}");
-    }
-
-    // Return the same result.
-    return $found;
-  }
-}
\ No newline at end of file
diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php
index 31e9d31b..179cbd41 100644
--- a/modules/gallery/libraries/SafeString.php
+++ b/modules/gallery/libraries/SafeString.php
@@ -153,7 +153,7 @@ class SafeString_Core {
    * Purify the string, removing any potentially malicious or unsafe HTML / JavaScript.
    */
   private static function _purify_for_html($dirty_html) {
-    if (method_exists("purifier", "purify")) {
+    if (class_exists("purifier") && method_exists("purifier", "purify")) {
       return purifier::purify($dirty_html);
     } else {
       return self::_escape_for_html($dirty_html);
diff --git a/modules/gallery/libraries/Theme_View.php b/modules/gallery/libraries/Theme_View.php
index 986fc8a2..0a4c96e1 100644
--- a/modules/gallery/libraries/Theme_View.php
+++ b/modules/gallery/libraries/Theme_View.php
@@ -239,7 +239,7 @@ class Theme_View_Core extends Gallery_View {
           continue;
         }
         $helper_class = "{$module->name}_theme";
-        if (method_exists($helper_class, $function)) {
+        if (class_exists($helper_class) && method_exists($helper_class, $function)) {
           $blocks[] = call_user_func_array(
             array($helper_class, $function),
             array_merge(array($this), $args));
@@ -247,7 +247,7 @@ class Theme_View_Core extends Gallery_View {
       }
 
       $helper_class = theme::$site_theme_name . "_theme";
-      if (method_exists($helper_class, $function)) {
+      if (class_exists($helper_class) && method_exists($helper_class, $function)) {
         $blocks[] = call_user_func_array(
           array($helper_class, $function),
           array_merge(array($this), $args));
diff --git a/modules/gallery/tests/Html_Helper_Test.php b/modules/gallery/tests/Html_Helper_Test.php
index 476faa5a..4643e6fd 100644
--- a/modules/gallery/tests/Html_Helper_Test.php
+++ b/modules/gallery/tests/Html_Helper_Test.php
@@ -27,7 +27,7 @@ class Html_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function purify_test() {
     $safe_string = html::purify("hello <p  >world</p>");
-    $expected = method_exists("purifier", "purify")
+    $expected = (class_exists("purifier") && method_exists("purifier", "purify"))
       ? "hello <p>world</p>"
       : "hello &lt;p  &gt;world&lt;/p&gt;";
     $this->assert_equal($expected, $safe_string->unescaped());
diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php
index 946410d4..dab7d7df 100644
--- a/modules/gallery/tests/SafeString_Test.php
+++ b/modules/gallery/tests/SafeString_Test.php
@@ -91,7 +91,7 @@ class SafeString_Test extends Gallery_Unit_Test_Case {
 
   public function purify_test() {
     $safe_string = SafeString::purify("hello <p  >world</p>");
-    $expected = method_exists("purifier", "purify")
+    $expected = (class_exists("purifier") && method_exists("purifier", "purify"))
       ? "hello <p>world</p>"
       : "hello &lt;p  &gt;world&lt;/p&gt;";
     $this->assert_equal($expected, $safe_string);
@@ -100,7 +100,7 @@ class SafeString_Test extends Gallery_Unit_Test_Case {
   public function purify_twice_test() {
     $safe_string = SafeString::purify("hello <p  >world</p>");
     $safe_string_2 = SafeString::purify($safe_string);
-    $expected = method_exists("purifier", "purify")
+    $expected = (class_exists("purifier") && method_exists("purifier", "purify"))
       ? "hello <p>world</p>"
       : "hello &lt;p  &gt;world&lt;/p&gt;";
     $this->assert_equal($expected, $safe_string_2);
diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php
index 54ca6fe9..b3d59e0f 100644
--- a/modules/rest/controllers/rest.php
+++ b/modules/rest/controllers/rest.php
@@ -98,7 +98,7 @@ class Rest_Controller extends Controller {
       $handler_class = "{$function}_rest";
       $handler_method = $request->method;
 
-      if (!method_exists($handler_class, $handler_method)) {
+      if (!class_exists($handler_class) || !method_exists($handler_class, $handler_method)) {
         throw new Rest_Exception("Bad Request", 400);
       }
 
diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php
index 9b367feb..c6be1e1d 100644
--- a/modules/rest/helpers/rest.php
+++ b/modules/rest/helpers/rest.php
@@ -141,7 +141,7 @@ class rest_Core {
     }
 
     $class = "$components[1]_rest";
-    if (!method_exists($class, "resolve")) {
+    if (!class_exists($class) || !method_exists($class, "resolve")) {
       throw new Kohana_404_Exception($url);
     }
 
@@ -158,7 +158,7 @@ class rest_Core {
     $resource_type = array_shift($args);
 
     $class = "{$resource_type}_rest";
-    if (!method_exists($class, "url")) {
+    if (!class_exists($class) || !method_exists($class, "url")) {
       throw new Rest_Exception("Bad Request", 400);
     }
 
@@ -178,7 +178,7 @@ class rest_Core {
     foreach (module::active() as $module) {
       foreach (glob(MODPATH . "{$module->name}/helpers/*_rest.php") as $filename) {
         $class = str_replace(".php", "", basename($filename));
-        if (method_exists($class, "relationships")) {
+        if (class_exists($class) && method_exists($class, "relationships")) {
           if ($tmp = call_user_func(array($class, "relationships"), $resource_type, $resource)) {
             $results = array_merge($results, $tmp);
           }
diff --git a/modules/rss/controllers/rss.php b/modules/rss/controllers/rss.php
index 12461325..571995b3 100644
--- a/modules/rss/controllers/rss.php
+++ b/modules/rss/controllers/rss.php
@@ -32,7 +32,7 @@ class Rss_Controller extends Controller {
     // Run the appropriate feed callback
     if (module::is_active($module_id)) {
       $class_name = "{$module_id}_rss";
-      if (method_exists($class_name, "feed")) {
+      if (class_exists($class_name) && method_exists($class_name, "feed")) {
         $feed = call_user_func(
           array($class_name, "feed"), $feed_id,
           ($page - 1) * $page_size, $page_size, $id);
diff --git a/modules/rss/helpers/rss_block.php b/modules/rss/helpers/rss_block.php
index 74334e93..9a77b05d 100644
--- a/modules/rss/helpers/rss_block.php
+++ b/modules/rss/helpers/rss_block.php
@@ -29,7 +29,7 @@ class rss_block_Core {
       $feeds = array();
       foreach (module::active() as $module) {
         $class_name = "{$module->name}_rss";
-        if (method_exists($class_name, "available_feeds")) {
+        if (class_exists($class_name) && method_exists($class_name, "available_feeds")) {
           $feeds = array_merge($feeds,
             call_user_func(array($class_name, "available_feeds"), $theme->item(), $theme->tag()));
         }
-- 
cgit v1.2.3


From 4b28478776071bf764545de767b70939484519aa Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Tue, 5 Mar 2013 16:54:41 +0100
Subject: #2040 - Deactivate modules that no longer exist. - added
 module::deactivate_missing_modules() - revised module::deactivate() to change
 log message if the module is missing - added call to new function in
 module::get_obsolete_modules_message() - added call to new function when
 loading admin/maintenance menu

---
 modules/gallery/controllers/admin_maintenance.php |  1 +
 modules/gallery/helpers/module.php                | 24 +++++++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/admin_maintenance.php b/modules/gallery/controllers/admin_maintenance.php
index 23df33ee..32f20784 100644
--- a/modules/gallery/controllers/admin_maintenance.php
+++ b/modules/gallery/controllers/admin_maintenance.php
@@ -55,6 +55,7 @@ class Admin_Maintenance_Controller extends Admin_Controller {
       ->where("expiration", "<>", 0)
       ->where("expiration", "<=", time())
       ->execute();
+    module::deactivate_missing_modules();
   }
 
   /**
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index da201d20..1b6c8d1a 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -303,8 +303,25 @@ class module_Core {
 
     block_manager::deactivate_blocks($module_name);
 
-    log::success(
-      "module", t("Deactivated module %module_name", array("module_name" => $module_name)));
+    if (module::info($module_name)) {
+      log::success(
+        "module", t("Deactivated module %module_name", array("module_name" => $module_name)));
+    } else {
+      log::success(
+        "module", t("Deactivated missing module %module_name", array("module_name" => $module_name)));
+    }
+  }
+
+  /**
+   * Deactivate modules that are unavailable or missing, yet still active.
+   * This happens when a user deletes a module without deactivating it.
+   */
+  static function deactivate_missing_modules() {
+    foreach (self::$modules as $module_name => $module) {
+      if (module::is_active($module_name) && !module::info($module_name)) {
+        module::deactivate($module_name);
+      }
+    }
   }
 
   /**
@@ -553,6 +570,9 @@ class module_Core {
     $obsolete_modules = array("videos" => 4, "noffmpeg" => 1, "videodimensions" => 1,
                               "digibug" => 2);
 
+    // Before we check the active modules, deactivate any that are missing.
+    module::deactivate_missing_modules();
+
     $modules_found = array();
     foreach ($obsolete_modules as $module => $version) {
       if (module::is_active($module) && (module::get_version($module) <= $version)) {
-- 
cgit v1.2.3


From 74532b9c948c471115239fa1a0b54c10ccdbeb9c Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 5 Mar 2013 13:20:12 -0500
Subject: Remove stray line of debug output.

---
 modules/gallery/controllers/file_proxy.php | 1 -
 1 file changed, 1 deletion(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/file_proxy.php b/modules/gallery/controllers/file_proxy.php
index ac558a71..50f1aa26 100644
--- a/modules/gallery/controllers/file_proxy.php
+++ b/modules/gallery/controllers/file_proxy.php
@@ -146,7 +146,6 @@ class File_Proxy_Controller extends Controller {
       // going to buffer up whatever file we're proxying (and it may be very large).  This may
       // affect embedding or systems with PHP's output_buffering enabled.
       while (ob_get_level()) {
-        Kohana_Log::add("error","".print_r(ob_get_level(),1));
         if (!@ob_end_clean()) {
           // ob_end_clean() can return false if the buffer can't be removed for some reason
           // (zlib output compression buffers sometimes cause problems).
-- 
cgit v1.2.3


From a14030346897989b4a94931a8c1a1c9b38523ab2 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Wed, 13 Mar 2013 16:18:12 +0100
Subject: #2061 - Remove Uploadify from add_photo_form/add_photo_form_completed
 events. - Added code around event calls in uploader to add Uploadify-specific
 JS to update the inputs. - Removed Uploadify JS from tag_event.  Now it's
 uploader-agnostic. - Refactored tag_event autocomplete code (no functional
 changes).

---
 modules/gallery/controllers/uploader.php | 19 +++++++++++++--
 modules/tag/helpers/tag_event.php        | 40 ++++++++++----------------------
 2 files changed, 29 insertions(+), 30 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php
index 8e09dbed..c708db92 100644
--- a/modules/gallery/controllers/uploader.php
+++ b/modules/gallery/controllers/uploader.php
@@ -123,10 +123,25 @@ class Uploader_Controller extends Controller {
       ->label(t("Add photos to %album_title", array("album_title" => html::purify($album->title))));
     $group->uploadify("uploadify")->album($album);
 
-    $group = $form->group("actions");
-    $group->uploadify_buttons("");
+    $group_actions = $form->group("actions");
+    $group_actions->uploadify_buttons("");
 
+    $inputs_before_event = array_keys($form->add_photos->inputs);
     module::event("add_photos_form", $album, $form);
+    $inputs_after_event = array_keys($form->add_photos->inputs);
+
+    // For each new input in add_photos, attach JS to make uploadify update its value.
+    foreach (array_diff($inputs_after_event, $inputs_before_event) as $input) {
+      if (!$input) {
+        // Likely a script input - don't do anything with it.
+        continue;
+      }
+      $group->uploadify->script_data($input, $group->{$input}->value);
+      $group->script("")
+        ->text("$('input[name=\"$input\"]').change(function (event) {
+                  $('#g-uploadify').uploadifySettings('scriptData', {'$input': $(this).val()});
+                });");
+    }
 
     return $form;
   }
diff --git a/modules/tag/helpers/tag_event.php b/modules/tag/helpers/tag_event.php
index 08d5d53a..927153aa 100644
--- a/modules/tag/helpers/tag_event.php
+++ b/modules/tag/helpers/tag_event.php
@@ -69,18 +69,14 @@ class tag_event_Core {
   }
 
   static function item_edit_form($item, $form) {
-    $url = url::site("tags/autocomplete");
-    $form->script("")
-      ->text("$('form input[name=tags]').ready(function() {
-                $('form input[name=tags]').gallery_autocomplete('$url', {multiple: true});
-              });");
-
     $tag_names = array();
     foreach (tag::item_tags($item) as $tag) {
       $tag_names[] = $tag->name;
     }
     $form->edit_item->input("tags")->label(t("Tags (comma separated)"))
       ->value(implode(", ", $tag_names));
+
+    $form->script("")->text(self::_get_autocomplete_js());
   }
 
   static function item_edit_form_completed($item, $form) {
@@ -110,38 +106,21 @@ class tag_event_Core {
 
   static function add_photos_form($album, $form) {
     $group = $form->add_photos;
-    if (!is_object($group->uploadify)) {
-      return;
-    }
 
     $group->input("tags")
       ->label(t("Add tags to all uploaded files"))
       ->value("");
-    $group->uploadify->script_data("tags", "");
-
-    $autocomplete_url = url::site("tags/autocomplete");
-    $group->script("")
-      ->text("$('input[name=tags]')
-                .gallery_autocomplete(
-                  '$autocomplete_url',
-                  {multiple: true}
-                );
-              $('input[name=tags]')
-                .change(function (event) {
-                  $('#g-uploadify').uploadifySettings('scriptData', {'tags': $(this).val()});
-                });");
+
+    $group->script("")->text(self::_get_autocomplete_js());
   }
 
-  static function add_photos_form_completed($album, $form) {
+  static function add_photos_form_completed($item, $form) {
     $group = $form->add_photos;
-    if (!is_object($group->uploadify)) {
-      return;
-    }
 
-    foreach (explode(",", $form->add_photos->tags->value) as $tag_name) {
+    foreach (explode(",", $group->tags->value) as $tag_name) {
       $tag_name = trim($tag_name);
       if ($tag_name) {
-        $tag = tag::add($album, $tag_name);
+        $tag = tag::add($item, $tag_name);
       }
     }
   }
@@ -161,4 +140,9 @@ class tag_event_Core {
       $block->content->metadata = $info;
     }
   }
+
+  private static function _get_autocomplete_js() {
+    $url = url::site("tags/autocomplete");
+    return "$('input[name=\"tags\"]').gallery_autocomplete('$url', {multiple: true});";
+  }
 }
-- 
cgit v1.2.3


From 26e4f044bb861f478719388fc9fb503fa16af2d5 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Thu, 14 Mar 2013 18:15:24 +0100
Subject: #2058 - Separate uploader logic into Kohana-, Item-Model-, and
 Uploadify-specific functions. - Added _process_upload() to handle upload file
 validation with Kohana. - Added _add_item() to handle item creation and
 validation with the item model (and logs). - Removed these pieces from
 add_photo(), which is now rather Uploadify-specific. - Used $_FILES to get
 filename instead of assuming it's substr(basename($tmp_name), 10). - No net
 functional changes - works just like before.

---
 modules/gallery/controllers/uploader.php | 137 +++++++++++++++++++++----------
 1 file changed, 95 insertions(+), 42 deletions(-)

(limited to 'modules/gallery/controllers')

diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php
index c708db92..54e1476b 100644
--- a/modules/gallery/controllers/uploader.php
+++ b/modules/gallery/controllers/uploader.php
@@ -47,52 +47,25 @@ class Uploader_Controller extends Controller {
 
     $form = $this->_get_add_form($album);
 
-    // Uploadify adds its own field to the form, so validate that separately.
-    $file_validation = new Validation($_FILES);
-    $file_validation->add_rules(
-      "Filedata", "upload::valid",  "upload::required",
-      "upload::type[" . implode(",", legal_file::get_extensions()) . "]");
-
-    if ($form->validate() && $file_validation->validate()) {
-      $temp_filename = upload::save("Filedata");
-      system::delete_later($temp_filename);
+    if ($form->validate()) {
+      // Uploadify puts the result in $_FILES["Filedata"] - process it.
       try {
-        $item = ORM::factory("item");
-        $item->name = substr(basename($temp_filename), 10);  // Skip unique identifier Kohana adds
-        $item->title = item::convert_filename_to_title($item->name);
-        $item->parent_id = $album->id;
-        $item->set_data_file($temp_filename);
-
-        $path_info = @pathinfo($temp_filename);
-        if (array_key_exists("extension", $path_info) &&
-            legal_file::get_movie_extensions($path_info["extension"])) {
-          $item->type = "movie";
-          $item->save();
-          log::success("content", t("Added a movie"),
-                       html::anchor("movies/$item->id", t("view movie")));
-        } else {
-          $item->type = "photo";
-          $item->save();
-          log::success("content", t("Added a photo"),
-                       html::anchor("photos/$item->id", t("view photo")));
-        }
+        list ($tmp_name, $name) = $this->_process_upload("Filedata");
+      } catch (Exception $e) {
+        header("HTTP/1.1 400 Bad Request");
+        print "ERROR: " . $e->getMessage();
+        return;
+      }
 
+      // We have a valid upload file (of unknown type) - build an item from it.
+      try {
+        $item = $this->_add_item($id, $tmp_name, $name);
         module::event("add_photos_form_completed", $item, $form);
+        print "FILEID: $item->id";
       } catch (Exception $e) {
-        // The Flash uploader has no good way of reporting complex errors, so just keep it simple.
-        Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString());
-
-        // Ugh.  I hate to use instanceof, But this beats catching the exception separately since
-        // we mostly want to treat it the same way as all other exceptions
-        if ($e instanceof ORM_Validation_Exception) {
-          Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1));
-        }
-
         header("HTTP/1.1 500 Internal Server Error");
         print "ERROR: " . $e->getMessage();
-        return;
       }
-      print "FILEID: $item->id";
     } else {
       header("HTTP/1.1 400 Bad Request");
       print "ERROR: " . t("Invalid upload");
@@ -107,17 +80,17 @@ class Uploader_Controller extends Controller {
                (int)$success_count,
                array("error" => (int)$error_count));
     } else {
-      print t2("Uploaded %count photo", "Uploaded %count photos", $success_count);}
+      print t2("Uploaded %count photo", "Uploaded %count photos", $success_count);
+    }
   }
 
   public function finish() {
     access::verify_csrf();
-
     batch::stop();
     json::reply(array("result" => "success"));
   }
 
-  private function _get_add_form($album)  {
+  private function _get_add_form($album) {
     $form = new Forge("uploader/finish", "", "post", array("id" => "g-add-photos-form"));
     $group = $form->group("add_photos")
       ->label(t("Add photos to %album_title", array("album_title" => html::purify($album->title))));
@@ -145,4 +118,84 @@ class Uploader_Controller extends Controller {
 
     return $form;
   }
+
+  /**
+   * Process the uploaded file.  This handles the interface with Kohana's upload and validation
+   * code, and marks the new temp file for deletion on shutdown.  It returns the temp file path
+   * (tmp_name) and filename (name), analogous to their respective $_FILES elements.
+   * If the upload is invalid, it will throw an exception.  Note that no type-checking (e.g. jpg,
+   * mp4,...) is performed here.
+   * @TODO: consider moving this to a common controller which is extended by various uploaders.
+   *
+   * @param  string name of $_FILES input
+   * @return array  array($tmp_name, $name)
+   */
+  private function _process_upload($file) {
+    // Validate file data.  At this point, any file extension is still valid.
+    $file_validation = new Validation($_FILES);
+    $file_validation->add_rules($file, "upload::valid",  "upload::required");
+    if (!$file_validation->validate()) {
+      throw new Exception(t("Invalid upload"));
+    }
+
+    // Save temp file and mark for deletion when done.
+    $tmp_name = upload::save($file);
+    system::delete_later($tmp_name);
+
+    // Get uploaded filename.  This is different than tmp_name since it hasn't been uniquified.
+    $name = $_FILES[$file]["name"];
+
+    return array($tmp_name, $name);
+  }
+
+  /**
+   * Add photo or movie from upload.  Once we have a valid file, this generates an item model
+   * from it.  It returns the item model on success, and throws an exception and adds log entries
+   * on failure.
+   * @TODO: consider moving this to a common controller which is extended by various uploaders.
+   *
+   * @param  int    parent album id
+   * @param  string temp file path (analogous to $_FILES[...]["tmp_name"])
+   * @param  string filename       (analogous to $_FILES[...]["name"])
+   * @return object new item model
+   */
+  private function _add_item($album_id, $tmp_name, $name) {
+    $extension = pathinfo($name, PATHINFO_EXTENSION);
+
+    try {
+      $item = ORM::factory("item");
+      $item->name = $name;
+      $item->title = item::convert_filename_to_title($name);
+      $item->parent_id = $album_id;
+      $item->set_data_file($tmp_name);
+
+      if (!$extension) {
+        throw new Exception(t("Uploaded file has no extension"));
+      } else if (legal_file::get_photo_extensions($extension)) {
+        $item->type = "photo";
+        $item->save();
+        log::success("content", t("Added a photo"),
+                     html::anchor("photos/$item->id", t("view photo")));
+      } else if (movie::allow_uploads() && legal_file::get_movie_extensions($extension)) {
+        $item->type = "movie";
+        $item->save();
+        log::success("content", t("Added a movie"),
+                     html::anchor("movies/$item->id", t("view movie")));
+      } else {
+        throw new Exception(t("Uploaded file has illegal extension"));
+      }
+    } catch (Exception $e) {
+      // Log errors then re-throw exception.
+      Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString());
+
+      // If we have a validation error, add an additional log entry.
+      if ($e instanceof ORM_Validation_Exception) {
+        Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1));
+      }
+
+      throw $e;
+    }
+
+    return $item;
+  }
 }
-- 
cgit v1.2.3