Added validation to the edit functionality, since we can't trust any input

3 files changed, 62 insertions, 42 deletions

diff --git a/modules/gallery/helpers/gallery_rest.php b/modules/gallery/helpers/gallery_rest.php
index 30a37ad1..e31c4252 100644
--- a/modules/gallery/helpers/gallery_rest.php
+++ b/modules/gallery/helpers/gallery_rest.php
@@ -48,7 +48,7 @@ class gallery_rest_Core {
                            "size" => array("height" => $item->height,
                                            "width" => $item->width),
                            "description" => $item->description,
-                           "internet_address" => $item->slug);
+                           "slug" => $item->slug);
 
     $children = self::_get_children($item, $request);
     if (!empty($children) || $item->is_album()) {
@@ -58,10 +58,6 @@ class gallery_rest_Core {
   }
 
   static function put($request) {
-    if (empty($request->path)) {
-      return rest::invalid_request();
-    }
-
     $item = ORM::factory("item")
       ->where("relative_url_cache", $request->path)
       ->viewable()
@@ -75,37 +71,18 @@ class gallery_rest_Core {
       return rest::not_found("Resource: {$request->path} permission denied.");
     }
 
-    // Normalize the request
-    $new_values = array();
-    $fields = array("title", "description", "name", "slug");
-    if ($item->is_album()) {
-      $fields = array_merge($fields, array("sort_column", "sort_order"));
-    }
-    foreach ($fields as $field) {
-      $new_values[$field] = !empty($request->$field) ? $request->$field : $item->$field;
-    }
-    if ($item->id == 1) {
-      unset($new_values["name"]);
-    }
-    if ($item->id != 1 &&
-        ($new_values["name"] != $item->name || $new_values["slug"] != $item->slug)) {
-      // Make sure that there's not a conflict
-      $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]);
-      if (!empty($errors["name_conflict"])) {
-        return rest::fail(t("Renaming %path failed: new name exists",
-                            array("path" => $request->path)));
-      }
-      if (!empty($errors["slug_conflict"])) {
-        return rest::fail(t("Renaming %path failed: new internet address exists",
-                            array("path" => $request->path)));
-      }
-    }
-
-    item::update($item, $new_values);
+    // Validate the request data
+    $new_values = gallery_rest::_validate($item, $request);
+    $errors = $new_values->errors();
+    if (empty($errors)) {
+      item::update($item, $new_values->as_array());
 
-    log::success("content", "Updated $item->type", "<a href=\"{$item->type}s/$item->id\">view</a>");
+      log::success("content", "Updated $item->type", "<a href=\"{$item->type}s/$item->id\">view</a>");
 
-    return rest::success();
+      return rest::success();
+    } else {
+      return rest::validation_error($errors);
+    }
   }
 
   static function post($request) {
@@ -129,6 +106,8 @@ class gallery_rest_Core {
       return rest::not_found("Resource: {$request->path} permission denied.");
     }
 
+    // @TODO validate input values (assume nothing about the quality of input)
+
     if (empty($_FILES["image"])) {
       $new_item = album::create(
         $parent,
@@ -189,6 +168,7 @@ class gallery_rest_Core {
       return rest::invalid_request("Attempt to delete the root album");
     }
 
+    $parent = $item->parent();
     $item->delete();
 
     if ($item->is_album()) {
@@ -198,7 +178,7 @@ class gallery_rest_Core {
     }
     log::success("content", $msg);
 
-    return rest::success();
+    return rest::success(array("resource" => array("parent_path" => $parent->relative_url())));
   }
 
   private static function _get_children($item, $request) {
@@ -219,4 +199,34 @@ class gallery_rest_Core {
 
     return $children;
   }
+
+  private static function _validate($item, $request) {
+    $new_values = array();
+    $fields = array("title", "description", "name", "slug");
+    if ($item->id == 1) {
+      unset($request["name"]);
+      unset($request["slug"]);
+    }
+    foreach ($fields as $field) {
+      $new_values[$field] = isset($request->$field) ? $request->$field : $item->$field;
+    }
+
+    $new_values = new Validation($new_values);
+    foreach ($item->rules as $field => $rules) {
+      foreach (explode("|", $rules) as $rule) {
+        $new_values->add_rules($field, $rule);
+      }
+    }
+
+    if (($valid = $new_values->validate()) && $item->id != 1) {
+      $errors = item::check_for_conflicts($item, $new_values["name"], $new_values["slug"]);
+      if ($valid = empty($errors)) {
+        !empty($errors["name_conflict"]) OR $new_values->add_error("name", "Duplicate Name");
+        !empty($errors["slug_conflict"]) OR
+          $new_values->add_error("name", "Duplicate Internet Address");
+      }
+    }
+
+    return $new_values;
+  }
 }
diff --git a/modules/rest/controllers/rest.php b/modules/rest/controllers/rest.php
index d1404b29..7a5ab46a 100644
--- a/modules/rest/controllers/rest.php
+++ b/modules/rest/controllers/rest.php
@@ -67,7 +67,7 @@ class Rest_Controller extends Controller {
   }
 
   private function _normalize_request($args=array()) {
-   $method = strtolower($this->input->server("REQUEST_METHOD"));
+    $method = strtolower($this->input->server("REQUEST_METHOD"));
     $request = new stdClass();
     foreach (array_keys($this->input->get()) as $key) {
       $request->$key = $this->input->get($key);
@@ -78,8 +78,7 @@ class Rest_Controller extends Controller {
       }
     }
 
-    $override_method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", null));
-    $request->method = empty($override_method) ? $method : $override_method;
+    $request->method = strtolower($this->input->server("HTTP_X_GALLERY_REQUEST_METHOD", $method));
     $request->access_token = $this->input->server("HTTP_X_GALLERY_REQUEST_KEY");
     $request->path = implode("/", $args);
 
diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php
index 2c653f21..ad6ca7c7 100644
--- a/modules/rest/helpers/rest.php
+++ b/modules/rest/helpers/rest.php
@@ -62,14 +62,25 @@ class rest_Core {
   /**
    * Success
    */
-  static function success($response_data=null, $message=null) {
+  static function success($response_data=array(), $message=null) {
     $response = array("status" => "OK");
     if (!empty($message)) {
       $response["message"] = (string)$message;
     }
-    if ($response_data) {
-      $response = array_merge($response, $response_data);
-    }
+    $response = array_merge($response, $response_data);
+
+    // We don't need to save the session for this request
+    Session::abort_save();
+    return json_encode($response);
+  }
+
+  /**
+   * Validation Error
+   */
+  static function validation_error($error_data) {
+    $response = array("status" => "VALIDATE_ERROR");
+    $response = array_merge($response, array("fields" => $error_data));
+
     // We don't need to save the session for this request
     Session::abort_save();
     return json_encode($response);