From 370abeb177cc01fc2419ed4d5ffff50b385ed09d Mon Sep 17 00:00:00 2001 From: Ben Lowery Date: Fri, 29 Jan 2016 14:40:07 -0500 Subject: [PATCH] Reader/PostNorm: Leave https image srcs from non-wp hosts alone. This fixes images coming from private WPCOM sites and images from places like github and medium which block photon. It also fixes https images that rely on querystring arguments that photon cannot understand. --- client/lib/post-normalizer/index.js | 6 ++++ .../test/post-normalizer-test.js | 12 +++---- client/lib/safe-image-url/index.js | 7 +++- client/lib/safe-image-url/test/index.js | 32 +++++++++++++------ 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/client/lib/post-normalizer/index.js b/client/lib/post-normalizer/index.js index d2f7f53cfdb413..a3350dd5021d62 100644 --- a/client/lib/post-normalizer/index.js +++ b/client/lib/post-normalizer/index.js @@ -101,6 +101,12 @@ function maxWidthPhotonishURL( imageURL, width ) { return imageURL; } + if ( ! ( endsWith( parsedURL.host, 'wp.com' ) || + endsWith( parsedURL.host, 'wordpress.com' ) || + endsWith( parsedURL.host, 'gravatar.com' ) ) ) { + return imageURL; + } + isGravatar = parsedURL.host.indexOf( 'gravatar.com' ) !== -1; delete parsedURL.search; diff --git a/client/lib/post-normalizer/test/post-normalizer-test.js b/client/lib/post-normalizer/test/post-normalizer-test.js index 6d50882427863f..fdcf7f22a50330 100644 --- a/client/lib/post-normalizer/test/post-normalizer-test.js +++ b/client/lib/post-normalizer/test/post-normalizer-test.js @@ -218,9 +218,9 @@ describe( 'post-normalizer', function() { } }; normalizer( post, [ normalizer.safeImageProperties( 200 ) ], function( err, normalized ) { - assert.strictEqual( normalized.author.avatar_URL, 'http://example.com/me.jpg-SAFE?w=200&quality=80&strip=info' ); - assert.strictEqual( normalized.featured_image, 'http://foo.bar/-SAFE?w=200&quality=80&strip=info' ); - assert.strictEqual( normalized.featured_media.uri, 'http://example.com/media.jpg-SAFE?w=200&quality=80&strip=info' ); + assert.strictEqual( normalized.author.avatar_URL, 'http://example.com/me.jpg-SAFE' ); + assert.strictEqual( normalized.featured_image, 'http://foo.bar/-SAFE' ); + assert.strictEqual( normalized.featured_media.uri, 'http://example.com/media.jpg-SAFE' ); done( err ); } ); } ); @@ -417,13 +417,13 @@ describe( 'post-normalizer', function() { ); } ); - it( 'can route all images through photon if a size is specified', function( done ) { + it( 'only routes images through photon if a size is specified and the host is a photonable host', function( done ) { normalizer( { - content: '' + content: '' }, [ normalizer.withContentDOM( [ normalizer.content.safeContentImages( 400 ) ] ) ], function( err, normalized ) { - assert.equal( normalized.content, '' ); + assert.equal( normalized.content, '' ); done( err ); } ); diff --git a/client/lib/safe-image-url/index.js b/client/lib/safe-image-url/index.js index 16457cee52d5ea..5ad3b7ea8b8ace 100644 --- a/client/lib/safe-image-url/index.js +++ b/client/lib/safe-image-url/index.js @@ -2,7 +2,8 @@ * External Dependencies */ var photon = require( 'photon' ), - uri = require( 'url' ); + uri = require( 'url' ), + startsWith = require( 'lodash/string/startsWith' ); /** * Internal Dependencies @@ -31,6 +32,10 @@ function safeImageURL( url ) { return url; } + if ( startsWith( url, 'https:' ) ) { + return url; + } + const parsed = uri.parse( url, false, true ); if ( /^([-a-zA-Z0-9_]+\.)*(gravatar.com|wordpress.com|wp.com|a8c.com)$/.test( parsed.hostname ) ) { diff --git a/client/lib/safe-image-url/test/index.js b/client/lib/safe-image-url/test/index.js index 286a37b55f3f1c..af82f10d994f02 100644 --- a/client/lib/safe-image-url/test/index.js +++ b/client/lib/safe-image-url/test/index.js @@ -17,22 +17,34 @@ describe( 'safe-image-url', function() { expect( safeImage( 'http://example.com/foo' ) ).to.eql( 'https://i1.wp.com/example.com/foo' ); } ); - it( 'should make a non-wpcom https url safe', function() { - expect( safeImage( 'https://example.com/foo' ) ).to.eql( 'https://i1.wp.com/example.com/foo' ); + it( 'should leave non-wpcom https url alone', function() { + expect( safeImage( 'https://example.com/foo' ) ).to.eql( 'https://example.com/foo' ); } ); it( 'should make wp-com like subdomain url safe', function() { - expect( safeImage( 'https://wordpress.com.example.com/foo' ) ).to.eql( + expect( safeImage( 'http://wordpress.com.example.com/foo' ) ).to.eql( 'https://i0.wp.com/wordpress.com.example.com/foo' ); } ); + it( 'should leave wp-com like subdomain url alone', function() { + expect( safeImage( 'https://wordpress.com.example.com/foo' ) ).to.eql( + 'https://wordpress.com.example.com/foo' + ); + } ); + it( 'should make domain ending by wp-com url safe', function() { - expect( safeImage( 'https://examplewordpress.com/foo' ) ).to.eql( + expect( safeImage( 'http://examplewordpress.com/foo' ) ).to.eql( 'https://i0.wp.com/examplewordpress.com/foo' ); } ); + it( 'should leave domain ending by wp-com url alone', function() { + expect( safeImage( 'https://examplewordpress.com/foo' ) ).to.eql( + 'https://examplewordpress.com/foo' + ); + } ); + it( 'should make a non-wpcom protocol relative url safe', function() { expect( safeImage( '//example.com/foo' ) ).to.eql( 'https://i1.wp.com/example.com/foo' ); } ); @@ -58,11 +70,11 @@ describe( 'safe-image-url', function() { expect( safeImage( 'https://gravatar.com/' ) ).to.eql( 'https://gravatar.com/' ); } ); - it( 'should strip querystring args from photoned urls', function() { - expect( safeImage( 'https://example.com/foo?bar' ) ).to.eql( 'https://i1.wp.com/example.com/foo' ); - expect( safeImage( 'https://example.com/foo.jpg?bar' ) ).to.eql( 'https://i0.wp.com/example.com/foo.jpg' ); - expect( safeImage( 'https://example.com/foo.jpeg?bar' ) ).to.eql( 'https://i0.wp.com/example.com/foo.jpeg' ); - expect( safeImage( 'https://example.com/foo.gif?bar' ) ).to.eql( 'https://i2.wp.com/example.com/foo.gif' ); - expect( safeImage( 'https://example.com/foo.png?bar' ) ).to.eql( 'https://i0.wp.com/example.com/foo.png' ); + it( 'should leave https urls alone', function() { + expect( safeImage( 'https://example.com/foo?bar' ) ).to.eql( 'https://example.com/foo?bar' ); + expect( safeImage( 'https://example.com/foo.jpg?bar' ) ).to.eql( 'https://example.com/foo.jpg?bar' ); + expect( safeImage( 'https://example.com/foo.jpeg?bar' ) ).to.eql( 'https://example.com/foo.jpeg?bar' ); + expect( safeImage( 'https://example.com/foo.gif?bar' ) ).to.eql( 'https://example.com/foo.gif?bar' ); + expect( safeImage( 'https://example.com/foo.png?bar' ) ).to.eql( 'https://example.com/foo.png?bar' ); } ); } );