Skip to content

Commit

Permalink
Reader: if an image URL can't be made safe, discard it before the car…
Browse files Browse the repository at this point in the history
…d type is chosen (#46144)
  • Loading branch information
bluefuton authored Oct 8, 2020
1 parent 4fabc36 commit 41f07f8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 7 deletions.
10 changes: 5 additions & 5 deletions client/lib/post-normalizer/rule-keep-valid-images.js
Original file line number Diff line number Diff line change
@@ -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 );
}
Expand Down
30 changes: 28 additions & 2 deletions client/lib/post-normalizer/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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', () => {
Expand Down
3 changes: 3 additions & 0 deletions client/lib/post-normalizer/test/mocks/lib/safe-image-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
Expand Down
5 changes: 5 additions & 0 deletions client/lib/safe-image-url/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -83,6 +87,7 @@ export default function safeImageUrl( url ) {
if ( ! parsedUrl?.protocol ) {
parsedUrl.protocol = 'https';
}

url = getUrlFromParts( parsedUrl ).toString();
}

Expand Down

0 comments on commit 41f07f8

Please sign in to comment.