From 323aa0293343fbd98de5319dda102be0231516a7 Mon Sep 17 00:00:00 2001
From: Brion Vibber <brion@pobox.com>
Date: Fri, 23 Feb 2018 09:42:10 +0000
Subject: [PATCH] Adjust tests for JPEG segmentation fix

Added a second test case truncated in a different place, and
added some clarifying comments.

Change-Id: I7b8e5296a203264b7e7e428f82c8c948242a1272
---
 includes/media/JpegMetadataExtractor.php           |   2 ++
 .../{jpeg-xmp-loop.jpg => jpeg-segment-loop1.jpg}  | Bin
 tests/phpunit/data/media/jpeg-segment-loop2.jpg    | Bin 0 -> 24 bytes
 .../includes/media/JpegMetadataExtractorTest.php   |  13 ++++++++++++-
 4 files changed, 14 insertions(+), 1 deletion(-)
 rename tests/phpunit/data/media/{jpeg-xmp-loop.jpg => jpeg-segment-loop1.jpg} (100%)
 create mode 100644 tests/phpunit/data/media/jpeg-segment-loop2.jpg

diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php
index 0bd01cd6c9..3c778f36c1 100644
--- a/includes/media/JpegMetadataExtractor.php
+++ b/includes/media/JpegMetadataExtractor.php
@@ -158,6 +158,8 @@ class JpegMetadataExtractor {
 				if ( $size['int'] < 2 ) {
 					throw new MWException( "invalid marker size in jpeg" );
 				}
+				// Note it's possible to seek beyond end of file if truncated.
+				// fseek doesn't report a failure in this case.
 				fseek( $fh, $size['int'] - 2, SEEK_CUR );
 			}
 		}
diff --git a/tests/phpunit/data/media/jpeg-xmp-loop.jpg b/tests/phpunit/data/media/jpeg-segment-loop1.jpg
similarity index 100%
rename from tests/phpunit/data/media/jpeg-xmp-loop.jpg
rename to tests/phpunit/data/media/jpeg-segment-loop1.jpg
diff --git a/tests/phpunit/data/media/jpeg-segment-loop2.jpg b/tests/phpunit/data/media/jpeg-segment-loop2.jpg
new file mode 100644
index 0000000000000000000000000000000000000000..e3a7505c001e98f6912a1323d61a1361e789f687
GIT binary patch
literal 24
ecmex=<NpH&0WUXCHwH#V28Ibh$nbwABLe_-NC%++

literal 0
HcmV?d00001

diff --git a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
index 97921727f5..c943cef906 100644
--- a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
+++ b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
@@ -110,8 +110,19 @@ class JpegMetadataExtractorTest extends MediaWikiTestCase {
 	}
 
 	public function testInfiniteRead() {
+		// test file truncated right after a segment, which previously
+		// caused an infinite loop looking for the next segment byte.
 		// Should get past infinite loop and throw in wfUnpack()
 		$this->setExpectedException( 'MWException' );
-		$res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-loop.jpg' );
+		$res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-segment-loop1.jpg' );
+	}
+
+	public function testInfiniteRead2() {
+		// test file truncated after a segment's marker and size, which
+		// would cause a seek past end of file. Seek past end of file
+		// doesn't actually fail, but prevents further reading and was
+		// devolving into the previous case (testInfiniteRead).
+		$this->setExpectedException( 'MWException' );
+		$res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-segment-loop2.jpg' );
 	}
 }
-- 
2.20.1