Merge pull request #6500 from marc1706/ticket/15687

[ticket/15687] Add attachment filename to attachment URL

phpBB/config/default/routing/storage.yml

@@ -4,8 +4,9 @@ phpbb_storage_avatar:
         _controller:  storage.controller.avatar:handle
 
 phpbb_storage_attachment:
-    path:             /attachment/{file}
+    path:             /attachment/{id}/{filename}
     defaults:
-        _controller:  storage.controller.attachment:handle
+        filename: ''
+        _controller:  storage.controller.attachment:handle_attachment
     requirements:
         id:         \d+

phpBB/download/file.php

@@ -47,7 +47,7 @@ $thumbnail = $request->variable('t', false);
 
 $response = new RedirectResponse(
 	$controller_helper->route('phpbb_storage_attachment', array(
-		'file'	=> $attach_id,
+		'id'	=> $attach_id,
 		't'		=> $thumbnail,
 	), false),
 	301

phpBB/includes/acp/acp_attachments.php

@@ -1112,7 +1112,13 @@ class acp_attachments
 						'PHYSICAL_FILENAME'	=> utf8_basename($row['physical_filename']),
 						'ATTACH_ID'			=> $row['attach_id'],
 						'POST_ID'			=> (!empty($post_ids[$row['attach_id']])) ? $post_ids[$row['attach_id']] : '',
-						'U_FILE'			=> $this->controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']])
+						'U_FILE'			=> $this->controller_helper->route(
+							'phpbb_storage_attachment',
+							[
+								'id'		=> (int) $row['attach_id'],
+								'filename'	=> $row['real_filename'],
+							]
+						),
 					]);
 				}
 				$db->sql_freeresult($result);
@@ -1302,7 +1308,13 @@ class acp_attachments
 						'S_IN_MESSAGE'		=> (bool) $row['in_message'],
 
 						'U_VIEW_TOPIC'		=> append_sid("{$phpbb_root_path}viewtopic.$phpEx", "p={$row['post_msg_id']}") . "#p{$row['post_msg_id']}",
-						'U_FILE'			=> $this->controller_helper->route('phpbb_storage_attachment', ['file' => $row['attach_id']])
+						'U_FILE'			=> $this->controller_helper->route(
+							'phpbb_storage_attachment',
+							[
+								'id'		=> (int) $row['attach_id'],
+								'filename'	=> $row['real_filename'],
+							]
+						)
 					));
 				}
 

phpBB/includes/acp/acp_users.php

@@ -2306,7 +2306,13 @@ class acp_users
 
 						'S_IN_MESSAGE'		=> $row['in_message'],
 
-						'U_DOWNLOAD'		=> $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']]),
+						'U_DOWNLOAD'		=> $controller_helper->route(
+							'phpbb_storage_attachment',
+							[
+								'id'		=> (int) $row['attach_id'],
+								'filename'	=> $row['real_filename'],
+							]
+						),
 						'U_VIEW_TOPIC'		=> $view_topic)
 					);
 				}

phpBB/includes/functions_content.php

@@ -1285,14 +1285,26 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a
 				$display_cat = attachment_category::NONE;
 			}
 
-			$download_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id']]);
+			$download_link = $controller_helper->route(
+				'phpbb_storage_attachment',
+				[
+					'id'		=> (int) $attachment['attach_id'],
+					'filename'	=> $attachment['real_filename'],
+				]
+			);
 			$l_downloaded_viewed = 'VIEWED_COUNTS';
 
 			switch ($display_cat)
 			{
 				// Images
 				case attachment_category::IMAGE:
-					$inline_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id']]);
+					$inline_link = $controller_helper->route(
+						'phpbb_storage_attachment',
+						[
+							'id'		=> (int) $attachment['attach_id'],
+							'filename'	=> $attachment['real_filename'],
+						]
+					);
 
 					$block_array += array(
 						'S_IMAGE'		=> true,
@@ -1304,7 +1316,14 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a
 
 				// Images, but display Thumbnail
 				case attachment_category::THUMB:
-					$thumbnail_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id'], 't' => 1]);
+					$thumbnail_link = $controller_helper->route(
+						'phpbb_storage_attachment',
+						[
+							'id'		=> (int) $attachment['attach_id'],
+							'filename'	=> $attachment['real_filename'],
+							't'			=> 1,
+						]
+					);
 
 					$block_array += array(
 						'S_THUMBNAIL'		=> true,

phpBB/includes/functions_posting.php

@@ -868,7 +868,14 @@ function posting_gen_attachment_entry($attachment_data, &$filename_data, $show_a
 				$hidden .= '<input type="hidden" name="attachment_data[' . $count . '][' . $key . ']" value="' . $value . '" />';
 			}
 
-			$download_link = $phpbb_container->get('controller.helper')->route('phpbb_storage_attachment', ['file' => (int) $attach_row['attach_id']]);
+			$download_link = $phpbb_container->get('controller.helper')
+				->route(
+					'phpbb_storage_attachment',
+					[
+						'id'		=> (int) $attach_row['attach_id'],
+						'filename'	=> $attach_row['real_filename'],
+					]
+				);
 
 			$attachrow_template_vars[(int) $attach_row['attach_id']] = array(
 				'FILENAME'			=> utf8_basename($attach_row['real_filename']),

phpBB/includes/message_parser.php

@@ -1716,7 +1716,13 @@ class parse_message extends bbcode_firstpass
 
 						if (isset($this->plupload) && $this->plupload->is_active())
 						{
-							$download_url = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $new_entry['attach_id']]);
+							$download_url = $controller_helper->route(
+								'phpbb_storage_attachment',
+								[
+									'id'		=> (int) $new_entry['attach_id'],
+									'filename'	=> $new_entry['real_filename'],
+								]
+							);
 
 							// Send the client the attachment data to maintain state
 							$json_response->send(array('data' => $this->attachment_data, 'download_url' => $download_url));

phpBB/includes/ucp/ucp_attachments.php

@@ -185,7 +185,13 @@ class ucp_attachments
 					'S_IN_MESSAGE'		=> $row['in_message'],
 					'S_LOCKED'			=> !$row['in_message'] && !$auth->acl_get('m_edit', $row['forum_id']) && ($row['forum_status'] == ITEM_LOCKED || $row['topic_status'] == ITEM_LOCKED || $row['post_edit_locked']),
 
-					'U_VIEW_ATTACHMENT'	=> $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']]),
+					'U_VIEW_ATTACHMENT'	=> $controller_helper->route(
+						'phpbb_storage_attachment',
+						[
+							'id'		=> (int) $row['attach_id'],
+							'filename'	=> $row['real_filename'],
+						]
+					),
 					'U_VIEW_TOPIC'		=> $view_topic)
 				);
 

phpBB/phpbb/storage/controller/attachment.php

@@ -86,11 +86,14 @@ class attachment extends controller
 	}
 
 	/**
-	 * {@inheritdoc}
+	 * Handle attachments
+	 *
+	 * @param int $id File ID
+	 * @param string $filename Filename
 	 */
-	public function handle(string $file): Response
+	public function handle_attachment(int $id, string $filename): Response
 	{
-		$attach_id = (int) $file;
+		$attach_id = $id;
 		$thumbnail = $this->request->variable('t', false);
 
 		$this->language->add_lang('viewtopic');
@@ -109,7 +112,8 @@ class attachment extends controller
 				is_orphan, physical_filename, real_filename, extension, mimetype,
 				filesize, filetime
 			FROM ' . ATTACHMENTS_TABLE . "
-			WHERE attach_id = $attach_id";
+			WHERE attach_id = $attach_id" .
+				(($filename) ? " AND real_filename = '" . $this->db->sql_escape($filename) . "'" : '');
 		$result = $this->db->sql_query($sql);
 		$attachment = $this->db->sql_fetchrow($result);
 		$this->db->sql_freeresult($result);

tests/functional/acp_attachments_test.php

@@ -86,7 +86,7 @@ class phpbb_functional_acp_attachments_test extends phpbb_functional_test_case
 		// Get attach id, the link looks similar to ./download/attachment/3
 		$attach_link = $crawler->filter('span.file-name > a')->attr('href');
 		$matches = [];
-		preg_match('/\/([0-9]+)$/', $attach_link, $matches);
+		preg_match('/\/([0-9]+)\/valid\.jpg$/', $attach_link, $matches);
 		$attach_id = (int) $matches[1];
 
 		// Set file time older than 3 hours to consider it orphan