Fixes #4059 page handlers all return nothing

8 files changed, 74 insertions, 89 deletions

diff --git a/engine/lib/admin.php b/engine/lib/admin.php
index ae6429baf..c0a5c0995 100644
--- a/engine/lib/admin.php
+++ b/engine/lib/admin.php
@@ -314,7 +314,7 @@ function admin_init() {
 	// automatic adding of widgets for admin
 	elgg_register_event_handler('make_admin', 'user', 'elgg_add_admin_widgets');
 
-	elgg_register_page_handler('admin', 'admin_settings_page_handler');
+	elgg_register_page_handler('admin', 'admin_page_handler');
 	elgg_register_page_handler('admin_plugin_screenshot', 'admin_plugin_screenshot_page_handler');
 	elgg_register_page_handler('admin_plugin_text_file', 'admin_markdown_page_handler');
 }
@@ -430,7 +430,7 @@ function admin_pagesetup() {
  * @return void
  * @access private
  */
-function admin_settings_page_handler($page) {
+function admin_page_handler($page) {
 
 	admin_gatekeeper();
 	elgg_admin_add_plugin_settings_menu();
@@ -485,7 +485,7 @@ function admin_settings_page_handler($page) {
  * admin_plugin_screenshot/<plugin_id>/<size>/<ss_name>.<ext>
  *
  * @param array $pages The pages array
- * @return true
+ * @return void
  * @access private
  */
 function admin_plugin_screenshot_page_handler($pages) {
@@ -524,8 +524,6 @@ function admin_plugin_screenshot_page_handler($pages) {
 			echo file_get_contents($file);
 			break;
 	}
-
-	return true;
 }
 
 /**
@@ -541,6 +539,7 @@ function admin_plugin_screenshot_page_handler($pages) {
  *	* LICENSE.txt
  *
  * @param type $page
+ * @return void
  * @access private
  */
 function admin_markdown_page_handler($pages) {
diff --git a/engine/lib/deprecated-1.8.php b/engine/lib/deprecated-1.8.php
index beba7d2b7..e1866498b 100644
--- a/engine/lib/deprecated-1.8.php
+++ b/engine/lib/deprecated-1.8.php
@@ -4735,3 +4735,40 @@ function remove_from_river_by_id($id) {
 
 	return elgg_delete_river(array('id' => $id));
 }
+
+/**
+ * A default page handler
+ * Tries to locate a suitable file to include. Only works for core pages, not plugins.
+ *
+ * @param array  $page    The page URL elements
+ * @param string $handler The base handler
+ *
+ * @return true|false Depending on success
+ * @deprecated 1.8
+ */
+function default_page_handler($page, $handler) {
+	global $CONFIG;
+
+	elgg_deprecated_notice("default_page_handler is deprecated", "1.8");
+
+	$page = implode('/', $page);
+
+	// protect against including arbitary files
+	$page = str_replace("..", "", $page);
+
+	$callpath = $CONFIG->path . $handler . "/" . $page;
+	if (is_dir($callpath)) {
+		$callpath = sanitise_filepath($callpath);
+		$callpath .= "index.php";
+		if (file_exists($callpath)) {
+			if (include($callpath)) {
+				return TRUE;
+			}
+		}
+	} else if (file_exists($callpath)) {
+		include($callpath);
+		return TRUE;
+	}
+
+	return FALSE;
+}
diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php
index a6f5fbc6f..03774deb4 100644
--- a/engine/lib/elgglib.php
+++ b/engine/lib/elgglib.php
@@ -1745,7 +1745,7 @@ function _elgg_shutdown_hook() {
  * @access private
  */
 function elgg_js_page_handler($page) {
-	return elgg_cacheable_view_page_handler($page, 'js');
+	elgg_cacheable_view_page_handler($page, 'js');
 }
 
 /**
@@ -1777,8 +1777,6 @@ function elgg_ajax_page_handler($page) {
 
 		echo elgg_view($view, $vars);
 	}
-	
-	return true;
 }
 
 /**
@@ -1798,7 +1796,7 @@ function elgg_css_page_handler($page) {
 		$page[0] = 'elgg';
 	}
 	
-	return elgg_cacheable_view_page_handler($page, 'css');
+	elgg_cacheable_view_page_handler($page, 'css');
 }
 
 /**
@@ -1809,7 +1807,7 @@ function elgg_css_page_handler($page) {
  * @param array  $page The page array
  * @param string $type The type: js or css
  *
- * @return mixed
+ * @return void
  * @access private
  */
 function elgg_cacheable_view_page_handler($page, $type) {
@@ -1824,7 +1822,7 @@ function elgg_cacheable_view_page_handler($page, $type) {
 			break;
 
 		default:
-			return false;
+			return;
 			break;
 	}
 
@@ -1851,8 +1849,6 @@ function elgg_cacheable_view_page_handler($page, $type) {
 
 		echo $return;
 	}
-
-	return true;
 }
 
 /**
@@ -1886,6 +1882,8 @@ function elgg_sql_reverse_order_by_clause($order_by) {
  *
  * Used as a callback for ElggBatch.
  *
+ * @todo why aren't these static methods on ElggBatch?
+ *
  * @param object $object The object to enable
  * @return bool
  * @access private
diff --git a/engine/lib/pagehandler.php b/engine/lib/pagehandler.php
index 0d5e5f89b..91e568c1d 100644
--- a/engine/lib/pagehandler.php
+++ b/engine/lib/pagehandler.php
@@ -9,12 +9,10 @@
 /**
  * Turns the current page over to the page handler, allowing registered handlers to take over.
  *
- * If a page handler returns FALSE, the request is handed over to the default_page_handler.
- *
  * @param string $handler The name of the handler type (eg 'blog')
  * @param array  $page    The parameters to the page, as an array (exploded by '/' slashes)
  *
- * @return true|false Depending on whether a registered page handler was found
+ * @return bool
  * @access private
  */
 function page_handler($handler, $page) {
@@ -42,26 +40,12 @@ function page_handler($handler, $page) {
 	$handler = $params['handler'];
 	$page = $params['segments'];
 
-	if (!isset($CONFIG->pagehandler) || empty($handler)) {
-		$result = false;
-	} else if (isset($CONFIG->pagehandler[$handler]) && is_callable($CONFIG->pagehandler[$handler])) {
+	if (isset($CONFIG->pagehandler) && !empty($handler) && isset($CONFIG->pagehandler[$handler])) {
 		$function = $CONFIG->pagehandler[$handler];
-		$result = call_user_func($function, $page, $handler);
-		if ($result !== false) {
-			$result = true;
-		}
-	} else {
-		$result = false;
+		call_user_func($function, $page, $handler);
 	}
 
-	if (!$result) {
-		$result = default_page_handler($page, $handler);
-	}
-	if ($result !== false) {
-		$result = true;
-	}
-
-	return $result;
+	return headers_sent();
 }
 
 /**
@@ -74,14 +58,15 @@ function page_handler($handler, $page) {
  * For example, the URL http://yoururl/blog/username/friends/ would result in the call:
  * blog_page_handler(array('username','friends'), blog);
  *
- * Page handler functions should return true or the default page handler will be called.
- *
  * A request to register a page handler with the same identifier as previously registered
  * handler will replace the previous one.
  *
  * The context is set to the page handler identifier before the registered
  * page handler function is called. For the above example, the context is set to 'blog'.
  *
+ * Requests not handled are forwarded to the front page with a reason of 404.
+ * Plugins can register for the 'forward', '404' plugin hook. @see forward()
+ *
  * @param string $handler  The page type to handle
  * @param string $function Your function name
  *
@@ -119,38 +104,3 @@ function elgg_unregister_page_handler($handler) {
 
 	unset($CONFIG->pagehandler[$handler]);
 }
-
-/**
- * A default page handler
- * Tries to locate a suitable file to include. Only works for core pages, not plugins.
- *
- * @param array  $page    The page URL elements
- * @param string $handler The base handler
- *
- * @return true|false Depending on success
- * @access private
- */
-function default_page_handler($page, $handler) {
-	global $CONFIG;
-
-	$page = implode('/', $page);
-
-	// protect against including arbitary files
-	$page = str_replace("..", "", $page);
-
-	$callpath = $CONFIG->path . $handler . "/" . $page;
-	if (is_dir($callpath)) {
-		$callpath = sanitise_filepath($callpath);
-		$callpath .= "index.php";
-		if (file_exists($callpath)) {
-			if (include($callpath)) {
-				return TRUE;
-			}
-		}
-	} else if (file_exists($callpath)) {
-		include($callpath);
-		return TRUE;
-	}
-
-	return FALSE;
-}
diff --git a/engine/lib/river.php b/engine/lib/river.php
index f430eb224..b2fead824 100644
--- a/engine/lib/river.php
+++ b/engine/lib/river.php
@@ -586,6 +586,7 @@ function update_river_access_by_object($object_guid, $access_id) {
  * Page handler for activiy
  *
  * @param array $page
+ * @return void
  * @access private
  */
 function elgg_river_page_handler($page) {
diff --git a/engine/lib/tags.php b/engine/lib/tags.php
index 6275d653c..b2c9a672f 100644
--- a/engine/lib/tags.php
+++ b/engine/lib/tags.php
@@ -325,21 +325,18 @@ function elgg_get_registered_tag_metadata_names() {
  * @access private
  */
 function elgg_tagcloud_page_handler($page) {
-	switch ($page[0]) {
-		default:
-			$title = elgg_view_title(elgg_echo('tags:site_cloud'));
-			$options = array(
-				'threshold' => 0,
-				'limit' => 100,
-				'tag_name' => 'tags',
-			);
-			$tags = elgg_view_tagcloud($options);
-			$content = $title . $tags;
-			$body = elgg_view_layout('one_sidebar', array('content' => $content));
-
-			echo elgg_view_page(elgg_echo('tags:site_cloud'), $body);
-			break;
-	}
+
+	$title = elgg_view_title(elgg_echo('tags:site_cloud'));
+	$options = array(
+		'threshold' => 0,
+		'limit' => 100,
+		'tag_name' => 'tags',
+	);
+	$tags = elgg_view_tagcloud($options);
+	$content = $title . $tags;
+	$body = elgg_view_layout('one_sidebar', array('content' => $content));
+
+	echo elgg_view_page(elgg_echo('tags:site_cloud'), $body);
 }
 
 /**
diff --git a/engine/lib/user_settings.php b/engine/lib/user_settings.php
index bb5d8d6c4..12084662b 100644
--- a/engine/lib/user_settings.php
+++ b/engine/lib/user_settings.php
@@ -304,7 +304,7 @@ function usersettings_page_handler($page) {
 		$page[0] = 'user';
 	}
 
-	if ($page[1]) {
+	if (isset($page[1])) {
 		$user = get_user_by_username($page[1]);
 		elgg_set_page_owner_guid($user->guid);
 	} else {
@@ -324,12 +324,13 @@ function usersettings_page_handler($page) {
 			$path = $CONFIG->path . "pages/settings/tools.php";
 			break;
 		case 'user':
-		default:
 			$path = $CONFIG->path . "pages/settings/account.php";
 			break;
 	}
 
-	require($path);
+	if (isset($path)) {
+		require $path;
+	}
 }
 
 /**
diff --git a/engine/lib/users.php b/engine/lib/users.php
index 843b897e9..3001a2ac6 100644
--- a/engine/lib/users.php
+++ b/engine/lib/users.php
@@ -1072,7 +1072,7 @@ function friends_page_handler($page_elements) {
 function friends_of_page_handler($page_elements) {
 	elgg_set_context('friends');
 	if (isset($page_elements[0]) && $user = get_user_by_username($page_elements[0])) {
-		set_page_owner($user->getGUID());
+		elgg_set_page_owner_guid($user->getGUID());
 	}
 	if (elgg_get_logged_in_user_guid() == elgg_get_page_owner_guid()) {
 		collections_submenu_items();
@@ -1404,6 +1404,7 @@ function elgg_profile_fields_setup() {
  * /avatar/view/<username>/<size>/<icontime>
  *
  * @param array $page
+ * @return void
  * @access private
  */
 function elgg_avatar_page_handler($page) {
@@ -1426,6 +1427,7 @@ function elgg_avatar_page_handler($page) {
  * Profile page handler
  *
  * @param array $page
+ * @return void
  * @access private
  */
 function elgg_profile_page_handler($page) {