From 9ce26a564d066a33ba7ae2a6502e3d57e7e4d48b Mon Sep 17 00:00:00 2001
From: Derk-Jan Hartman <hartman.wiki@gmail.com>
Date: Thu, 12 Sep 2019 00:12:22 +0200
Subject: [PATCH] Fix XMP parser errors due to trailing nullchar

JPEG files can have trailing \0 chars at the end of the XMP value. Use
trim() to remove these from the string value.

Bug: T118799
Change-Id: Id4ab223ef432e5d2c0dd3b4e332320db02422700
---
 includes/media/JpegMetadataExtractor.php         |   9 ++++++---
 tests/phpunit/data/media/jpeg-xmp-nullchar.jpg   | Bin 0 -> 3309 bytes
 .../includes/media/JpegMetadataExtractorTest.php |   8 +++++++-
 3 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 tests/phpunit/data/media/jpeg-xmp-nullchar.jpg

diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php
index 8a26f606f9..dffb1f988b 100644
--- a/includes/media/JpegMetadataExtractor.php
+++ b/includes/media/JpegMetadataExtractor.php
@@ -122,14 +122,17 @@ class JpegMetadataExtractor {
 				$temp = self::jpegExtractMarker( $fh );
 				// check what type of app segment this is.
 				if ( substr( $temp, 0, 29 ) === "http://ns.adobe.com/xap/1.0/\x00" && $showXMP ) {
-					$segments["XMP"] = substr( $temp, 29 );
+					// use trim to remove trailing \0 chars
+					$segments["XMP"] = trim( substr( $temp, 29 ) );
 				} elseif ( substr( $temp, 0, 35 ) === "http://ns.adobe.com/xmp/extension/\x00" && $showXMP ) {
-					$segments["XMP_ext"][] = substr( $temp, 35 );
+					// use trim to remove trailing \0 chars
+					$segments["XMP_ext"][] = trim( substr( $temp, 35 ) );
 				} elseif ( substr( $temp, 0, 29 ) === "XMP\x00://ns.adobe.com/xap/1.0/\x00" && $showXMP ) {
 					// Some images (especially flickr images) seem to have this.
 					// I really have no idea what the deal is with them, but
 					// whatever...
-					$segments["XMP"] = substr( $temp, 29 );
+					// use trim to remove trailing \0 chars
+					$segments["XMP"] = trim( substr( $temp, 29 ) );
 					wfDebug( __METHOD__ . ' Found XMP section with wrong app identifier '
 						. "Using anyways.\n" );
 				} elseif ( substr( $temp, 0, 6 ) === "Exif\0\0" ) {
diff --git a/tests/phpunit/data/media/jpeg-xmp-nullchar.jpg b/tests/phpunit/data/media/jpeg-xmp-nullchar.jpg
new file mode 100644
index 0000000000000000000000000000000000000000..76ae2b486cbc237b448fc0d742ed80fd7874b62f
GIT binary patch
literal 3309
zcmex=<NpH&0WUXCHwH#VMg|WcWcdG<!6YCfza+mnBfmhwSkHjL!pYN@g@pkGBxIN*
z8CXhEi%UQZ=Cu5L872k>CI*)O54lA%N=gc>^!4+K^%7I^lT!7P^K<np5)1SV^$heG
zZ0suv5|gu2OB9k)(=+pI)!*;luda}pVyhl*>TBRz;GCL~=}}db8eHWUl3bOYY?-2N
zZ^va*VO5b^kegbPs8ErclUHn2VXF={+zQAB3G1sXfR&VF+p2r!CZ?xaS-DnZriJ9^
z=O~!#85^hr%_vGqvkG!?gPH*3*{VaGT3%kRS8lAAUzDzIXlZGwZ(yWvWTXpJp<7&;
zSCUwvn^&w1G=WP2Y=%o}adJ^+K}lwQo&w0s#H9Sv5?gh3E(NI3Damk?3rdS}z~-hT
z>!;?V=BDPA6axdoP#<WVf`Uy-vQ=gZP%JYoGquPrs~|O97wAvjg5t~~y{v+C8+}v-
zKnLi9Y(#Pa&|hF7pvOQ=csPNKr#%?8grqPY4X)APA}It&QaqZvMuUr_5FknMXzCgb
zE|NlkB*nuhbs<`~sd>O=Z8@<0%JBa-gENDgl&qqxgqotfqPB*bu4TNPrHP4UX>f#V
zd{^bf{*KDl<{7JR@0zjb{F3I@eg6-hfAsR}kFQg<|NsC0-T&JkzP<q&!l<FGZE0j#
zU}snGW^U`;Hzb4q2N(o77`PZTm>HEAm;@P_1sVSzVUS>8U}j_lH|=2rBMU1tP)1pR
zfr*KUkp&^m$iytjAf(75Z0M*IIFZ!|s7M4&FVGB-s359|AkkY4Jj{$hdx1vSGhF)D
z^#0P6T{ka8eElS;_SdC&XM)S779Nr2^}j#XRLSpL7GkiBp?hDUNyhh0+Bq6Gc1sAJ
zy*qhv?>sj*Aq~^@_6Cf0mEVuarhIbX>wA;jpM2=^^ON?aueIMU&)>f2W$4<%pPdU@
Wo7y@#0-dfD*ln;%Y?F}xe-i)%`)#rS

literal 0
HcmV?d00001

diff --git a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
index 6c56510170..4a0a6d0cad 100644
--- a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
+++ b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php
@@ -1,7 +1,7 @@
 <?php
 /**
  * @todo Could use a test of extended XMP segments. Hard to find programs that
- * create example files, and creating my own in vim propbably wouldn't
+ * create example files, and creating my own in vim probably wouldn't
  * serve as a very good "test". (Adobe photoshop probably creates such files
  * but it costs money). The implementation of it currently in MediaWiki is based
  * solely on reading the standard, without any real world test files.
@@ -76,6 +76,12 @@ class JpegMetadataExtractorTest extends MediaWikiIntegrationTestCase {
 		$this->assertEquals( $expected, bin2hex( $res['PSIR'][0] ) );
 	}
 
+	public function testXMPExtractionNullChar() {
+		$res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-nullchar.jpg' );
+		$expected = file_get_contents( $this->filePath . 'jpeg-xmp-psir.xmp' );
+		$this->assertEquals( $expected, $res['XMP'] );
+	}
+
 	public function testXMPExtractionAltAppId() {
 		$res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-alt.jpg' );
 		$expected = file_get_contents( $this->filePath . 'jpeg-xmp-psir.xmp' );
-- 
2.20.1