mirror of
https://github.com/mattermost/mattermost.git
synced 2025-02-25 18:55:24 -06:00
Add handling of posts with unsafe links. (#26129)
* Add handling of posts with unsafe links. * Lint * Add some tests. * Remove interal links exception * Add exception for links that start with siteurl, except image proxy * Allow only permalinks * Don't interperate regex in siteURL * Negate the external requests helper * Modify prop check to only check for 'true' * Move regex outside function.
This commit is contained in:
parent
daab9d5ff5
commit
3bf8574b0d
@ -5,9 +5,23 @@ import {connect} from 'react-redux';
|
|||||||
import {bindActionCreators} from 'redux';
|
import {bindActionCreators} from 'redux';
|
||||||
import type {Dispatch} from 'redux';
|
import type {Dispatch} from 'redux';
|
||||||
|
|
||||||
|
import {getPost} from 'mattermost-redux/selectors/entities/posts';
|
||||||
|
|
||||||
import {openModal} from 'actions/views/modals';
|
import {openModal} from 'actions/views/modals';
|
||||||
|
|
||||||
|
import type {GlobalState} from 'types/store';
|
||||||
|
|
||||||
import MarkdownImage from './markdown_image';
|
import MarkdownImage from './markdown_image';
|
||||||
|
import type {Props} from './markdown_image';
|
||||||
|
|
||||||
|
function mapStateToProps(state: GlobalState, ownProps: Props) {
|
||||||
|
const post = getPost(state, ownProps.postId);
|
||||||
|
const isUnsafeLinksPost = post?.props?.unsafe_links === 'true';
|
||||||
|
|
||||||
|
return {
|
||||||
|
isUnsafeLinksPost,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
function mapDispatchToProps(dispatch: Dispatch) {
|
function mapDispatchToProps(dispatch: Dispatch) {
|
||||||
return {
|
return {
|
||||||
@ -17,6 +31,6 @@ function mapDispatchToProps(dispatch: Dispatch) {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const connector = connect(null, mapDispatchToProps);
|
const connector = connect(mapStateToProps, mapDispatchToProps);
|
||||||
|
|
||||||
export default connector(MarkdownImage);
|
export default connector(MarkdownImage);
|
||||||
|
@ -332,4 +332,12 @@ describe('components/MarkdownImage', () => {
|
|||||||
|
|
||||||
expect(childrenWrapper).toMatchSnapshot();
|
expect(childrenWrapper).toMatchSnapshot();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('should render a alt text if the link is unsafe', () => {
|
||||||
|
const props = {...baseProps, isUnsafeLinksPost: true};
|
||||||
|
const wrapper = shallow(
|
||||||
|
<MarkdownImage {...props}/>,
|
||||||
|
);
|
||||||
|
expect(wrapper.text()).toBe(props.alt);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
@ -36,6 +36,7 @@ export type Props = {
|
|||||||
openModal: <P>(modalData: ModalData<P>) => void;
|
openModal: <P>(modalData: ModalData<P>) => void;
|
||||||
};
|
};
|
||||||
hideUtilities?: boolean;
|
hideUtilities?: boolean;
|
||||||
|
isUnsafeLinksPost: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
type State = {
|
type State = {
|
||||||
@ -156,6 +157,9 @@ export default class MarkdownImage extends PureComponent<Props, State> {
|
|||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
if (this.props.isUnsafeLinksPost) {
|
||||||
|
return <>{alt}</>;
|
||||||
|
}
|
||||||
return (
|
return (
|
||||||
<ExternalImage
|
<ExternalImage
|
||||||
src={src}
|
src={src}
|
||||||
|
@ -163,4 +163,23 @@ this is long text this is long text this is long text this is long text this is
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('unsafe mode links are rendered as text : link', () => {
|
||||||
|
const testCases = [
|
||||||
|
{input: '[link text](http://markdownlink.com)', expected: '<p>link text : http://markdownlink.com</p>'},
|
||||||
|
{input: '[link text](//markdownlink.com/test)', expected: '<p>link text : //markdownlink.com/test</p>'},
|
||||||
|
{input: '[link text](http://my.site.com/whatever)', expected: '<p>link text : http://my.site.com/whatever</p>'},
|
||||||
|
{input: '[link text](http://my.site.com/api/v4/image?url=ohno)', expected: '<p>link text : http://my.site.com/api/v4/image?url=ohno</p>'},
|
||||||
|
{input: '[link text](http://my.site.com/_redirect/pl/c18xpcpusjd88en1g4j7us31ur)', expected: '<p><a class="theme markdown__link" href="http://my.site.com/_redirect/pl/c18xpcpusjd88en1g4j7us31ur" rel="noreferrer" data-link="/_redirect/pl/c18xpcpusjd88en1g4j7us31ur">link text</a></p>'},
|
||||||
|
{input: '[link text](http://my.site.com/someteam/pl/c18xpcpusjd88en1g4j7us31ur)', expected: '<p><a class="theme markdown__link" href="http://my.site.com/someteam/pl/c18xpcpusjd88en1g4j7us31ur" rel="noreferrer" data-link="/someteam/pl/c18xpcpusjd88en1g4j7us31ur">link text</a></p>'},
|
||||||
|
{input: '[link text](http://my.site.com/_redirect/pl/c18xpcpusjd88en1g4j7us31ur/ohno)', expected: '<p>link text : http://my.site.com/_redirect/pl/c18xpcpusjd88en1g4j7us31ur/ohno</p>'},
|
||||||
|
{input: '[link text](http://my.site.com/more/stuff/here/pl/c18xpcpusjd88en1g4j7us31ur)', expected: '<p>link text : http://my.site.com/more/stuff/here/pl/c18xpcpusjd88en1g4j7us31ur</p>'},
|
||||||
|
{input: '[link text](http://myqsite.com/someteam/pl/c18xpcpusjd88en1g4j7us31ur)', expected: '<p>link text : http://myqsite.com/someteam/pl/c18xpcpusjd88en1g4j7us31ur</p>'},
|
||||||
|
|
||||||
|
];
|
||||||
|
for (const testCase of testCases) {
|
||||||
|
const output = format(testCase.input, {unsafeLinks: true, siteURL: 'http://my.site.com'});
|
||||||
|
expect(output).toEqual(testCase.expected);
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
@ -7,7 +7,7 @@ import type {MarkedOptions} from 'marked';
|
|||||||
import EmojiMap from 'utils/emoji_map';
|
import EmojiMap from 'utils/emoji_map';
|
||||||
import * as PostUtils from 'utils/post_utils';
|
import * as PostUtils from 'utils/post_utils';
|
||||||
import * as TextFormatting from 'utils/text_formatting';
|
import * as TextFormatting from 'utils/text_formatting';
|
||||||
import {getScheme, isUrlSafe, shouldOpenInNewTab} from 'utils/url';
|
import {mightTriggerExternalRequest, getScheme, isUrlSafe, shouldOpenInNewTab} from 'utils/url';
|
||||||
|
|
||||||
import {parseImageDimensions} from './helpers';
|
import {parseImageDimensions} from './helpers';
|
||||||
|
|
||||||
@ -136,6 +136,13 @@ export default class Renderer extends marked.Renderer {
|
|||||||
public link(href: string, title: string, text: string, isUrl = false) {
|
public link(href: string, title: string, text: string, isUrl = false) {
|
||||||
let outHref = href;
|
let outHref = href;
|
||||||
|
|
||||||
|
if (this.formattingOptions.unsafeLinks && mightTriggerExternalRequest(href, this.formattingOptions.siteURL)) {
|
||||||
|
if (text === href) {
|
||||||
|
return text;
|
||||||
|
}
|
||||||
|
return text + ' : ' + href;
|
||||||
|
}
|
||||||
|
|
||||||
if (!href.startsWith('/')) {
|
if (!href.startsWith('/')) {
|
||||||
const scheme = getScheme(href);
|
const scheme = getScheme(href);
|
||||||
if (!scheme) {
|
if (!scheme) {
|
||||||
|
@ -200,6 +200,13 @@ export interface TextFormattingOptionsBase {
|
|||||||
* Defaults to `false`.
|
* Defaults to `false`.
|
||||||
*/
|
*/
|
||||||
atPlanMentions: boolean;
|
atPlanMentions: boolean;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* If true, the renderer will assume links are not safe.
|
||||||
|
*
|
||||||
|
* Defaults to `false`.
|
||||||
|
*/
|
||||||
|
unsafeLinks: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type TextFormattingOptions = Partial<TextFormattingOptionsBase>;
|
export type TextFormattingOptions = Partial<TextFormattingOptionsBase>;
|
||||||
@ -227,6 +234,7 @@ const DEFAULT_OPTIONS: TextFormattingOptions = {
|
|||||||
proxyImages: false,
|
proxyImages: false,
|
||||||
editedAt: 0,
|
editedAt: 0,
|
||||||
postId: '',
|
postId: '',
|
||||||
|
unsafeLinks: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -168,6 +168,24 @@ export function validateChannelUrl(url: string, intl?: IntlShape): Array<React.R
|
|||||||
return errors;
|
return errors;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Returns true when the URL could possibly cause any external requests.
|
||||||
|
// Currently returns false only for permalinks
|
||||||
|
const permalinkPath = new RegExp('^/[0-9a-z_-]{1,64}/pl/[0-9a-z_-]{26}$');
|
||||||
|
export function mightTriggerExternalRequest(url: string, siteURL?: string): boolean {
|
||||||
|
if (siteURL && siteURL !== '') {
|
||||||
|
let standardSiteURL = siteURL;
|
||||||
|
if (standardSiteURL[standardSiteURL.length - 1] === '/') {
|
||||||
|
standardSiteURL = standardSiteURL.substring(0, standardSiteURL.length - 1);
|
||||||
|
}
|
||||||
|
if (!url.startsWith(standardSiteURL)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
const afterSiteURL = url.substring(standardSiteURL.length);
|
||||||
|
return !permalinkPath.test(afterSiteURL);
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
export function isInternalURL(url: string, siteURL?: string): boolean {
|
export function isInternalURL(url: string, siteURL?: string): boolean {
|
||||||
return url.startsWith(siteURL || '') || url.startsWith('/');
|
return url.startsWith(siteURL || '') || url.startsWith('/');
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user