From 41f07f86dbfefc562ccd66aa5eaa2f5b4687db3e Mon Sep 17 00:00:00 2001
From: Chris R <chris.rosser@automattic.com>
Date: Fri, 9 Oct 2020 10:38:06 +1300
Subject: [PATCH] Reader: if an image URL can't be made safe, discard it before
 the card type is chosen (#46144)

---
 .../post-normalizer/rule-keep-valid-images.js | 10 +++----
 client/lib/post-normalizer/test/index.js      | 30 +++++++++++++++++--
 .../test/mocks/lib/safe-image-url.js          |  3 ++
 client/lib/safe-image-url/index.js            |  5 ++++
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/client/lib/post-normalizer/rule-keep-valid-images.js b/client/lib/post-normalizer/rule-keep-valid-images.js
index 775fb016ecb992..3c131fab7aa7ee 100644
--- a/client/lib/post-normalizer/rule-keep-valid-images.js
+++ b/client/lib/post-normalizer/rule-keep-valid-images.js
@@ -1,22 +1,22 @@
 /**
  * External dependencies
  */
-
 import { filter } from 'lodash';
 
 /**
- * Internal Dependencies
+ * Internal dependencies
  */
+import safeImageUrl from 'lib/safe-image-url';
 
-function imageHasMinWidthAndHeight( width, height ) {
+function isValidImage( width, height ) {
 	return function ( image ) {
-		return image.width >= width && image.height >= height;
+		return image.width >= width && image.height >= height && safeImageUrl( image.src );
 	};
 }
 
 export default function keepValidImages( minWidth, minHeight ) {
 	return function keepValidImagesForWidthAndHeight( post ) {
-		const imageFilter = imageHasMinWidthAndHeight( minWidth, minHeight );
+		const imageFilter = isValidImage( minWidth, minHeight );
 		if ( post.images ) {
 			post.images = filter( post.images, imageFilter );
 		}
diff --git a/client/lib/post-normalizer/test/index.js b/client/lib/post-normalizer/test/index.js
index f0497d0c67cc04..6eb26f56f7fae9 100644
--- a/client/lib/post-normalizer/test/index.js
+++ b/client/lib/post-normalizer/test/index.js
@@ -589,9 +589,9 @@ describe( 'index', () => {
 	} );
 
 	describe( 'keepValidImages', () => {
-		test( 'should filter post.images based on size', () => {
+		test( 'should filter post.images and post.content_images based on size', () => {
 			function fakeImage( width, height ) {
-				return { width, height };
+				return { width, height, src: 'http://example.com/giraffe.jpg' };
 			}
 
 			const post = {
@@ -609,6 +609,32 @@ describe( 'index', () => {
 			expect( normalized.images ).toHaveLength( 1 );
 			expect( normalized.content_images ).toHaveLength( 2 );
 		} );
+
+		test( 'should filter post.images and post.content_images for unsafe URLs', () => {
+			function fakeImage( src ) {
+				return { width: 500, height: 500, src };
+			}
+
+			const goodSrc = 'http://example.com/giraffe.jpg';
+			// External images with query string parameters that aren't related to image formatting
+			// should be omitted
+			const badSrc = 'http://example.com/adserver?ad=123456';
+
+			const post = {
+				images: [ fakeImage( goodSrc ), fakeImage( goodSrc ), fakeImage( badSrc ) ],
+				content_images: [
+					fakeImage( goodSrc ),
+					fakeImage( goodSrc ),
+					fakeImage( goodSrc ),
+					fakeImage( badSrc ),
+				],
+			};
+
+			const normalized = keepValidImages( 100, 200 )( post );
+
+			expect( normalized.images ).toHaveLength( 2 );
+			expect( normalized.content_images ).toHaveLength( 3 );
+		} );
 	} );
 
 	describe( 'content.makeEmbedsSafe', () => {
diff --git a/client/lib/post-normalizer/test/mocks/lib/safe-image-url.js b/client/lib/post-normalizer/test/mocks/lib/safe-image-url.js
index 3f112826806874..f1842b4d2ee974 100644
--- a/client/lib/post-normalizer/test/mocks/lib/safe-image-url.js
+++ b/client/lib/post-normalizer/test/mocks/lib/safe-image-url.js
@@ -11,6 +11,9 @@ let returnValue;
 
 function makeSafe( url ) {
 	const parts = getUrlParts( url );
+	if ( parts.searchParams.get( 'ad' ) ) {
+		return null;
+	}
 	if ( ! parts.protocol ) {
 		parts.protocol = 'fake';
 	}
diff --git a/client/lib/safe-image-url/index.js b/client/lib/safe-image-url/index.js
index b9f6a3b8ec3bd8..f00dddf9a02be2 100644
--- a/client/lib/safe-image-url/index.js
+++ b/client/lib/safe-image-url/index.js
@@ -54,6 +54,10 @@ export default function safeImageUrl( url ) {
 		return null;
 	}
 
+	if ( url.length < 1 ) {
+		return null;
+	}
+
 	if ( REGEX_EXEMPT_URL.test( url ) ) {
 		return url;
 	}
@@ -83,6 +87,7 @@ export default function safeImageUrl( url ) {
 		if ( ! parsedUrl?.protocol ) {
 			parsedUrl.protocol = 'https';
 		}
+
 		url = getUrlFromParts( parsedUrl ).toString();
 	}