From 27be4ae6064b8609a641f4bf239999d5a8ec2c60 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sun, 10 Feb 2013 10:10:44 +0100
Subject: Follow-on to 0312d1b071bd4434ddb3f82888b0323da6bf3732 for #1994. -
 Updated function comments to match revisions. - No functional changes.

---
 modules/gallery/helpers/movie.php | 4 +---
 modules/gallery/helpers/photo.php | 6 ++----
 2 files changed, 3 insertions(+), 7 deletions(-)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index d4b907a2..637d8ac0 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -138,9 +138,7 @@ class movie_Core {
    *     -> return metadata from ffmpeg
    *   Input is *not* standard movie type that is *not* supported by ffmpeg but is legal
    *     -> return zero width, height, and duration; mime type and extension according to legal_file
-   *   Input is *not* standard movie type that is *not* supported by ffmpeg and is *not* legal
-   *     -> return zero width, height, and duration; null mime type and extension
-   *   Input is not readable or does not exist
+   *   Input is illegal, unidentifiable, unreadable, or does not exist
    *     -> throw exception
    * Note: movie_get_file_metadata events can change any of the above cases (except the last one).
    */
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 2d32f0d3..004cc7c4 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -94,10 +94,8 @@ class photo_Core {
    *   Input is *not* standard photo type that is supported by getimagesize (e.g. tif, bmp...)
    *     -> return metadata from getimagesize()
    *   Input is *not* standard photo type that is *not* supported by getimagesize but is legal
-   *     -> return zero width and height, mime type and extension according to legal_file
-   *   Input is *not* standard photo type that is *not* supported by getimagesize and is *not* legal
-   *     -> return zero width and height, null mime type and extension
-   *   Input is not readable or does not exist
+   *     -> return metadata if found by photo_get_file_metadata events
+   *   Input is illegal, unidentifiable, unreadable, or does not exist
    *     -> throw exception
    * Note: photo_get_file_metadata events can change any of the above cases (except the last one).
    */
-- 
cgit v1.2.3


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 ++++++++++++++++++++++++++
 modules/gallery/helpers/gallery_event.php     |   4 ++
 modules/gallery/helpers/movie.php             |  28 ++++++++++
 modules/gallery/images/ffmpeg.png             | Bin 0 -> 2888 bytes
 modules/gallery/tests/xss_data.txt            |   1 +
 modules/gallery/views/admin_movies.html.php   |  44 ++++++++++++++++
 modules/gallery/views/form_uploadify.html.php |   2 +-
 7 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 modules/gallery/controllers/admin_movies.php
 create mode 100644 modules/gallery/images/ffmpeg.png
 create mode 100644 modules/gallery/views/admin_movies.html.php

(limited to 'modules/gallery/helpers')

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;
+  }
+}
diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php
index aeb1c7eb..54c60296 100644
--- a/modules/gallery/helpers/gallery_event.php
+++ b/modules/gallery/helpers/gallery_event.php
@@ -398,6 +398,10 @@ class gallery_event_Core {
                         ->id("graphics_toolkits")
                         ->label(t("Graphics"))
                         ->url(url::site("admin/graphics")))
+               ->append(Menu::factory("link")
+                        ->id("movies_settings")
+                        ->label(t("Movies"))
+                        ->url(url::site("admin/movies")))
                ->append(Menu::factory("link")
                         ->id("languages")
                         ->label(t("Languages"))
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 863e1cf9..aff2acc1 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -146,6 +146,34 @@ class movie_Core {
     return $ffmpeg_path;
   }
 
+  /**
+   * Return version number and build date of ffmpeg if found, empty string(s) if not.  When using
+   * static builds that aren't official releases, the version numbers are strange, hence why the
+   * date can be useful.
+   */
+  static function get_ffmpeg_version() {
+    $ffmpeg = movie::find_ffmpeg();
+    if (empty($ffmpeg)) {
+      return array("", "");
+    }
+
+    // Find version using -h argument since -version wasn't available in early versions.
+    // To keep the preg_match searches quick, we'll trim the (otherwise long) result.
+    $cmd = escapeshellcmd($ffmpeg) . " -h 2>&1";
+    $result = substr(`$cmd`, 0, 1000);
+    if (preg_match("/ffmpeg version (\S+)/i", $result, $matches_version)) {
+      // Version number found - see if we can get the build date or copyright year as well.
+      if (preg_match("/built on (\S+\s\S+\s\S+)/i", $result, $matches_build_date)) {
+        return array(trim($matches_version[1], ","), trim($matches_build_date[1], ","));
+      } else if (preg_match("/copyright \S*\s?2000-(\d{4})/i", $result, $matches_copyright_date)) {
+        return array(trim($matches_version[1], ","), $matches_copyright_date[1]);
+      } else {
+        return array(trim($matches_version[1], ","), "");
+      }
+    }
+    return array("", "");
+  }
+
   /**
    * Return the width, height, mime_type, extension and duration of the given movie file.
    * Metadata is first generated using ffmpeg (or set to defaults if it fails),
diff --git a/modules/gallery/images/ffmpeg.png b/modules/gallery/images/ffmpeg.png
new file mode 100644
index 00000000..6be8b62a
Binary files /dev/null and b/modules/gallery/images/ffmpeg.png differ
diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 51347f86..67a8b948 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -111,6 +111,7 @@ modules/gallery/views/admin_modules_confirm.html.php         11  DIRTY_ATTR $css
 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()
 modules/gallery/views/admin_modules_confirm.html.php         18  DIRTY    form::hidden($module,1)
+modules/gallery/views/admin_movies.html.php                  43  DIRTY    $form
 modules/gallery/views/admin_sidebar.html.php                 50  DIRTY    $available
 modules/gallery/views/admin_sidebar.html.php                 58  DIRTY    $active
 modules/gallery/views/admin_sidebar_blocks.html.php          4   DIRTY_ATTR $ref
diff --git a/modules/gallery/views/admin_movies.html.php b/modules/gallery/views/admin_movies.html.php
new file mode 100644
index 00000000..e7810711
--- /dev/null
+++ b/modules/gallery/views/admin_movies.html.php
@@ -0,0 +1,44 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<div id="g-movies-admin" class="g-block ui-helper-clearfix">
+  <h1> <?= t("Movies settings") ?> </h1>
+  <p>
+    <?= t("Gallery comes with everything it needs to upload and play movies.") ?>
+    <?= t("However, it needs the FFmpeg toolkit to extract thumbnails and size information from them.") ?>
+  </p>
+  <p>
+    <?= t("Although popular, FFmpeg is not installed on all Linux systems.") ?>
+    <?= t("To use FFmpeg without fully installing it, download a pre-compiled, <b>static build</b> of FFmpeg from one of the links <a href=\"%url\">here</a>.", array("url" => "http://ffmpeg.org/download.html")) ?>
+    <?= t("Then, put the \"ffmpeg\" file in Gallery's \"bin\" directory (e.g. \"/gallery/bin\"), where Gallery will auto-detect it.") ?>
+  </p>
+  <p>
+    <?= t("Movies will work without FFmpeg, but their thumbnails will be placeholders.") ?>
+  </p>
+  <p>
+    <?= t("Can't get FFmpeg configured on your system? <a href=\"%url\">We can help!</a>",
+          array("url" => "http://codex.galleryproject.org/Gallery3:FAQ#Why_does_it_say_I.27m_missing_ffmpeg.3F")) ?>
+  </p>
+
+  <div class="g-available">
+    <h2> <?= t("Current configuration") ?> </h2>
+    <div id="g-ffmpeg" class="g-block">
+      <img class="logo" width="284" height="70" src="<?= url::file("modules/gallery/images/ffmpeg.png"); ?>" alt="<? t("Visit the FFmpeg project site") ?>" />
+      <p>
+        <?= t("FFmpeg is a cross-platform standalone audio/video program.") ?><br/>
+        <?= t("Please refer to the <a href=\"%url\">FFmpeg website</a> for more information.", array("url" => "http://ffmpeg.org")) ?>
+      </p>
+      <div class="g-module-status g-info">
+        <? if ($ffmpeg_dir): ?>
+          <? if ($ffmpeg_version): ?>
+            <p><?= t("FFmpeg version %version was found in %dir", array("version" => $ffmpeg_version, "dir" => $ffmpeg_dir)) ?></p>
+          <? else: ?>
+            <p><?= t("FFmpeg (of unknown version) was found in %dir", array("dir" => $ffmpeg_dir)) ?></p>
+          <? endif ?>
+        <? else: ?>
+          <p><?= t("We could not locate FFmpeg on your system.") ?></p>
+        <? endif ?>
+      </div>
+    </div>
+  </div>
+
+  <?= $form ?>
+</div>
diff --git a/modules/gallery/views/form_uploadify.html.php b/modules/gallery/views/form_uploadify.html.php
index 4426514a..c13e3418 100644
--- a/modules/gallery/views/form_uploadify.html.php
+++ b/modules/gallery/views/form_uploadify.html.php
@@ -131,7 +131,7 @@
 
     <? if (identity::active_user()->admin && !$movies_allowed): ?>
     <p class="g-warning">
-      <?= t("Can't find <i>ffmpeg</i> on your system. Movie uploading disabled. <a href=\"%help_url\">Help!</a>", array("help_url" => "http://codex.galleryproject.org/Gallery3:FAQ#Why_does_it_say_I.27m_missing_ffmpeg.3F")) ?>
+      <?= t("Movie uploading is disabled on your system. <a href=\"%help_url\">Help!</a>", array("help_url" => url::site("admin/movies"))) ?>
     </p>
     <? endif ?>
   </div>
-- 
cgit v1.2.3


From 7bdccade98e76df3a7830bc45bc42321a77c709a Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Fri, 15 Feb 2013 19:09:06 -0500
Subject: Every 500th request prune a single old file from var/tmp and
 var/logs. Might not be aggressive enough.  Fixes #2005.

---
 modules/gallery/helpers/gallery_event.php | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php
index 54c60296..eb54e521 100644
--- a/modules/gallery/helpers/gallery_event.php
+++ b/modules/gallery/helpers/gallery_event.php
@@ -36,6 +36,34 @@ class gallery_event_Core {
     locales::set_request_locale();
   }
 
+  static function gallery_shutdown() {
+    // Every 500th request, do a pass over var/logs and var/tmp and delete old files.
+    // Limit ourselves to deleting a single file so that we don't spend too much CPU
+    // time on it.  As long as servers call this at least twice a day they'll eventually
+    // wind up with a clean var/logs directory because we only create 1 file a day there.
+    // 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
+      $threshold = time() - 1209600; // older than 2 weeks
+      foreach(array("logs", "tmp") as $dir) {
+        $dir = VARPATH . $dir;
+        if ($dh = opendir($dir)) {
+          while (($file = readdir($dh)) !== false) {
+            if ($file[0] == ".") {
+              continue;
+            }
+
+            if (filemtime("$dir/$file") <= $threshold) {
+              unlink("$dir/$file");
+              break;
+            }
+          }
+        }
+      }
+    }
+  }
+
   static function user_deleted($user) {
     $admin = identity::admin_user();
     if (!empty($admin)) {          // could be empty if there is not identity provider
-- 
cgit v1.2.3


From 0dd12caa6f4c0452167b0e621a01bee28e71b0af Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Fri, 15 Feb 2013 19:12:08 -0500
Subject: Follow-on to 7bdccade98e76df3a7830bc45bc42321a77c709a - point out
 that there's duplicated code in gallery_event.

---
 modules/gallery/helpers/gallery_task.php | 1 +
 1 file changed, 1 insertion(+)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php
index 856d2639..37de2f93 100644
--- a/modules/gallery/helpers/gallery_task.php
+++ b/modules/gallery/helpers/gallery_task.php
@@ -281,6 +281,7 @@ class gallery_task_Core {
       switch ($task->get("mode", "init")) {
       case "init":
         $threshold = time() - 1209600; // older than 2 weeks
+        // Note that this code is roughly duplicated in gallery_event::gallery_shutdown
         foreach(array("logs", "tmp") as $dir) {
           $dir = VARPATH . $dir;
           if ($dh = opendir($dir)) {
-- 
cgit v1.2.3


From 96ad7789c84b5b81320f9cb94b4c8cb4e10ad92f Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Fri, 15 Feb 2013 16:15:40 -0800
Subject: Follow-on to 7bdccade98e76df3a7830bc45bc42321a77c709a to exclude
 directories for now.

---
 modules/gallery/helpers/gallery_event.php | 5 +++++
 modules/gallery/helpers/gallery_task.php  | 5 +++++
 2 files changed, 10 insertions(+)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php
index eb54e521..26432ef5 100644
--- a/modules/gallery/helpers/gallery_event.php
+++ b/modules/gallery/helpers/gallery_event.php
@@ -54,6 +54,11 @@ class gallery_event_Core {
               continue;
             }
 
+            // Ignore directories for now, but we should really address them in the long term.
+            if (is_dir("$dir/$file")) {
+              continue;
+            }
+
             if (filemtime("$dir/$file") <= $threshold) {
               unlink("$dir/$file");
               break;
diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php
index 37de2f93..a79cb2d5 100644
--- a/modules/gallery/helpers/gallery_task.php
+++ b/modules/gallery/helpers/gallery_task.php
@@ -290,6 +290,11 @@ class gallery_task_Core {
                 continue;
               }
 
+              // Ignore directories for now, but we should really address them in the long term.
+              if (is_dir("$dir/$file")) {
+                continue;
+              }
+
               if (filemtime("$dir/$file") <= $threshold) {
                 $files[] = "$dir/$file";
               }
-- 
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/helpers')

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/helpers')

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/helpers')

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/helpers')

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/helpers')

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 e7b224461a4137666f97eb23c063bbb2bf5fdc7a Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sun, 3 Mar 2013 00:16:38 +0100
Subject: #2034 - Add webm and ogv as valid movie types. - added them to
 legal_file helper - revised unit tests

---
 modules/gallery/helpers/legal_file.php           |  3 ++-
 modules/gallery/tests/Legal_File_Helper_Test.php | 14 +++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index eb9c25de..f8547011 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -70,7 +70,8 @@ class legal_file_Core {
     if (empty(self::$movie_types_by_extension)) {
       $types_by_extension_wrapper = new stdClass();
       $types_by_extension_wrapper->types_by_extension = array(
-        "flv" => "video/x-flv", "mp4" => "video/mp4", "m4v" => "video/x-m4v");
+        "flv" => "video/x-flv", "mp4" => "video/mp4", "m4v" => "video/x-m4v",
+        "webm" => "video/webm", "ogv" => "video/ogg");
       module::event("movie_types_by_extension", $types_by_extension_wrapper);
       foreach (self::$blacklist as $key) {
         unset($types_by_extension_wrapper->types_by_extension[$key]);
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 7ed5214b..3f520131 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -37,7 +37,7 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal(null, legal_file::get_movie_types_by_extension("php.flv")); // invalid w/ .
 
     // No extension returns full array
-    $this->assert_equal(3, count(legal_file::get_movie_types_by_extension()));
+    $this->assert_equal(5, count(legal_file::get_movie_types_by_extension()));
   }
 
   public function get_types_by_extension_test() {
@@ -47,7 +47,7 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal(null, legal_file::get_types_by_extension("php.flv"));      // invalid w/ .
 
     // No extension returns full array
-    $this->assert_equal(7, count(legal_file::get_types_by_extension()));
+    $this->assert_equal(9, count(legal_file::get_types_by_extension()));
   }
 
   public function get_photo_extensions_test() {
@@ -69,7 +69,7 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal(false, legal_file::get_movie_extensions("php.jpg")); // invalid w/ .
 
     // No extension returns full array
-    $this->assert_equal(3, count(legal_file::get_movie_extensions()));
+    $this->assert_equal(5, count(legal_file::get_movie_extensions()));
   }
 
   public function get_extensions_test() {
@@ -79,12 +79,12 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal(false, legal_file::get_extensions("php.jpg")); // invalid w/ .
 
     // No extension returns full array
-    $this->assert_equal(7, count(legal_file::get_extensions()));
+    $this->assert_equal(9, count(legal_file::get_extensions()));
   }
 
   public function get_filters_test() {
-    // All 7 extensions both uppercase and lowercase
-    $this->assert_equal(14, count(legal_file::get_filters()));
+    // All 9 extensions both uppercase and lowercase
+    $this->assert_equal(18, count(legal_file::get_filters()));
   }
 
   public function get_photo_types_test() {
@@ -94,7 +94,7 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function get_movie_types_test() {
     // Note that this is one *more* than movie extensions since video/flv is added.
-    $this->assert_equal(4, count(legal_file::get_movie_types()));
+    $this->assert_equal(6, count(legal_file::get_movie_types()));
   }
 
   public function change_extension_test() {
-- 
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/helpers')

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 18f38f0dff93665267bd1497b196d9270fdbdc88 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sat, 9 Mar 2013 12:51:45 +0100
Subject: #2046 - Change Gallery over to using MediaElementJS as its movie
 player.

---
 modules/gallery/helpers/gallery_theme.php        |  4 ++
 modules/gallery/models/item.php                  | 72 ++++++++++++------------
 modules/gallery/tests/xss_data.txt               | 14 ++---
 modules/gallery/views/movieplayer-flash.html.php | 50 ----------------
 modules/gallery/views/movieplayer.html.php       | 17 ++++++
 themes/wind/css/screen.css                       |  2 +-
 6 files changed, 66 insertions(+), 93 deletions(-)
 delete mode 100644 modules/gallery/views/movieplayer-flash.html.php
 create mode 100644 modules/gallery/views/movieplayer.html.php

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/gallery_theme.php b/modules/gallery/helpers/gallery_theme.php
index 3c6d71e9..e5f6b0b4 100644
--- a/modules/gallery/helpers/gallery_theme.php
+++ b/modules/gallery/helpers/gallery_theme.php
@@ -49,6 +49,10 @@ class gallery_theme_Core {
         . $theme->script("l10n_client.js");
     }
 
+    // Add MediaElementJS library
+    $buf .= $theme->script("mediaelementjs/mediaelement.js");
+    $buf .= $theme->script("mediaelementjs/mediaelementplayer.js");
+    $buf .= $theme->css("mediaelementjs/mediaelementplayer.css");
     $buf .= $theme->css("uploadify/uploadify.css");
     return $buf;
   }
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index e8afaec3..1e16d307 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -737,40 +737,42 @@ class Item_Model_Core extends ORM_MPTT {
   }
 
   /**
-   * Return a view for movies.  By default this is a Flowplayer v3 <script> tag, but
-   * movie_img events can override this and provide their own player/view.  If no player/view
-   * is found and the movie is unsupported by Flowplayer v3, this returns a simple download link.
+   * Return a view for movies.  By default, this uses MediaElementPlayer on an HTML5-compliant
+   * <video> object, but movie_img events can override this and provide their own player/view.
+   * If none are found and the player can't play the movie, this returns a simple download link.
    * @param array $extra_attrs
    * @return string
    */
   public function movie_img($extra_attrs) {
-    $max_size = module::get_var("gallery", "resize_size", 640);
+    $player_width = module::get_var("gallery", "resize_size", 640);
     $width = $this->width;
     $height = $this->height;
     if ($width == 0 || $height == 0) {
-      // Not set correctly, likely because ffmpeg isn't available.  Making the window 0x0 causes the
-      // video to be effectively unviewable.  So, let's guess: set width to max_size and guess a
-      // height (using 4:3 aspect ratio).  Once the video metadata is loaded, js in
-      // movieplayer-flash.html.php will correct these values.
-      $width = $max_size;
+      // Not set correctly, likely because FFmpeg isn't available.  Making the window 0x0 causes the
+      // player to be unviewable during loading.  So, let's guess: set width to player_width and
+      // guess a height (using 4:3 aspect ratio).  Once the video metadata is loaded, the player
+      // will correct these values.
+      $width = $player_width;
       $height = ceil($width * 3/4);
     }
-    $attrs = array_merge(array("id" => "g-item-id-{$this->id}"), $extra_attrs,
-                         array("class" => "g-movie"));
+    $div_attrs = array_merge(array("id" => "g-item-id-{$this->id}"), $extra_attrs,
+                             array("class" => "g-movie", "style" => "width: {$player_width}px;"));
 
     // Run movie_img events, which can either:
-    //  - generate a view, which is used in place of the standard Flowplayer v3 player
+    //  - generate a view, which is used in place of the standard MediaElementPlayer
     //    (use view variable)
-    //  - alter the arguments sent to the standard player
-    //    (use fp_flash_params and fp_flash_config variables)
+    //  - change the file sent to the player
+    //    (use width, height, url, and filename variables)
+    //  - alter the arguments sent to the player
+    //    (use video_attrs and player_options variables)
     $movie_img = new stdClass();
-    $movie_img->max_size = $max_size;
     $movie_img->width = $width;
     $movie_img->height = $height;
-    $movie_img->attrs = $attrs;
     $movie_img->url = $this->file_url(true);
-    $movie_img->fp_flash_params = array(); // add'l Flowplayer params values (will be json encoded)
-    $movie_img->fp_flash_config = array(); // add'l Flowplayer config values (will be json encoded)
+    $movie_img->filename = $this->name;
+    $movie_img->div_attrs = $div_attrs;   // attrs for the outer .g-movie <div>
+    $movie_img->video_attrs = array();    // add'l <video> attrs
+    $movie_img->player_options = array(); // add'l MediaElementPlayer options (will be json encoded)
     $movie_img->view = array();
     module::event("movie_img", $movie_img, $this);
 
@@ -778,26 +780,26 @@ class Item_Model_Core extends ORM_MPTT {
       // View generated - use it
       $view = implode("\n", $movie_img->view);
     } else {
-      // View NOT generated - see if filetype supported by Flowplayer v3
-      // Note that the extension list below is hard-coded and doesn't use the legal_file helper
-      // since anything else will not work in Flowplayer v3.
-      if (in_array(strtolower(pathinfo($this->name, PATHINFO_EXTENSION)),
-                   array("flv", "mp4", "m4v", "mov", "f4v"))) {
-        // Filetype supported by Flowplayer v3 - use it (default)
-        $view = new View("movieplayer-flash.html");
-        $view->max_size = $movie_img->max_size;
+      // View not generated - see if the filetype is supported by MediaElementPlayer.
+      // Note that the extension list below doesn't use the legal_file helper but rather
+      // is hard-coded based on player specifications.
+      $extension = strtolower(pathinfo($movie_img->filename, PATHINFO_EXTENSION));
+      if (in_array($extension, array("webm", "ogv", "mp4", "flv", "m4v", "mov", "f4v", "wmv"))) {
+        // Filetype supported by MediaElementPlayer - use it (default)
+        $view = new View("movieplayer.html");
         $view->width = $movie_img->width;
         $view->height = $movie_img->height;
-        $view->attrs = $movie_img->attrs;
-        $view->url = $movie_img->url;
-        $view->fp_flash_params = $movie_img->fp_flash_params;
-        $view->fp_flash_config = $movie_img->fp_flash_config;
+        $view->div_attrs = $movie_img->div_attrs;
+        $view->video_attrs = array_merge(array("controls" => "controls", "autoplay" => "autoplay",
+                                         "style" => "max-width: 100%"), $movie_img->video_attrs);
+        $view->source_attrs = array("type" => legal_file::get_movie_types_by_extension($extension),
+                                    "src" => $movie_img->url);
+        $view->player_options = $movie_img->player_options;
       } else {
-        // Filetype NOT supported by Flowplayer v3 - display download link
-        $attrs = array_merge($attrs, array("style" => "width: {$max_size}px;",
-                                           "download" => $this->name, // forces download (HTML5 only)
-                                           "class" => "g-movie g-movie-download-link"));
-        $view = html::anchor($this->file_url(true), t("Click here to download item."), $attrs);
+        // Filetype not supported by MediaElementPlayer - display download link
+        $div_attrs["class"] .= " g-movie-download-link"; // add class
+        $div_attrs["download"] = $movie_img->filename;   // force download (HTML5 only)
+        $view = html::anchor($movie_img->url, t("Click here to download item."), $div_attrs);
       }
     }
     return $view;
diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 9c796999..7e77a70b 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -223,13 +223,13 @@ modules/gallery/views/menu_dialog.html.php                   5   DIRTY_JS $menu-
 modules/gallery/views/menu_link.html.php                     3   DIRTY    $menu->css_id?"id='{$menu->css_id}'":""
 modules/gallery/views/menu_link.html.php                     4   DIRTY_ATTR $menu->css_class
 modules/gallery/views/menu_link.html.php                     5   DIRTY_JS $menu->url
-modules/gallery/views/movieplayer-flash.html.php             3   DIRTY    html::anchor($url,"",$attrs)
-modules/gallery/views/movieplayer-flash.html.php             5   DIRTY_JS $attrs["id"]
-modules/gallery/views/movieplayer-flash.html.php             6   DIRTY_JS $max_size
-modules/gallery/views/movieplayer-flash.html.php             24  DIRTY_JS url::abs_file("lib/flowplayer-flash/flowplayer.swf")
-modules/gallery/views/movieplayer-flash.html.php             31  DIRTY_JS url::abs_file("lib/flowplayer-flash/flowplayer.pseudostreaming-byterange.swf")
-modules/gallery/views/movieplayer-flash.html.php             49  DIRTY_JS $width
-modules/gallery/views/movieplayer-flash.html.php             49  DIRTY_JS $height
+modules/gallery/views/movieplayer.html.php                   2   DIRTY    html::attributes($div_attrs)
+modules/gallery/views/movieplayer.html.php                   3   DIRTY    html::attributes($video_attrs)
+modules/gallery/views/movieplayer.html.php                   4   DIRTY    html::attributes($source_attrs)
+modules/gallery/views/movieplayer.html.php                   8   DIRTY_JS $div_attrs["id"]
+modules/gallery/views/movieplayer.html.php                   10  DIRTY_JS $width
+modules/gallery/views/movieplayer.html.php                   11  DIRTY_JS $height
+modules/gallery/views/movieplayer.html.php                   14  DIRTY_JS url::abs_file("lib/mediaelementjs/")
 modules/gallery/views/permissions_browse.html.php            3   DIRTY_JS url::site("permissions/form/__ITEM__")
 modules/gallery/views/permissions_browse.html.php            16  DIRTY_JS url::site("permissions/change/__CMD__/__GROUP__/__PERM__/__ITEM__?csrf=$csrf")
 modules/gallery/views/permissions_browse.html.php            43  DIRTY_ATTR $parent->id
diff --git a/modules/gallery/views/movieplayer-flash.html.php b/modules/gallery/views/movieplayer-flash.html.php
deleted file mode 100644
index 12303bf4..00000000
--- a/modules/gallery/views/movieplayer-flash.html.php
+++ /dev/null
@@ -1,50 +0,0 @@
-<?php defined("SYSPATH") or die("No direct script access.") ?>
-<script type="text/javascript" src="<?= url::file("lib/flowplayer-flash/flowplayer.js") ?>"></script>
-<?= html::anchor($url, "", $attrs) ?>
-<script type="text/javascript">
-  var id = "<?= $attrs["id"] ?>";
-  var max_size = <?= $max_size ?>;
-  // set the size of the movie html anchor, taking into account max_size and height of control bar
-  function set_movie_size(width, height) {
-    if((width > max_size) || (height > max_size)) {
-      if (width > height) {
-        height = Math.ceil(height * max_size / width);
-        width = max_size;
-      } else {
-        width = Math.ceil(width * max_size / height);
-        height = max_size;
-      }
-    }
-    height += flowplayer(id).getConfig().plugins.controls.height;
-    $("#" + id).css({width: width, height: height});
-  };
-  // setup flowplayer
-  flowplayer(id,
-    $.extend(true, {
-      "src": "<?= url::abs_file("lib/flowplayer-flash/flowplayer.swf") ?>",
-      "wmode": "transparent",
-      "provider": "pseudostreaming"
-    }, <?= json_encode($fp_flash_params) ?>),
-    $.extend(true, {
-      "plugins": {
-        "pseudostreaming": {
-          "url": "<?= url::abs_file("lib/flowplayer-flash/flowplayer.pseudostreaming-byterange.swf") ?>"
-        },
-        "controls": {
-          "autoHide": "always",
-          "hideDelay": 2000,
-          "height": 24
-        }
-      },
-      "clip": {
-        "scaling": "fit",
-        "onMetaData": function(clip) {
-          // set movie size a second time using actual size from metadata
-          set_movie_size(parseInt(clip.metaData.width), parseInt(clip.metaData.height));
-        }
-      }
-    }, <?= json_encode($fp_flash_config) ?>)
-  ).ipad();
-  // set movie size using width and height passed from movie_img function
-  $("document").ready(set_movie_size(<?= $width ?>, <?= $height ?>));
-</script>
diff --git a/modules/gallery/views/movieplayer.html.php b/modules/gallery/views/movieplayer.html.php
new file mode 100644
index 00000000..f78cc91a
--- /dev/null
+++ b/modules/gallery/views/movieplayer.html.php
@@ -0,0 +1,17 @@
+<?php defined("SYSPATH") or die("No direct script access.") ?>
+<div <?= html::attributes($div_attrs) ?>>
+  <video <?= html::attributes($video_attrs) ?>>
+    <source <?= html::attributes($source_attrs) ?>>
+  </video>
+</div>
+<script type="text/javascript">
+  $("#<?= $div_attrs["id"] ?> video").mediaelementplayer(
+    $.extend(true, {
+      defaultVideoWidth: <?= $width ?>,
+      defaultVideoHeight: <?= $height ?>,
+      startVolume: 1.0,
+      features: ["playpause", "progress", "current", "duration", "volume", "fullscreen"],
+      pluginPath: "<?= url::abs_file("lib/mediaelementjs/") ?>"
+    }, <?= json_encode($player_options) ?>)
+  );
+</script>
diff --git a/themes/wind/css/screen.css b/themes/wind/css/screen.css
index cbeaed34..fa1704b0 100644
--- a/themes/wind/css/screen.css
+++ b/themes/wind/css/screen.css
@@ -531,7 +531,7 @@ td {
 }
 
 #g-item img.g-resize,
-#g-item a.g-movie {
+#g-item .g-movie {
   display: block;
   margin: 0 auto;
 }
-- 
cgit v1.2.3


From de3f9edb88aaf0486485838de1b115f2d9ac87c9 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Sat, 9 Mar 2013 15:59:23 +0100
Subject: Follow-on to #1935 - Ensure ffmpeg is executable, remove possible
 doubled "/". - movie::find_ffmpeg - made it use is_executable instead of just
 file_exists. - system::find_binary - removed possible doubled "/" in paths.

---
 modules/gallery/helpers/movie.php  | 3 ++-
 modules/gallery/helpers/system.php | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index aff2acc1..2f190881 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -138,7 +138,8 @@ class movie_Core {
    * Return the path to the ffmpeg binary if one exists and is executable, or null.
    */
   static function find_ffmpeg() {
-    if (!($ffmpeg_path = module::get_var("gallery", "ffmpeg_path")) || !file_exists($ffmpeg_path)) {
+    if (!($ffmpeg_path = module::get_var("gallery", "ffmpeg_path")) ||
+        !@is_executable($ffmpeg_path)) {
       $ffmpeg_path = system::find_binary(
         "ffmpeg", module::get_var("gallery", "graphics_toolkit_path"));
       module::set_var("gallery", "ffmpeg_path", $ffmpeg_path);
diff --git a/modules/gallery/helpers/system.php b/modules/gallery/helpers/system.php
index 7d56466e..f0879d6a 100644
--- a/modules/gallery/helpers/system.php
+++ b/modules/gallery/helpers/system.php
@@ -47,6 +47,7 @@ class system_Core {
       explode(":", module::get_var("gallery", "extra_binary_paths")));
 
     foreach ($paths as $path) {
+      $path = rtrim($path, "/");
       $candidate = "$path/$binary";
       // @suppress errors below to avoid open_basedir issues
       if (@file_exists($candidate)) {
-- 
cgit v1.2.3


From ed20798b99c0c6ab90e4d141ff74d7c2ca606ae7 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Tue, 12 Mar 2013 12:14:34 +0100
Subject: #2057 - Revise item name and slug validation - backslashes, refactor,
 error messages. - disallowed backslashes in item validation. - refactored the
 validation logic in the item model a bit. - added no_backslash error messages
 in edit album/photo/movie forms. - fixed error messages in add album forum
 (some missing, some text different from edit) - added unit tests - updated to
 v58 to correct any existing backslashes in item names

---
 installer/install.sql                         |  2 +-
 modules/gallery/helpers/album.php             |  9 ++++--
 modules/gallery/helpers/gallery_installer.php | 20 ++++++++++++
 modules/gallery/helpers/movie.php             |  1 +
 modules/gallery/helpers/photo.php             |  1 +
 modules/gallery/models/item.php               | 44 +++++++++++++++++---------
 modules/gallery/module.info                   |  2 +-
 modules/gallery/tests/Item_Model_Test.php     | 45 ++++++++++++++++++++++++++-
 8 files changed, 104 insertions(+), 20 deletions(-)

(limited to 'modules/gallery/helpers')

diff --git a/installer/install.sql b/installer/install.sql
index f4938f6f..3f63cf7c 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',57,1);
+INSERT INTO {modules} VALUES (1,1,'gallery',58,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/gallery/helpers/album.php b/modules/gallery/helpers/album.php
index 23aed8ac..fe6b03fc 100644
--- a/modules/gallery/helpers/album.php
+++ b/modules/gallery/helpers/album.php
@@ -34,11 +34,15 @@ class album_Core {
       ->error_messages("length", t("Your title is too long"));
     $group->textarea("description")->label(t("Description"));
     $group->input("name")->label(t("Directory name"))
-      ->error_messages("no_slashes", t("The directory name can't contain the \"/\" character"))
+      ->error_messages("no_slashes", t("The directory name can't contain a \"/\""))
+      ->error_messages("no_backslashes", t("The directory name can't contain a \"\\\""))
+      ->error_messages("no_trailing_period", t("The directory name can't end in \".\""))
       ->error_messages("required", t("You must provide a directory name"))
       ->error_messages("length", t("Your directory name is too long"))
       ->error_messages("conflict", t("There is already a movie, photo or album with this name"));
     $group->input("slug")->label(t("Internet Address"))
+      ->error_messages(
+        "conflict", t("There is already a movie, photo or album with this internet address"))
       ->error_messages(
         "reserved", t("This address is reserved and can't be used."))
       ->error_messages(
@@ -64,13 +68,14 @@ class album_Core {
     $group = $form->group("edit_item")->label(t("Edit Album"));
 
     $group->input("title")->label(t("Title"))->value($parent->title)
-        ->error_messages("required", t("You must provide a title"))
+      ->error_messages("required", t("You must provide a title"))
       ->error_messages("length", t("Your title is too long"));
     $group->textarea("description")->label(t("Description"))->value($parent->description);
     if ($parent->id != 1) {
       $group->input("name")->label(t("Directory Name"))->value($parent->name)
         ->error_messages("conflict", t("There is already a movie, photo or album with this name"))
         ->error_messages("no_slashes", t("The directory name can't contain a \"/\""))
+        ->error_messages("no_backslashes", t("The directory name can't contain a \"\\\""))
         ->error_messages("no_trailing_period", t("The directory name can't end in \".\""))
         ->error_messages("required", t("You must provide a directory name"))
         ->error_messages("length", t("Your directory name is too long"));
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index d49be83f..f1604150 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -809,6 +809,26 @@ class gallery_installer {
         ->execute();
       module::set_version("gallery", $version = 57);
     }
+
+    if ($version == 57) {
+      // In v58 we changed the Item_Model validation code to disallow files or directories with
+      // backslashes in them, and we need to fix any existing items that have them.  This is
+      // pretty unlikely, as having backslashes would have probably already caused other issues for
+      // users, but we should check anyway.  This might be slow, but if it times out it can just
+      // pick up where it left off.
+      foreach (db::build()
+               ->from("items")
+               ->select("id")
+               ->where(db::expr("`name` REGEXP '\\\\\\\\'"), "=", 1)  // one \, 3x escaped
+               ->order_by("id", "asc")
+               ->execute() as $row) {
+        set_time_limit(30);
+        $item = ORM::factory("item", $row->id);
+        $item->name = str_replace("\\", "_", $item->name);
+        $item->save();
+      }
+      module::set_version("gallery", $version = 58);
+    }
   }
 
   static function uninstall() {
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 2f190881..4613df61 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -38,6 +38,7 @@ class movie_Core {
       ->error_messages(
         "conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The movie name can't contain a \"/\""))
+      ->error_messages("no_backslashes", t("The movie name can't contain a \"\\\""))
       ->error_messages("no_trailing_period", t("The movie name can't end in \".\""))
       ->error_messages("illegal_data_file_extension", t("You cannot change the movie file extension"))
       ->error_messages("required", t("You must provide a movie file name"))
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 004cc7c4..ecf81e66 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -35,6 +35,7 @@ class photo_Core {
     $group->input("name")->label(t("Filename"))->value($photo->name)
       ->error_messages("conflict", t("There is already a movie, photo or album with this name"))
       ->error_messages("no_slashes", t("The photo name can't contain a \"/\""))
+      ->error_messages("no_backslashes", t("The photo name can't contain a \"\\\""))
       ->error_messages("no_trailing_period", t("The photo name can't end in \".\""))
       ->error_messages("illegal_data_file_extension", t("You cannot change the photo file extension"))
       ->error_messages("required", t("You must provide a photo file name"))
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 1e16d307..b708c503 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -889,12 +889,17 @@ class Item_Model_Core extends ORM_MPTT {
   }
 
   /**
-   * Validate that the desired slug does not conflict.
+   * Validate the item slug.  It can return the following error messages:
+   * - not_url_safe: has illegal characters
+   * - conflict: has conflicting slug
+   * - reserved (items in root only): has same slug as a controller
    */
   public function valid_slug(Validation $v, $field) {
     if (preg_match("/[^A-Za-z0-9-_]/", $this->slug)) {
       $v->add_error("slug", "not_url_safe");
-    } else if (db::build()
+    }
+
+    if (db::build()
         ->from("items")
         ->where("parent_id", "=", $this->parent_id)
         ->where("id", "<>", $this->id)
@@ -902,11 +907,20 @@ class Item_Model_Core extends ORM_MPTT {
         ->count_records()) {
       $v->add_error("slug", "conflict");
     }
+
+    if ($this->parent_id == 1 && Kohana::auto_load("{$this->slug}_Controller")) {
+      $v->add_error("slug", "reserved");
+      return;
+    }
   }
 
   /**
-   * Validate the item name.  It can't conflict with other names, can't contain slashes or
-   * trailing periods.
+   * Validate the item name.  It can return the following error messages:
+   * - no_slashes: contains slashes
+   * - no_backslashes: contains backslashes
+   * - no_trailing_period: has a trailing period
+   * - illegal_data_file_extension (non-albums only): has double, no, or illegal extension
+   * - conflict: has conflicting name
    */
   public function valid_name(Validation $v, $field) {
     if (strpos($this->name, "/") !== false) {
@@ -914,18 +928,23 @@ class Item_Model_Core extends ORM_MPTT {
       return;
     }
 
-    if (rtrim($this->name, ".") !== $this->name) {
-      $v->add_error("name", "no_trailing_period");
+    if (strpos($this->name, "\\") !== false) {
+      $v->add_error("name", "no_backslashes");
       return;
     }
 
-    // Do not accept files with double extensions, they can cause problems on some
-    // versions of Apache.
-    if (!$this->is_album() && substr_count($this->name, ".") > 1) {
-      $v->add_error("name", "illegal_data_file_extension");
+    if (rtrim($this->name, ".") !== $this->name) {
+      $v->add_error("name", "no_trailing_period");
+      return;
     }
 
     if ($this->is_movie() || $this->is_photo()) {
+      if (substr_count($this->name, ".") > 1) {
+        // Do not accept files with double extensions, as they can
+        // cause problems on some versions of Apache.
+        $v->add_error("name", "illegal_data_file_extension");
+      }
+
       $ext = pathinfo($this->name, PATHINFO_EXTENSION);
 
       if (!$this->loaded() && !$ext) {
@@ -967,11 +986,6 @@ class Item_Model_Core extends ORM_MPTT {
         return;
       }
     }
-
-    if ($this->parent_id == 1 && Kohana::auto_load("{$this->slug}_Controller")) {
-      $v->add_error("slug", "reserved");
-      return;
-    }
   }
 
   /**
diff --git a/modules/gallery/module.info b/modules/gallery/module.info
index 7f49b72e..49023e45 100644
--- a/modules/gallery/module.info
+++ b/modules/gallery/module.info
@@ -1,6 +1,6 @@
 name = "Gallery 3"
 description = "Gallery core application"
-version = 57
+version = 58
 author_name = "Gallery Team"
 author_url = "http://codex.galleryproject.org/Gallery:Team"
 info_url = "http://codex.galleryproject.org/Gallery3:Modules:gallery"
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 83c9f79d..e3a4a6b7 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -124,13 +124,56 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal($fullsize_file, file_get_contents($photo->file_path()));
   }
 
-  public function item_rename_wont_accept_slash_test() {
+  public function photo_rename_wont_accept_slash_test() {
     $item = test::random_photo();
     $item->name = "/no_slashes/allowed/";
     $item->save();
     $this->assert_equal("no_slashes_allowed.jpg", $item->name);
   }
 
+  public function photo_rename_wont_accept_backslash_test() {
+    $item = test::random_photo();
+    $item->name = "\\no_backslashes\\allowed\\";
+    $item->save();
+    $this->assert_equal("no_backslashes_allowed.jpg", $item->name);
+  }
+
+  public function album_rename_wont_accept_slash_test() {
+    try {
+      $item = test::random_album();
+      $item->name = "/no_album_slashes/allowed/";
+      $item->save();
+    } catch (ORM_Validation_Exception $e) {
+      $this->assert_same(array("name" => "no_slashes"), $e->validation->errors());
+      return;  // pass
+    }
+    $this->assert_true(false, "Shouldn't get here");
+  }
+
+  public function album_rename_wont_accept_backslash_test() {
+    try {
+      $item = test::random_album();
+      $item->name = "\\no_album_backslashes\\allowed\\";
+      $item->save();
+    } catch (ORM_Validation_Exception $e) {
+      $this->assert_same(array("name" => "no_backslashes"), $e->validation->errors());
+      return;  // pass
+    }
+    $this->assert_true(false, "Shouldn't get here");
+  }
+
+  public function album_rename_wont_accept_trailing_period_test() {
+    try {
+      $item = test::random_album();
+      $item->name = ".no_trailing_period.allowed.";
+      $item->save();
+    } catch (ORM_Validation_Exception $e) {
+      $this->assert_same(array("name" => "no_trailing_period"), $e->validation->errors());
+      return;  // pass
+    }
+    $this->assert_true(false, "Shouldn't get here");
+  }
+
   public function move_album_test() {
     $album2 = test::random_album();
     $album1 = test::random_album($album2);
-- 
cgit v1.2.3


From 8d0e1b4c4d456d9d2d94c29412629374d0b26d35 Mon Sep 17 00:00:00 2001
From: shadlaws <shad@shadlaws.com>
Date: Wed, 13 Mar 2013 10:07:58 +0100
Subject: #2059 - Add album name sanitizing similar to photo/movie filename
 sanitizing. - added legal_file::sanitize_dirname(), analogous to
 sanitize_filename. - revised item model to use new function when adding or
 updating an album. - added some legal_file unit tests. - revised some item
 model unit tests.

---
 modules/gallery/helpers/legal_file.php           |  29 +++++-
 modules/gallery/models/item.php                  |  23 +++--
 modules/gallery/tests/Item_Model_Test.php        | 110 ++++++++++++++++++-----
 modules/gallery/tests/Legal_File_Helper_Test.php |  18 ++++
 4 files changed, 153 insertions(+), 27 deletions(-)

(limited to 'modules/gallery/helpers')

diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index f8547011..9f02fe70 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -298,7 +298,7 @@ class legal_file_Core {
     $filename = str_replace("/", "_", $filename);
     $filename = str_replace("\\", "_", $filename);
 
-    // Remove extra dots from the filename.  This will also remove extraneous underscores.
+    // Remove extra dots from the filename.  Also removes extraneous and leading/trailing underscores.
     $filename = legal_file::smash_extensions($filename);
 
     // It's possible that the filename has no base (e.g. ".jpg") - if so, give it a generic one.
@@ -308,4 +308,31 @@ class legal_file_Core {
 
     return $filename;
   }
+
+  /**
+   * Sanitize a directory name for an album.  This returns a completely legal and valid
+   * directory name.
+   *
+   * @param  string $dirname (with no parent directory)
+   * @return string sanitized dirname
+   */
+  static function sanitize_dirname($dirname) {
+    // It should be a dirname without a parent directory - remove all slashes (and backslashes).
+    $dirname = str_replace("/", "_", $dirname);
+    $dirname = str_replace("\\", "_", $dirname);
+
+    // Remove extraneous and leading/trailing underscores.
+    $dirname = preg_replace("/[_]+/", "_", $dirname);
+    $dirname = trim($dirname, "_");
+
+    // Remove any trailing dots.
+    $dirname = rtrim($dirname, ".");
+
+    // It's possible that the dirname is now empty - if so, give it a generic one.
+    if (empty($dirname)) {
+      $dirname = "album";
+    }
+
+    return $dirname;
+  }
 }
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index b708c503..1d4f35da 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -365,14 +365,20 @@ class Item_Model_Core extends ORM_MPTT {
           $this->weight = item::get_max_weight();
         }
 
-        // Process the data file info.
-        if (isset($this->data_file)) {
-          $this->_process_data_file_info();
-        } else if (!$this->is_album()) {
-          // Unless it's an album, new items must have a data file.
-          $this->data_file_error = true;
+        if ($this->is_album()) {
+          // Sanitize the album name.
+          $this->name = legal_file::sanitize_dirname($this->name);
+        } else {
+          // Process the data file info.  This also sanitizes the item name.
+          if (isset($this->data_file)) {
+            $this->_process_data_file_info();
+          } else {
+            // New photos and movies must have a data file.
+            $this->data_file_error = true;
+          }
         }
 
+
         // Make an url friendly slug from the name, if necessary
         if (empty($this->slug)) {
           $this->slug = item::convert_filename_to_slug(pathinfo($this->name, PATHINFO_FILENAME));
@@ -437,6 +443,11 @@ class Item_Model_Core extends ORM_MPTT {
             pathinfo($original->name, PATHINFO_EXTENSION), $this->type);
         }
 
+        // If an album's name changed, sanitize it.
+        if ($this->is_album() && array_key_exists("name", $this->changed)) {
+          $this->name = legal_file::sanitize_dirname($this->name);
+        }
+
         // If an album's cover has changed (or been removed), delete any existing album cover,
         // reset the thumb metadata, and mark the thumb as dirty.
         if (array_key_exists("album_cover_item_id", $this->changed) && $this->is_album()) {
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index e3a4a6b7..b6849413 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -125,53 +125,123 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   }
 
   public function photo_rename_wont_accept_slash_test() {
-    $item = test::random_photo();
+    $item = test::random_photo_unsaved();
+    $item->name = "/no_slashes/allowed/";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_slashes", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_slashes_allowed.jpg", $item->name);
+    // Should be corrected on update.
     $item->name = "/no_slashes/allowed/";
     $item->save();
     $this->assert_equal("no_slashes_allowed.jpg", $item->name);
   }
 
   public function photo_rename_wont_accept_backslash_test() {
-    $item = test::random_photo();
+    $item = test::random_photo_unsaved();
+    $item->name = "\\no_backslashes\\allowed\\";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_backslashes", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_backslashes_allowed.jpg", $item->name);
+    // Should be corrected on update.
     $item->name = "\\no_backslashes\\allowed\\";
     $item->save();
     $this->assert_equal("no_backslashes_allowed.jpg", $item->name);
   }
 
+  public function photo_rename_wont_accept_trailing_period_test() {
+    $item = test::random_photo_unsaved();
+    $item->name = "no_trailing_period_allowed.";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_trailing_period", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_trailing_period_allowed.jpg", $item->name);
+    // Should be corrected on update.
+    $item->name = "no_trailing_period_allowed.";
+    $item->save();
+    $this->assert_equal("no_trailing_period_allowed.jpg", $item->name);
+  }
+
   public function album_rename_wont_accept_slash_test() {
+    $item = test::random_album_unsaved();
+    $item->name = "/no_album_slashes/allowed/";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = "/no_album_slashes/allowed/";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_slashes"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_slashes", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_album_slashes_allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = "/no_album_slashes/allowed/";
+    $item->save();
+    $this->assert_equal("no_album_slashes_allowed", $item->name);
   }
 
   public function album_rename_wont_accept_backslash_test() {
+    $item = test::random_album_unsaved();
+    $item->name = "\\no_album_backslashes\\allowed\\";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = "\\no_album_backslashes\\allowed\\";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_backslashes"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_backslashes", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_album_backslashes_allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = "\\no_album_backslashes\\allowed\\";
+    $item->save();
+    $this->assert_equal("no_album_backslashes_allowed", $item->name);
   }
 
   public function album_rename_wont_accept_trailing_period_test() {
+    $item = test::random_album_unsaved();
+    $item->name = ".no_trailing_period.allowed.";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = ".no_trailing_period.allowed.";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_trailing_period"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_trailing_period", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal(".no_trailing_period.allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = ".no_trailing_period.allowed.";
+    $item->save();
+    $this->assert_equal(".no_trailing_period.allowed", $item->name);
   }
 
   public function move_album_test() {
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 3f520131..aab41c41 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -194,4 +194,22 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
       }
     }
   }
+
+  public function sanitize_dirname_with_no_rename_test() {
+    $this->assert_equal("foo", legal_file::sanitize_dirname("foo"));
+    $this->assert_equal("foo.bar", legal_file::sanitize_dirname("foo.bar"));
+    $this->assert_equal(".foo.bar...baz", legal_file::sanitize_dirname(".foo.bar...baz"));
+    $this->assert_equal("foo bar  spaces", legal_file::sanitize_dirname("foo bar  spaces"));
+    $this->assert_equal("j'écris@un#nom_bizarre(mais quand_même_ça_passe \$ÇÀ@€",
+      legal_file::sanitize_dirname("j'écris@un#nom_bizarre(mais quand_même_ça_passe \$ÇÀ@€"));
+  }
+
+  public function sanitize_filename_with_corrections_test() {
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("/foo/bar/"));
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("\\foo\\bar\\"));
+    $this->assert_equal(".foo..bar", legal_file::sanitize_dirname(".foo..bar."));
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("_foo__bar_"));
+    $this->assert_equal("album", legal_file::sanitize_dirname("_"));
+    $this->assert_equal("album", legal_file::sanitize_dirname(null));
+  }
 }
\ No newline at end of file
-- 
cgit v1.2.3