Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: LEAP-344: View all annotation tab is not loading monorepo #5244

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion label_studio/frontend/dist/lsf/js/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion label_studio/frontend/dist/lsf/js/main.js.map

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions label_studio/frontend/dist/lsf/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "fix: OPTIC-341: Incorrect icon width causing layout issues with TabPanels in monorepo",
"commit": "2f5b6e1f3aac6aacf23b66ac64f1d6a8f34d4036",
"branch": "master",
"date": "2023-12-08T16:36:04Z"
"message": "fix: LEAP-344: View all annotation tab is not loading",
"commit": "ddb613a1a98e5a78a939601907a06bf9e9882367",
"branch": "fb-leap-344",
"date": "2024-01-08T11:24:45Z"
}
3 changes: 2 additions & 1 deletion web/libs/editor/src/components/App/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { FF_DEV_1170, FF_DEV_3873, FF_LSDV_4620_3_ML, isFF } from '../../utils/f
import { Annotation } from './Annotation';
import { Button } from '../../common/Button/Button';
import { reactCleaner } from '../../utils/reactCleaner';
import { sanitizeHtml } from '../../utils/html';

/**
* App
Expand Down Expand Up @@ -241,7 +242,7 @@ class App extends Component {
<>
{store.showingDescription && (
<Segment>
<div dangerouslySetInnerHTML={{ __html: store.description }} />
<div dangerouslySetInnerHTML={{ __html: sanitizeHtml(store.description) }} />
</Segment>
)}
</>
Expand Down
5 changes: 5 additions & 0 deletions web/libs/editor/src/components/App/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ This triggers next rerender with next annotation until all the annotations are r
class Item extends Component {
componentDidMount() {
Promise.all(this.props.annotation.objects.map(o => {
// as the image has lazy load, and the image is not being added to the viewport
// until it's loaded we need to skip the validation assuming that it's always ready,
// otherwise we'll get a blank canvas
if (o.type === 'image') return Promise.resolve();

return o.isReady
? Promise.resolve(o.isReady)
: new Promise(resolve => {
Expand Down
6 changes: 3 additions & 3 deletions web/libs/editor/src/components/Comments/CommentForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { FF_DEV_3873, isFF } from '../../utils/feature-flags';

export type CommentFormProps = {
commentStore: any,
value?: string,
annotationStore: any,
onChange?: (value: string) => void,
inline?: boolean,
rows?: number,
Expand All @@ -19,7 +19,7 @@ export type CommentFormProps = {

export const CommentForm: FC<CommentFormProps> = observer(({
commentStore,
value = '',
annotationStore,
inline = true,
onChange,
rows = 1,
Expand Down Expand Up @@ -52,7 +52,6 @@ export const CommentForm: FC<CommentFormProps> = observer(({
commentStore.setCurrentComment(comment || '');
}, [commentStore]);


useEffect(() => {
if (!isFF(FF_DEV_3873)) {
commentStore.setAddedCommentThisSession(false);
Expand All @@ -72,6 +71,7 @@ export const CommentForm: FC<CommentFormProps> = observer(({
commentStore.setCommentFormSubmit(() => onSubmit());
}, [actionRef, commentStore]);

const value = commentStore.currentComment[annotationStore.selected.id] || '';

return (
<Block ref={formRef} tag="form" name="comment-form" mod={{ inline }} onSubmit={onSubmit}>
Expand Down
4 changes: 2 additions & 2 deletions web/libs/editor/src/components/Comments/Comments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { FF_DEV_3034, isFF } from '../../utils/feature-flags';
import './Comments.styl';


export const Comments: FC<{ commentStore: any, cacheKey?: string }> = observer(({ commentStore, cacheKey }) => {
export const Comments: FC<{ annotationStore: any, commentStore: any, cacheKey?: string }> = observer(({ annotationStore, commentStore, cacheKey }) => {
const mounted = useMounted();

const loadComments = async () => {
Expand Down Expand Up @@ -45,7 +45,7 @@ export const Comments: FC<{ commentStore: any, cacheKey?: string }> = observer((

return (
<Block name="comments">
<CommentForm commentStore={commentStore} inline />
<CommentForm commentStore={commentStore} annotationStore={annotationStore} inline />
<CommentsList commentStore={commentStore} />
</Block>
);
Expand Down
3 changes: 2 additions & 1 deletion web/libs/editor/src/components/ErrorMessage/ErrorMessage.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React from 'react';
import styles from './ErrorMessage.module.scss';
import { sanitizeHtml } from '../../utils/html';

export const ErrorMessage = ({ error }) => {
if (typeof error === 'string') {
return <div className={styles.error} dangerouslySetInnerHTML={{ __html: error }} />;
return <div className={styles.error} dangerouslySetInnerHTML={{ __html: sanitizeHtml(error) }} />;
}
const body = error instanceof Error ? error.message : error;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Modal } from 'antd';
import { sanitizeHtml } from '../../utils/html';

export const InstructionsModal = ({
title,
Expand Down Expand Up @@ -50,7 +51,7 @@ export const InstructionsModal = ({
{typeof children === 'string' ? (
<p
style={contentStyle}
dangerouslySetInnerHTML={{ __html: children }}
dangerouslySetInnerHTML={{ __html: sanitizeHtml(children) }}
/>
) : (
<p style={contentStyle}>{children}</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const CommentsTab: FC<any> = inject('store')(observer(({ store }) => {
<Block name="comments-panel">
<Elem name="section-tab">
<Elem name="section-content">
<CommentsComponent commentStore={store.commentStore} cacheKey={`task.${store.task.id}`} />
<CommentsComponent annotationStore={store.annotationStore} commentStore={store.commentStore} cacheKey={`task.${store.task.id}`} />
</Elem>
</Elem>
</Block>
Expand Down Expand Up @@ -169,6 +169,7 @@ const GeneralPanel: FC<any> = inject('store')(observer(({ store, currentEntity }
</Elem>
<Elem name="section-content">
<CommentsComponent
annotationStore={store.annotationStore}
commentStore={store.commentStore}
cacheKey={`task.${store.task.id}`}
/>
Expand Down
32 changes: 23 additions & 9 deletions web/libs/editor/src/core/Hotkey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ keymaster.filter = function(event) {
const ALIASES = {
'plus': '=', // "ctrl plus" is actually a "ctrl =" because shift is not used
'minus': '-',
// Here is a magic trick. Keymaster doesn't work with comma correctly (it breaks down everything upon unbinding), but the key code for comma it expects is 188
// And the magic is that '¼' has the same keycode. So we are going to trick keymaster to handle this in the right way.
',': '¼',
};

export const Hotkey = (
Expand Down Expand Up @@ -144,17 +147,27 @@ export const Hotkey = (
});
};

const getKeys = (key: string) => {
const tokenRegex = /((?:\w+\+)*(?:[^,]+|,)),?/g;

return [...key.replace(/\s/,'').matchAll(tokenRegex)].map(match => match[1]);
};

const unbind = () => {
for (const scope of [DEFAULT_SCOPE, INPUT_SCOPE]) {
for (const key of Object.keys(_hotkeys_map)) {
if (isFF(FF_LSDV_1148)) {
removeKeyHandlerRef(scope, key);
keymaster.unbind(key, scope);
rebindKeyHandlers(scope, key);
} else {
keymaster.unbind(key, scope);
const keys = getKeys(key);

for (const key of keys) {
if (isFF(FF_LSDV_1148)) {
removeKeyHandlerRef(scope, key);
keymaster.unbind(key, scope);
rebindKeyHandlers(scope, key);
} else {
keymaster.unbind(key, scope);
}
delete _hotkeys_desc[key];
}
delete _hotkeys_desc[key];
}
}

Expand All @@ -165,8 +178,9 @@ export const Hotkey = (

return {
applyAliases(key: string) {
return key
.split(',')
const keys = getKeys(key);

return keys
.map(k => k.split('+').map(k => ALIASES[k.trim()] ?? k).join('+'))
.join(',');
},
Expand Down
6 changes: 2 additions & 4 deletions web/libs/editor/src/mixins/Regions.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ const RegionsMixin = types

score: types.maybeNull(types.number),

hidden: types.optional(types.boolean, false),

filtered: types.optional(types.boolean, false),

parentID: types.optional(types.string, ''),
Expand All @@ -23,8 +21,6 @@ const RegionsMixin = types
// Dynamic preannotations enabled
dynamic: false,

locked: false,

origin: types.optional(types.enumeration([
'prediction',
'prediction-changed',
Expand All @@ -36,6 +32,8 @@ const RegionsMixin = types
.volatile(() => ({
// selected: false,
_highlighted: false,
hidden: false,
locked: false,
isDrawing: false,
perRegionFocusRequest: null,
shapeRef: null,
Expand Down
4 changes: 2 additions & 2 deletions web/libs/editor/src/stores/Comment/CommentStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const CommentStore = types
.volatile(() => ({
addedCommentThisSession: false,
commentFormSubmit: () => {},
currentComment: '',
currentComment: {},
inputRef: {},
tooltipMessage: '',
}))
Expand Down Expand Up @@ -75,7 +75,7 @@ export const CommentStore = types
}

function setCurrentComment(comment) {
self.currentComment = comment;
self.currentComment = { ...self.currentComment, [self.annotation.id]: comment };
}

function setCommentFormSubmit(submitCallback) {
Expand Down
3 changes: 2 additions & 1 deletion web/libs/editor/src/tags/control/Choice.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { Block, Elem } from '../../utils/bem';
import './Choice/Choice.styl';
import { LsChevron } from '../../assets/icons';
import { HintTooltip } from '../../components/Taxonomy/Taxonomy';
import { sanitizeHtml } from '../../utils/html';

/**
* The `Choice` tag represents a single choice for annotations. Use with the `Choices` tag or `Taxonomy` tag to provide specific choice options.
Expand Down Expand Up @@ -207,7 +208,7 @@ const HtxNewChoiceView = ({ item, store }) => {
onChange={changeHandler}
>
<HintTooltip title={item.hint} wrapper="span">
{item.html ? <span dangerouslySetInnerHTML={{ __html: item.html }}/> : item._value }
{item.html ? <span dangerouslySetInnerHTML={{ __html: sanitizeHtml(item.html) }}/> : item._value }
{showHotkey && (<Hint>[{item.hotkey}]</Hint>)}
</HintTooltip>
</Elem>
Expand Down
3 changes: 2 additions & 1 deletion web/libs/editor/src/tags/control/Label.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import ToolsManager from '../../tools/Manager';
import Utils from '../../utils';
import { parseValue } from '../../utils/data';
import { FF_DEV_2128, isFF } from '../../utils/feature-flags';
import { sanitizeHtml } from '../../utils/html';

/**
* The `Label` tag represents a single label. Use with the `Labels` tag, including `BrushLabels`, `EllipseLabels`, `HyperTextLabels`, `KeyPointLabels`, and other `Labels` tags to specify the value of a specific label.
Expand Down Expand Up @@ -304,7 +305,7 @@ const HtxLabelView = inject('store')(
selected={item.selected}
onClick={item.onClick}
>
{item.html ? <div title={item._value} dangerouslySetInnerHTML={{ __html: item.html }}/> : item._value }
{item.html ? <div title={item._value} dangerouslySetInnerHTML={{ __html: sanitizeHtml(item.html) }}/> : item._value }
{item.showalias === true && item.alias && (
<span style={Utils.styleToProp(item.aliasstyle)}>&nbsp;{item.alias}</span>
)}
Expand Down
9 changes: 7 additions & 2 deletions web/libs/editor/src/tags/object/RichText/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { findRangeNative, rangeToGlobalOffset } from '../../../utils/selection-t
import { escapeHtml, isValidObjectURL } from '../../../utils/utilities';
import ObjectBase from '../Base';
import { cloneNode } from '../../../core/Helpers';
import { FF_LSDV_4620_3, isFF } from '../../../utils/feature-flags';
import { FF_LSDV_4620_3, FF_SAFE_TEXT, isFF } from '../../../utils/feature-flags';
import DomManager from './domManager';
import { STATE_CLASS_MODS } from '../../../mixins/HighlightMixin';
import Constants from '../../../core/Constants';
Expand Down Expand Up @@ -220,7 +220,12 @@ const Model = types

// clean up the html — remove scripts and iframes
// nodes count better be the same, so replace them with stubs
self._value = sanitizeHtml(String(val));
// we should not sanitize text tasks because we already have htmlEscape in view.js
if (isFF(FF_SAFE_TEXT) && self.type === 'text') {
self._value = val;
} else {
self._value = sanitizeHtml(String(val));
}

self._regionsCache.forEach(({ region, annotation }) => {
region.setText(self._value.substring(region.startOffset, region.endOffset));
Expand Down
3 changes: 2 additions & 1 deletion web/libs/editor/src/tags/visual/Style.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { observer } from 'mobx-react';

import Registry from '../../core/Registry';
import { guidGenerator } from '../../utils/unique';
import { sanitizeHtml } from '../../utils/html';

/**
* The `Style` tag is used in combination with the View tag to apply custom CSS properties to the labeling interface. See the [CSS Reference](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference) on the MDN page for a full list of available properties that you can reference. You can also adjust default Label Studio CSS classes. Use the browser developer tools to inspect the element on the UI and locate the class name, then specify that class name in the `Style` tag.
Expand Down Expand Up @@ -69,7 +70,7 @@ const Model = types.model({
const StyleModel = types.compose('StyleModel', Model);

const HtxStyle = observer(({ item }) => {
return <style dangerouslySetInnerHTML={{ __html: item.value }}></style>;
return <style dangerouslySetInnerHTML={{ __html: sanitizeHtml(item.value) }}></style>;
});

Registry.addTag('style', StyleModel, HtxStyle);
Expand Down
35 changes: 35 additions & 0 deletions web/libs/editor/src/utils/__tests__/html.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { sanitizeHtml } from '../html';

const htmlSanitizeList = [
{
input: '<iframe src="http://malicious.com"></iframe>',
expected: '',
},{
input: '<script>alert(\'XSS\');</script>',
expected: '',
}, {
input: '"><img src=x onerror=alert(\'XSS\')>',
expected: '"&gt;<img src="x" />',
},{
input: '<script>alert(1)</script foo=\'bar\'>',
expected: '',
},{
input: '><script>alert(\'XSS\')</script>',
expected: '&gt;',
},{
input: '<?xml version="1.0" encoding="ISO-8859-1"?><foo><![CDATA[<script>alert(\'XSS\');</script>]]></foo>',
expected: '<foo></foo>',
},{
input: 'It\'s a test to check if <, > and & are escaped',
expected: 'It\'s a test to check if &lt;, &gt; and &amp; are escaped',
},
];


describe('Helper function html sanitize', () => {
test('sanitize html list', () => {
htmlSanitizeList.forEach((item) => {
expect(sanitizeHtml(item.input)).toBe(item.expected);
});
});
});
4 changes: 2 additions & 2 deletions web/libs/editor/src/utils/bem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ const assembleClass = (block: string, elem?: string, mix?: CNMix | CNMix[], mod?
}

const attachNamespace = (cls: string) => {
if (new RegExp(CSS_PREFIX).test(cls)) return cls;
else return `${CSS_PREFIX}${cls}`;
if (typeof cls !== 'string') console.error('Non-string classname: ', cls);
return String(cls).startsWith(CSS_PREFIX) ? cls : `${CSS_PREFIX}${cls}`;
};

return finalClass.map(attachNamespace).join(' ');
Expand Down
1 change: 1 addition & 0 deletions web/libs/editor/src/utils/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ export const FF_LEAP_187 = 'fflag_feat_front_leap_187_video_seek_on_select_short
*/
export const FF_ZOOM_OPTIM = 'fflag_fix_front_leap_32_zoom_perf_190923_short';

export const FF_SAFE_TEXT = 'fflag_fix_leap_466_text_sanitization';

Object.assign(window, {
APP_SETTINGS: {
Expand Down
11 changes: 10 additions & 1 deletion web/libs/editor/src/utils/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,11 +540,20 @@ function sanitizeHtml(html = []) {
'ontimeupdate', 'ontoggle', 'onunhandledrejection', 'onunload',
'onvolumechange', 'onwaiting', 'onwheel'];

const disallowedTags = {
'script': true,
'iframe': true,
};

return sanitizeHTML(html, {
allowedTags: false,
allowedAttributes: false,
disallowedTagsMode: 'discard',
disallowedTags: ['script', 'iframe'],
allowVulnerableTags: true,
exclusiveFilter(frame) {
//...except those in the blacklist
return disallowedTags[frame.tag];
},
nonTextTags: ['script', 'textarea', 'option', 'noscript'],
transformTags: {
'*': (tagName, attribs) => {
Expand Down