Merge branch 'develop-ascraeus' into develop

* develop-ascraeus:
  [ticket/13031] Slightly change behavior of choose_mime_type and add unit tests
  [ticket/13031] Guess with all mimetype guessers and pick best guess
  [ticket/13031] Only use mimetype guesser guess if it helps us

phpBB/includes/functions_upload.php

@@ -232,7 +232,12 @@ class filespec
 	{
 		if ($this->mimetype_guesser !== null)
 		{
-			$this->mimetype = $this->mimetype_guesser->guess($filename);
+			$mimetype = $this->mimetype_guesser->guess($filename, $this->uploadname);
+
+			if ($mimetype !== 'application/octet-stream')
+			{
+				$this->mimetype = $mimetype;
+			}
 		}
 
 		return $this->mimetype;

phpBB/phpbb/mimetype/guesser.php

@@ -115,17 +115,42 @@ class guesser
 			return false;
 		}
 
+		$mimetype = 'application/octet-stream';
+
 		foreach ($this->guessers as $guesser)
 		{
-			$mimetype = $guesser->guess($file, $file_name);
+			$mimetype_guess = $guesser->guess($file, $file_name);
 
-			// Try to guess something that is not the fallback application/octet-stream
+			$mimetype = $this->choose_mime_type($mimetype, $mimetype_guess);
-			if ($mimetype !== null && $mimetype !== 'application/octet-stream')
-			{
-				return $mimetype;
-			}
 		}
 		// Return any mimetype if we got a result or the fallback value
-		return (!empty($mimetype)) ? $mimetype : 'application/octet-stream';
+		return $mimetype;
+	}
+
+	/**
+	 * Choose the best mime type based on the current mime type and the guess
+	 * If a guesser returns nulls or application/octet-stream, we will keep
+	 * the current guess. Guesses with a slash inside them will be favored over
+	 * already existing ones. However, any guess that will pass the first check
+	 * will always overwrite the default application/octet-stream.
+	 *
+	 * @param	string	$mime_type	The current mime type
+	 * @param	string	$guess		The current mime type guess
+	 *
+	 * @return string The best mime type based on current mime type and guess
+	 */
+	public function choose_mime_type($mime_type, $guess)
+	{
+		if ($guess === null || $guess == 'application/octet-stream')
+		{
+			return $mime_type;
+		}
+
+		if ($mime_type == 'application/octet-stream' || strpos($guess, '/') !== false)
+		{
+			$mime_type = $guess;
+		}
+
+		return $mime_type;
 	}
 }

tests/functional/fileupload_form_test.php

@@ -107,9 +107,9 @@ class phpbb_functional_fileupload_form_test extends phpbb_functional_test_case
 
 		$crawler = $this->upload_file('disallowed.jpg', 'image/jpeg');
 
-		// Hitting the ATTACHED_IMAGE_NOT_IMAGE error means we passed the
+		// Hitting the UNABLE_GET_IMAGE_SIZE error means we passed the
 		// DISALLOWED_CONTENT check
-		$this->assertContains($this->lang('ATTACHED_IMAGE_NOT_IMAGE'), $crawler->text());
+		$this->assertContainsLang('UNABLE_GET_IMAGE_SIZE', $crawler->text());
 	}
 
 	public function test_too_large()

tests/mimetype/guesser_test.php

@@ -206,4 +206,25 @@ class guesser_test extends \phpbb_test_case
 		$this->assertInstanceOf('\phpbb\mimetype\content_guesser', $guessers[0]);
 		$this->assertInstanceOf('\phpbb\mimetype\extension_guesser', $guessers[3]);
 	}
+
+	public function data_choose_mime_type()
+	{
+		return array(
+			array('application/octet-stream', 'application/octet-stream', null),
+			array('application/octet-stream', 'application/octet-stream', 'application/octet-stream'),
+			array('binary', 'application/octet-stream', 'binary'),
+			array('image/jpeg', 'application/octet-stream', 'image/jpeg'),
+			array('image/jpeg', 'binary', 'image/jpeg'),
+			array('image/jpeg', 'image/jpg', 'image/jpeg'),
+			array('image/jpeg', 'image/jpeg', 'binary'),
+		);
+	}
+
+	/**
+	 * @dataProvider data_choose_mime_type
+	 */
+	public function test_choose_mime_type($expected, $mime_type, $guess)
+	{
+		$this->assertSame($expected, $this->guesser->choose_mime_type($mime_type, $guess));
+	}
 }