Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Commit

Permalink
MM-13924 Ensure OpenGraph and message attachment images go through th…
Browse files Browse the repository at this point in the history
…e proxy (#2331)

Doing this on the app instead of the server because it might otherwise cause problems when we're requesting image dimensions. I was looking at a more generic way of doing this, but this doesn't seem like the time to add it since we're just about to kick off RC testing.

#### Ticket Link
https://mattermost.atlassian.net/browse/MM-13924
  • Loading branch information
hmhealey committed Feb 7, 2019
1 parent d566bfe commit 53fa593
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';

import {doPostAction} from 'mattermost-redux/actions/posts';
import {getConfig} from 'mattermost-redux/selectors/entities/general';

import MessageAttachment from './message_attachment';

function mapStateToProps(state) {
return {
hasImageProxy: getConfig(state).HasImageProxy === 'true',
};
}

function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators({
Expand All @@ -16,4 +23,4 @@ function mapDispatchToProps(dispatch) {
};
}

export default connect(null, mapDispatchToProps)(MessageAttachment);
export default connect(mapStateToProps, mapDispatchToProps)(MessageAttachment);
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import React from 'react';

import {postListScrollChange} from 'actions/global_actions';

import {isUrlSafe} from 'utils/url.jsx';
import {getImageSrc} from 'utils/post_utils';
import {isUrlSafe} from 'utils/url';
import {handleFormattedTextClick} from 'utils/utils';

import Markdown from 'components/markdown';
Expand Down Expand Up @@ -36,6 +37,11 @@ export default class MessageAttachment extends React.PureComponent {
*/
options: PropTypes.object,

/**
* Whether or not the server has an image proxy enabled
*/
hasImageProxy: PropTypes.bool.isRequired,

/**
* images object for dimensions
*/
Expand Down Expand Up @@ -254,7 +260,7 @@ export default class MessageAttachment extends React.PureComponent {
author.push(
<img
className='attachment__author-icon'
src={attachment.author_icon}
src={getImageSrc(attachment.author_icon, this.props.hasImageProxy)}
key={'attachment__author-icon'}
height='14'
width='14'
Expand Down Expand Up @@ -333,7 +339,7 @@ export default class MessageAttachment extends React.PureComponent {
<SizeAwareImage
className='attachment__image'
onHeightReceived={this.handleHeightReceivedForImageUrl}
src={attachment.image_url}
src={getImageSrc(attachment.image_url, this.props.hasImageProxy)}
dimensions={this.props.imagesMetadata[attachment.image_url]}
/>
</div>
Expand All @@ -343,12 +349,10 @@ export default class MessageAttachment extends React.PureComponent {
let thumb;
if (attachment.thumb_url) {
thumb = (
<div
className='attachment__thumb-container'
>
<div className='attachment__thumb-container'>
<SizeAwareImage
onHeightReceived={this.handleHeightReceivedForThumbUrl}
src={attachment.thumb_url}
src={getImageSrc(attachment.thumb_url, this.props.hasImageProxy)}
dimensions={this.props.imagesMetadata[attachment.thumb_url]}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// See LICENSE.txt for license information.

import React from 'react';
import {shallow} from 'enzyme';
import {mount, shallow} from 'enzyme';

import MessageAttachment from 'components/post_view/message_attachments/message_attachment/message_attachment.jsx';
import {postListScrollChange} from 'actions/global_actions';
Expand All @@ -29,6 +29,7 @@ describe('components/post_view/MessageAttachment', () => {
postId: 'post_id',
attachment,
actions: {doPostAction: jest.fn()},
hasImageProxy: false,
imagesMetadata: {
image_url: {
height: 200,
Expand Down Expand Up @@ -118,4 +119,22 @@ describe('components/post_view/MessageAttachment', () => {
wrapper = shallow(<MessageAttachment {...props}/>);
expect(wrapper.instance().getFieldsTable()).toMatchSnapshot();
});

test('should proxy external images if image proxy is enabled', () => {
const props = {
...baseProps,
attachment: {
author_icon: 'http:https://example.com/author.png',
image_url: 'http:https://example.com/image.png',
thumb_url: 'http:https://example.com/thumb.png',
},
hasImageProxy: true,
};

const wrapper = mount(<MessageAttachment {...props}/>);

expect(wrapper.find('.attachment__author-icon').prop('src')).toMatch(`/api/v4/image?url=${encodeURIComponent(props.attachment.author_icon)}`);
expect(wrapper.find('.attachment__image-container img').prop('src')).toMatch(`/api/v4/image?url=${encodeURIComponent(props.attachment.image_url)}`);
expect(wrapper.find('.attachment__thumb-container img').prop('src')).toMatch(`/api/v4/image?url=${encodeURIComponent(props.attachment.thumb_url)}`);
});
});
2 changes: 2 additions & 0 deletions components/post_view/post_attachment_opengraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';
import {getOpenGraphMetadata} from 'mattermost-redux/actions/posts';
import {getConfig} from 'mattermost-redux/selectors/entities/general';
import {getOpenGraphMetadataForUrl} from 'mattermost-redux/selectors/entities/posts';
import {getCurrentUser} from 'mattermost-redux/selectors/entities/users';

Expand All @@ -14,6 +15,7 @@ import PostAttachmentOpenGraph from './post_attachment_opengraph.jsx';
function mapStateToProps(state, ownProps) {
return {
currentUser: getCurrentUser(state),
hasImageProxy: getConfig(state).HasImageProxy === 'true',
openGraphData: getOpenGraphMetadataForUrl(state, ownProps.link),
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as CommonUtils from 'utils/commons.jsx';
import {PostTypes} from 'utils/constants.jsx';
import {useSafeUrl} from 'utils/url';
import * as Utils from 'utils/utils.jsx';
import {isSystemMessage} from 'utils/post_utils.jsx';
import {getImageSrc, isSystemMessage} from 'utils/post_utils.jsx';
import {getFileDimensionsForDisplay} from 'utils/file_utils';

const MAX_DIMENSIONS_LARGE_IMAGE = {
Expand Down Expand Up @@ -50,6 +50,11 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
*/
openGraphData: PropTypes.object,

/**
* Whether or not the server has an image proxy enabled
*/
hasImageProxy: PropTypes.bool.isRequired,

isEmbedVisible: PropTypes.bool,
toggleEmbedVisibility: PropTypes.func.isRequired,

Expand All @@ -67,7 +72,7 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
super(props);

const removePreview = this.isRemovePreview(props.post, props.currentUser);
const imageUrl = this.getBestImageUrl(props.openGraphData);
const imageUrl = PostAttachmentOpenGraph.getBestImageUrl(props.openGraphData, props.hasImageProxy);
const {metadata} = props.post;
const hasLargeImage = metadata && metadata.images && metadata.images[imageUrl] && imageUrl ? this.hasLargeImage(metadata.images[imageUrl]) : false;

Expand Down Expand Up @@ -98,7 +103,7 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
}

if (!Utils.areObjectsEqual(nextProps.openGraphData, this.props.openGraphData)) {
const imageUrl = this.getBestImageUrl(nextProps.openGraphData);
const imageUrl = PostAttachmentOpenGraph.getBestImageUrl(nextProps.openGraphData, nextProps.hasImageProxy);
const {metadata} = nextProps.post;
const hasLargeImage = metadata && metadata.images && metadata.images[imageUrl] && imageUrl ? this.hasLargeImage(metadata.images[imageUrl]) : false;

Expand Down Expand Up @@ -129,15 +134,6 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
}
}

getBestImageUrl(data) {
if (!data || Utils.isEmptyObject(data.images)) {
return null;
}

const bestImage = CommonUtils.getNearestPoint(DIMENSIONS_NEAREST_POINT_IMAGE, data.images, 'width', 'height');
return bestImage.secure_url || bestImage.url;
}

hasLargeImage({height, width}) {
let hasLargeImage;
const largeImageMinRatio = 16 / 9;
Expand Down Expand Up @@ -355,4 +351,14 @@ export default class PostAttachmentOpenGraph extends React.PureComponent {
</div>
);
}

static getBestImageUrl(data, hasImageProxy) {
if (!data || !data.images || data.images.length === 0) {
return null;
}

const bestImage = CommonUtils.getNearestPoint(DIMENSIONS_NEAREST_POINT_IMAGE, data.images, 'width', 'height');

return getImageSrc(bestImage.secure_url || bestImage.url, hasImageProxy);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {Client4} from 'mattermost-redux/client';

import PostAttachmentOpenGraph from './post_attachment_opengraph';

describe('PostAttachmentOpenGraph', () => {
describe('getBestImageUrl', () => {
test('should return nothing with no OpenGraph metadata', () => {
const data = null;

expect(PostAttachmentOpenGraph.getBestImageUrl(data, false)).toEqual(null);
});

test('should return nothing with missing OpenGraph images', () => {
const data = {};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, false)).toEqual(null);
});

test('should return nothing with no OpenGraph images', () => {
const data = {
images: [],
};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, false)).toEqual(null);
});

test('should return secure_url if specified', () => {
const data = {
images: [{
secure_url: 'https://example.com/image.png',
url: 'http:https://example.com/image.png',
}],
};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, false)).toEqual(data.images[0].secure_url);
});

test('should return url if secure_url is not specified', () => {
const data = {
images: [{
secure_url: '',
url: 'http:https://example.com/image.png',
}],
};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, false)).toEqual(data.images[0].url);
});

test('should return a proxied image URL if the image proxy is enabled', () => {
const data = {
images: [{
url: 'http:https://example.com/image.png',
}],
};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, true).endsWith(`/api/v4/image?url=${encodeURIComponent(data.images[0].url)}`)).toEqual(true);
});

test('should not mangle a URL that is already proxied if the image proxy is enabled', () => {
const data = {
images: [{
url: `${Client4.getBaseRoute()}/image?url=${encodeURIComponent('http:https://example.com/image.png')}`,
}],
};

expect(PostAttachmentOpenGraph.getBestImageUrl(data, true)).toEqual(data.images[0].url);
});
});
});
11 changes: 9 additions & 2 deletions utils/post_utils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,16 @@ export function isEdited(post) {
}

export function getImageSrc(src, hasImageProxy) {
if (hasImageProxy) {
return Client4.getBaseRoute() + '/image?url=' + encodeURIComponent(src);
if (!src) {
return src;
}

const imageAPI = Client4.getBaseRoute() + '/image?url=';

if (hasImageProxy && !src.startsWith(imageAPI)) {
return imageAPI + encodeURIComponent(src);
}

return src;
}

Expand Down

0 comments on commit 53fa593

Please sign in to comment.