Skip to content

Commit

Permalink
Merge pull request #2922 from Automattic/try/leave-https-images-alone
Browse files Browse the repository at this point in the history
Reader/PostNorm: Leave https image srcs from non-wpcom hosts alone.
  • Loading branch information
blowery committed Feb 4, 2016
2 parents b6140c2 + 0a7ba01 commit 3061f0f
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 36 deletions.
4 changes: 2 additions & 2 deletions client/lib/media/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe( 'MediaUtils', function() {
photon: true
} );

expect( url ).to.equal( 'https://i2.wp.com/secure.gravatar.com/blavatar/4e21d703d81809d215ceaabbf07efbc6' );
expect( url ).to.equal( 'https://i2.wp.com/secure.gravatar.com/blavatar/4e21d703d81809d215ceaabbf07efbc6?ssl=1' );
} );

it( 'should generate the correct width-constrained photon URL', function() {
Expand All @@ -55,7 +55,7 @@ describe( 'MediaUtils', function() {
maxWidth: 450
} );

expect( url ).to.equal( 'https://i2.wp.com/secure.gravatar.com/blavatar/4e21d703d81809d215ceaabbf07efbc6?w=450' );
expect( url ).to.equal( 'https://i2.wp.com/secure.gravatar.com/blavatar/4e21d703d81809d215ceaabbf07efbc6?ssl=1&w=450' );
} );

it( 'should generate the correct width-constrained URL', function() {
Expand Down
18 changes: 11 additions & 7 deletions client/lib/post-normalizer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ var assign = require( 'lodash/object/assign' ),
var formatting = require( 'lib/formatting' ),
safeImageURL = require( 'lib/safe-image-url' );


const DEFAULT_PHOTON_QUALITY = 80, // 80 was chosen after some heuristic testing as the best blend of size and quality
READING_WORDS_PER_SECOND = 250 / 60; // Longreads says that people can read 250 words per minute. We want the rate in words per second.

const imageScaleFactor = ( typeof window !== 'undefined' && window.devicePixelRatio && window.devicePixelRatio > 1 ) ? 2 : 1,
TRANSPARENT_GIF = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7';

function debugForPost( post ) {
return function( msg ) {
debug( post.global_ID + ': ' + msg );
Expand All @@ -50,10 +57,7 @@ function stripAutoPlays( query ) {
return query;
}

const DEFAULT_PHOTON_QUALITY = 80, // 80 was chosen after some heuristic testing as the best blend of size and quality
READING_WORDS_PER_SECOND = 250 / 60; // Longreads says that people can read 250 words per minute. We want the rate in words per second.

const imageScaleFactor = ( typeof window !== 'undefined' && window.devicePixelRatio && window.devicePixelRatio > 1 ) ? 2 : 1;

/**
* Asynchronously normalizes an object shaped like a post. Works on a copy of the post and does not mutate the original post.
Expand Down Expand Up @@ -510,24 +514,24 @@ normalizePost.content = {
image.parentNode.removeChild( image );
// fun fact: removing the node from the DOM will not prevent it from loading. You actually have to
// change out the src to change what loads. The following is a 1x1 transparent gif as a data URL
image.setAttribute( 'src', 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7' );
image.setAttribute( 'src', TRANSPARENT_GIF );
image.removeAttribute( 'srcset' );
return;
}

safeSource = safeImageURL( imgSource );
if ( maxWidth ) {
if ( safeSource && maxWidth ) {
safeSource = maxWidthPhotonishURL( safeSource, maxWidth );
}

image.setAttribute( 'src', safeSource );
image.setAttribute( 'src', safeSource || TRANSPARENT_GIF );

if ( image.hasAttribute( 'srcset' ) ) {
const imgSrcSet = srcset.parse( image.getAttribute( 'srcset' ) ).map( imgSrc => {
if ( ! url.parse( imgSrc.url, false, true ).hostname ) {
imgSrc.url = url.resolve( post.URL, imgSrc.url );
}
imgSrc.url = safeImageURL( imgSrc.url );
imgSrc.url = safeImageURL( imgSrc.url || TRANSPARENT_GIF );
return imgSrc;
} );
image.setAttribute( 'srcset', srcset.stringify( imgSrcSet ) );
Expand Down
9 changes: 1 addition & 8 deletions client/lib/safe-image-url/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var photon = require( 'photon' ),
* We special case gravatar, because we control them.
*
* @param {string} url The URL to secure
* @return {string} The secured URL
* @return {string} The secured URL, or null if we couldn't make it safe
*/
function safeImageURL( url ) {
if ( typeof url !== 'string' ) {
Expand All @@ -38,13 +38,6 @@ function safeImageURL( url ) {
return url.replace( /^http:/, 'https:' );
}

// Photon doesn't support query strings
if ( parsed.query ) {
delete parsed.search;
delete parsed.query;
url = uri.format( parsed );
}
// run it through photon, even if it had a querystring we couldn't strip
return photon( url );
}

Expand Down
18 changes: 9 additions & 9 deletions client/lib/safe-image-url/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ describe( 'safe-image-url', function() {
} );

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' );
expect( safeImage( 'https://example.com/foo' ) ).to.eql( 'https://i1.wp.com/example.com/foo?ssl=1' );
} );

it( 'should make wp-com like subdomain url safe', function() {
expect( safeImage( 'https://wordpress.com.example.com/foo' ) ).to.eql(
'https://i0.wp.com/wordpress.com.example.com/foo'
'https://i0.wp.com/wordpress.com.example.com/foo?ssl=1'
);
} );

it( 'should make domain ending by wp-com url safe', function() {
expect( safeImage( 'https://examplewordpress.com/foo' ) ).to.eql(
'https://i0.wp.com/examplewordpress.com/foo'
'https://i0.wp.com/examplewordpress.com/foo?ssl=1'
);
} );

Expand Down Expand Up @@ -58,11 +58,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 return null for urls with querystrings', function() {
expect( safeImage( 'https://example.com/foo?bar' ) ).to.be.null;
expect( safeImage( 'https://example.com/foo.jpg?bar' ) ).to.be.null;
expect( safeImage( 'https://example.com/foo.jpeg?bar' ) ).to.be.null;
expect( safeImage( 'https://example.com/foo.gif?bar' ) ).to.be.null;
expect( safeImage( 'https://example.com/foo.png?bar' ) ).to.be.null;
} );
} );
12 changes: 3 additions & 9 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"node-sass": "3.4.2",
"page": "1.6.1",
"phone": "git+https://github.com/Automattic/node-phone.git#1.0.4-8",
"photon": "1.0.4",
"photon": "2.0.0",
"q": "1.0.1",
"qrcode.react": "0.5.2",
"qs": "4.0.0",
Expand Down

0 comments on commit 3061f0f

Please sign in to comment.