Use a more robust json_decode

Signed-off-by: Matt Friedman <[email protected]>
phpbb-extensions · Jun 26, 2024 · eada9dc · eada9dc
1 parent 69b874e
commit eada9dc
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 3 deletions.
diff --git a/controller/acp_controller.php b/controller/acp_controller.php
@@ -142,7 +142,7 @@ protected function list_announcements()
 				'EXPIRY_DATE'  => $row['announcement_expiry'],
 				'S_EXPIRED'    => $expired,
 				'S_ENABLED'    => $enabled,
-				'LOCATIONS'    => json_decode($row['announcement_locations'], true),
+				'LOCATIONS'    => $this->manager->decode_json($row['announcement_locations']),
 				'U_EDIT'       => $this->u_action . '&amp;action=add&amp;id=' . $row['announcement_id'],
 				'U_DELETE'     => $this->u_action . '&amp;action=delete&amp;id=' . $row['announcement_id'],
 				'U_MOVE_UP'    => $this->u_action . '&amp;action=move&amp;id=' . $row['announcement_id'] . '&amp;dir=up&amp;hash=' . generate_link_hash('up' . $row['announcement_id']),
@@ -229,6 +229,7 @@ protected function action_add()
 				$data['announcement_expiry'] = 0;
 			}
 
+			// Locations array should be json encoded for storage in the DB
 			$data['announcement_locations'] = json_encode($data['announcement_locations']);
 
 			// Prepare announcement text for storage
@@ -455,7 +456,7 @@ protected function log_change($msg, $params)
 	 */
 	protected function get_location_options($locations)
 	{
-		$selected = !empty($locations) ? json_decode($locations, true) : [];
+		$selected = !empty($locations) ? $this->manager->decode_json($locations) : [];
 
 		$forum_list = make_forum_select($selected, false, false, false, false, false, true);
 

diff --git a/event/listener.php b/event/listener.php
@@ -108,7 +108,7 @@ public function display_board_announcements()
 		$board_announcements_data = $this->manager->get_visible_announcements($this->user->data['user_id']);
 
 		$board_announcements_data = array_filter($board_announcements_data, function ($data) {
-			$locations = json_decode($data['announcement_locations'], true);
+			$locations = $this->manager->decode_json($data['announcement_locations']);
 			$is_index = $this->user->page['page_name'] === "index.$this->php_ext";
 			$current_page = $is_index ? ext::INDEX_ONLY : $this->request->variable('f', 0);
 

diff --git a/manager/manager.php b/manager/manager.php
@@ -189,6 +189,18 @@ public function close_announcement($id, $user_id)
 		return (bool) $this->nestedset->insert_tracked_item($id, ['user_id' => (int) $user_id]);
 	}
 
+	/**
+	 * Decode JSON data
+	 *
+	 * @param string $data
+	 * @return mixed decoded json data or original data if not valid json
+	 */
+	public function decode_json($data)
+	{
+		$result = json_decode($data, true);
+		return json_last_error() === JSON_ERROR_NONE ? $result : $data;
+	}
+
 	/**
 	 * Filter members only announcements
 	 *

diff --git a/tests/controller/acp_controller_test.php b/tests/controller/acp_controller_test.php
@@ -412,6 +412,10 @@ public function test_action_add_submit($id, $form, $preview, $submit, $valid_for
 			->withConsecutive(['submit'], ['preview'])
 			->willReturnOnConsecutiveCalls($submit, $preview);
 
+		$this->manager->expects(self::once())
+			->method('decode_json')
+			->willReturn([]);
+
 		$this->manager->expects(self::once())
 			->method('announcement_columns')
 			->willReturn([]);