Skip to content

Commit

Permalink
fix: LEAP-344: View all annotation tab is not loading (#5245)
Browse files Browse the repository at this point in the history
* [submodules] Build static HumanSignal/label-studio-frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7402267953

* [submodules] Build static HumanSignal/label-studio-frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7402358685

* [submodules] Build static HumanSignal/label-studio-frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7446764529

* ci: Build frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7446809287

* [submodules] Copy src HumanSignal/label-studio-frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7447643750

* [submodules] Build static HumanSignal/label-studio-frontend

Workflow run: https://github.com/HumanSignal/label-studio/actions/runs/7447643750

---------

Co-authored-by: Julio Sgarbi <julio.sgarbi@hotmail.com>
Co-authored-by: juliosgarbi <juliosgarbi@users.noreply.github.com>
Co-authored-by: robot-ci-heartex <robot-ci-heartex@users.noreply.github.com>
  • Loading branch information
4 people committed Jan 8, 2024
1 parent 010b5f2 commit 86ac80e
Show file tree
Hide file tree
Showing 24 changed files with 115 additions and 41 deletions.
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.

6 changes: 3 additions & 3 deletions label_studio/frontend/dist/lsf/version.json
@@ -1,6 +1,6 @@
{
"message": "fix: LEAP-248: Fix app crashing by hotkeys ",
"commit": "28c0e12c566794df3d749799276d2a0c60f28e2b",
"message": "fix: LEAP-344: View all annotation tab is not loading",
"commit": "eb1194c1e8ad5f9c0e725b2955dbc202f169649b",
"branch": "master",
"date": "2024-01-05T15:30:28Z"
"date": "2024-01-08T09:48:32Z"
}
2 changes: 1 addition & 1 deletion web/dist/libs/datamanager/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/datamanager/main.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions web/libs/editor/src/assets/icons/tools/eraser-tool.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion web/libs/editor/src/components/App/App.js
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
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
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
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
@@ -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
@@ -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
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
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
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
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
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
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
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
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
@@ -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
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
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
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

0 comments on commit 86ac80e

Please sign in to comment.