diff --git a/web-ui/javascript/src/App.js b/web-ui/javascript/src/App.js index a97a931bf..5dbb8bd8a 100644 --- a/web-ui/javascript/src/App.js +++ b/web-ui/javascript/src/App.js @@ -34,7 +34,9 @@ import { } from "./actions"; import {generateFilename} from "./utils/packs"; import { - LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN + LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN, + safeGetItem, + safeSetItem } from "./utils/storage"; import './App.css'; @@ -142,7 +144,7 @@ class App extends React.Component { if (nextProps.evergreen.announce !== this.props.evergreen.announce && nextProps.evergreen.announce !== null) { // Check last announce display time in local storage and compare to announce time let announceTime = Date.parse(nextProps.evergreen.announce.date); - let lastAnnounceShown = localStorage.getItem(LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN) || 0; + let lastAnnounceShown = safeGetItem(LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN) || 0; console.log('announce: ' + announceTime); console.log('last shown: ' + lastAnnounceShown); if (announceTime > lastAnnounceShown) { @@ -167,7 +169,7 @@ class App extends React.Component { }; dismissAnnounceDialog = () => { - localStorage.setItem(LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN, Date.now()); + safeSetItem(LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN, Date.now()); this.setState({announce: null}); }; diff --git a/web-ui/javascript/src/components/ErrorBoundary.js b/web-ui/javascript/src/components/ErrorBoundary.js new file mode 100644 index 000000000..65f3ade4c --- /dev/null +++ b/web-ui/javascript/src/components/ErrorBoundary.js @@ -0,0 +1,47 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +import React from 'react'; + +/* + * The last line of defence around the application: React 16 unmounts the whole tree when a render + * error goes uncaught, which the user sees as a blank page with no way back. This shows what + * happened and offers a reload. It deliberately depends on nothing — no i18n, no store — since + * either of those may be what failed; the two sentences are given in both languages instead. + */ +class ErrorBoundary extends React.Component { + + constructor(props) { + super(props); + this.state = { error: null }; + } + + static getDerivedStateFromError(error) { + return { error }; + } + + componentDidCatch(error, info) { + console.error('Unrecoverable render error', error, info && info.componentStack); + } + + render() { + if (this.state.error) { + const message = (this.state.error && this.state.error.message) || String(this.state.error); + return ( +
+

Something went wrong / Une erreur est survenue

+

STUdio hit an error it could not recover from. Reloading the page is safe: nothing on the device is affected by a display error.

+

STUdio a rencontré une erreur dont il ne peut pas se remettre. Recharger la page est sans risque : rien sur l'appareil n'est affecté par une erreur d'affichage.

+ 
{message}
+ +
+ ); + } + return this.props.children; + } +} + +export default ErrorBoundary; diff --git a/web-ui/javascript/src/components/ErrorBoundary.test.js b/web-ui/javascript/src/components/ErrorBoundary.test.js new file mode 100644 index 000000000..cb339efcc --- /dev/null +++ b/web-ui/javascript/src/components/ErrorBoundary.test.js @@ -0,0 +1,49 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +/* + * A render error anywhere used to unmount the whole tree: React 16 blanks the page when nothing + * catches. The boundary sits at the root and shows a message instead. It is rendered with + * react-dom into jsdom directly — the project has no component testing library and this does not + * add one. + */ + +import React from 'react'; +import ReactDOM from 'react-dom'; +import ErrorBoundary from './ErrorBoundary'; + +function Bomb() { + throw new Error('boom'); +} + +describe('ErrorBoundary', () => { + let container; + let consoleError; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + // React reports the caught error on console.error; that is expected here, not noise to fail on. + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + ReactDOM.unmountComponentAtNode(container); + container.remove(); + consoleError.mockRestore(); + }); + + it('renders its children when nothing throws', () => { + ReactDOM.render(

fine

, container); + expect(container.textContent).toBe('fine'); + }); + + it('shows a message instead of a blank page when a child throws', () => { + ReactDOM.render(, container); + expect(container.querySelector('.error-boundary')).not.toBeNull(); + expect(container.textContent).toContain('boom'); + }); +}); diff --git a/web-ui/javascript/src/components/PackLibrary.js b/web-ui/javascript/src/components/PackLibrary.js index 713111db6..c5dae09a8 100644 --- a/web-ui/javascript/src/components/PackLibrary.js +++ b/web-ui/javascript/src/components/PackLibrary.js @@ -26,9 +26,11 @@ import { import {AppContext} from "../AppContext"; import Modal from "./Modal"; import { - LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT + LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT, + safeGetItem } from "../utils/storage"; import {chooseDropAction, applyProvenanceVerdict} from "../utils/packs"; +import {parseJson} from "../utils/json"; import {verifyConversion} from "../services/library"; import './PackLibrary.css'; @@ -87,7 +89,11 @@ class PackLibrary extends React.Component { // Ignore missing node data return; } - var data = JSON.parse(packData); + var data = parseJson(packData); + if (!data) { + // A payload that is not JSON: not something this handler produced, so nothing to do + return; + } const driverFormat = this.state.device.metadata.driver; // Which artefact goes to the device is decided in utils/packs. It compares no timestamps and @@ -113,7 +119,7 @@ class PackLibrary extends React.Component { if (action === 'convert') { // No compatible pack: convert the source console.log('pack must be converted for driver: %s', driverFormat); // Ask for enriched raw format preference - if (driverFormat === 'raw' && localStorage.getItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === null) { + if (driverFormat === 'raw' && safeGetItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === null) { this.setState({ allowEnrichedDialog: { show: true, @@ -278,8 +284,11 @@ class PackLibrary extends React.Component { // Otherwise ignore missing node data / file return; } - var data = JSON.parse(packData); - + var data = parseJson(packData); + if (!data) { + return; + } + // Transfer pack and show progress this.props.addToLibrary(data.uuid, this.state.device.metadata.driver, this.context); }; @@ -309,7 +318,7 @@ class PackLibrary extends React.Component { onConvertLibraryPack = (pack, format) => { return () => { - if (format === 'raw' && localStorage.getItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === null) { + if (format === 'raw' && safeGetItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === null) { // Ask for enriched raw format preference this.setState({ allowEnrichedDialog: { diff --git a/web-ui/javascript/src/components/diagram/widgets/PackDiagramWidget.js b/web-ui/javascript/src/components/diagram/widgets/PackDiagramWidget.js index 276e8d536..e2e5c0675 100644 --- a/web-ui/javascript/src/components/diagram/widgets/PackDiagramWidget.js +++ b/web-ui/javascript/src/components/diagram/widgets/PackDiagramWidget.js @@ -23,6 +23,7 @@ import TrayItemWidget from "./TrayItemWidget"; import Modal from "../../Modal"; import {setEditorDiagram, setEditorFilename, setDiagramErrors} from "../../../actions"; import {generateFilename} from "../../../utils/packs"; +import {parseJson} from "../../../utils/json"; class PackDiagramWidget extends React.Component { @@ -146,7 +147,10 @@ class PackDiagramWidget extends React.Component { // Ignore missing node data return; } - var data = JSON.parse(nodeData); + var data = parseJson(nodeData); + if (!data) { + return; + } var node = null; switch (data.type) { case "stage": diff --git a/web-ui/javascript/src/index.js b/web-ui/javascript/src/index.js index 1792a0b7c..c3ddcc292 100644 --- a/web-ui/javascript/src/index.js +++ b/web-ui/javascript/src/index.js @@ -11,6 +11,7 @@ import { createStore, applyMiddleware } from 'redux'; import thunkMiddleware from 'redux-thunk'; import App from './App'; +import ErrorBoundary from './components/ErrorBoundary'; import rootReducer from './reducers'; import './i18n'; @@ -23,10 +24,12 @@ const store = createStore( ); ReactDOM.render( - - - - - , + + + + + + + , document.getElementById('root') ); diff --git a/web-ui/javascript/src/reducers/settings.js b/web-ui/javascript/src/reducers/settings.js index ef267b35c..835eeb529 100644 --- a/web-ui/javascript/src/reducers/settings.js +++ b/web-ui/javascript/src/reducers/settings.js @@ -6,22 +6,24 @@ import { LOCAL_STORAGE_ANNOUNCE_OPTOUT, - LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT + LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT, + safeGetItem, + safeSetItem } from "../utils/storage"; const initialState = { - announceOptOut: localStorage.getItem(LOCAL_STORAGE_ANNOUNCE_OPTOUT) === 'true' || false, - allowEnriched: localStorage.getItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === 'true' || false + announceOptOut: safeGetItem(LOCAL_STORAGE_ANNOUNCE_OPTOUT) === 'true', + allowEnriched: safeGetItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT) === 'true' }; const settings = (state = initialState, action) => { switch (action.type) { case 'SET_ANNOUNCE_OPTOUT': - localStorage.setItem(LOCAL_STORAGE_ANNOUNCE_OPTOUT, action.announceOptOut); + safeSetItem(LOCAL_STORAGE_ANNOUNCE_OPTOUT, action.announceOptOut); return { ...state, announceOptOut: action.announceOptOut }; case 'SET_ALLOW_ENRICHED': - localStorage.setItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT, action.allowEnriched); + safeSetItem(LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT, action.allowEnriched); return { ...state, allowEnriched: action.allowEnriched }; default: return state diff --git a/web-ui/javascript/src/services/library.js b/web-ui/javascript/src/services/library.js index 5f555935d..2136ca9e8 100644 --- a/web-ui/javascript/src/services/library.js +++ b/web-ui/javascript/src/services/library.js @@ -5,6 +5,7 @@ */ import {handleJsonOrError} from "../utils/fetch"; +import {parseJson} from "../utils/json"; export const fetchLibraryInfos = () => { return fetch('/api/library/infos') @@ -31,8 +32,13 @@ export const uploadToLibrary = async (uuid, path, packData, progressHandler) => xhr.upload.onprogress = progressHandler; } xhr.onload = () => { - console.log('xhr upload complete: ' + JSON.parse(xhr.responseText)); - resolve(JSON.parse(xhr.responseText)); + let response = parseJson(xhr.responseText); + if (response === undefined) { + reject(new Error('Upload failed: unexpected response from the server (' + xhr.status + ')')); + return; + } + console.log('xhr upload complete'); + resolve(response); }; xhr.open('post', '/api/library/upload', true); let formData = new FormData(); diff --git a/web-ui/javascript/src/utils/json.js b/web-ui/javascript/src/utils/json.js new file mode 100644 index 000000000..ad964c2e0 --- /dev/null +++ b/web-ui/javascript/src/utils/json.js @@ -0,0 +1,21 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +/* + * JSON.parse for input STUdio does not control: a drag-and-drop payload, an uploaded file, a + * response body. Anything that is not JSON answers undefined instead of throwing out of the + * handler that asked. + */ +export function parseJson(text) { + if (typeof text !== 'string' || text === '') { + return undefined; + } + try { + return JSON.parse(text); + } catch (e) { + return undefined; + } +} diff --git a/web-ui/javascript/src/utils/json.test.js b/web-ui/javascript/src/utils/json.test.js new file mode 100644 index 000000000..360e7f6a3 --- /dev/null +++ b/web-ui/javascript/src/utils/json.test.js @@ -0,0 +1,20 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +const { parseJson } = require('./json'); + +describe('parseJson', () => { + it('parses valid JSON', () => { + expect(parseJson('{"a":1}')).toEqual({ a: 1 }); + }); + + it('answers undefined for anything that is not JSON, instead of throwing', () => { + expect(parseJson('not json')).toBeUndefined(); + expect(parseJson('')).toBeUndefined(); + expect(parseJson(null)).toBeUndefined(); + expect(parseJson(undefined)).toBeUndefined(); + }); +}); diff --git a/web-ui/javascript/src/utils/reader.js b/web-ui/javascript/src/utils/reader.js index 7b9419530..5981d8495 100644 --- a/web-ui/javascript/src/utils/reader.js +++ b/web-ui/javascript/src/utils/reader.js @@ -13,6 +13,7 @@ import CoverNodeModel from "../components/diagram/models/CoverNodeModel"; import MenuNodeModel from "../components/diagram/models/MenuNodeModel"; import StoryNodeModel from "../components/diagram/models/StoryNodeModel"; import PackDiagramModel from "../components/diagram/models/PackDiagramModel"; +import {parseJson} from "./json"; export function readFromArchive(file) { @@ -20,9 +21,17 @@ export function readFromArchive(file) { var zip = new JSZip(); return zip.loadAsync(file) .then((archive) => { - // Read JSON story descriptor - return archive.file("story.json").async('string').then(storyJson => { - let json = JSON.parse(storyJson); + // Read JSON story descriptor. The archive is user input: say what is wrong with it + // rather than let a null or a SyntaxError escape from here. + let descriptor = archive.file("story.json"); + if (!descriptor) { + throw new Error("Not a story pack archive: no story.json inside"); + } + return descriptor.async('string').then(storyJson => { + let json = parseJson(storyJson); + if (!json) { + throw new Error("Not a story pack archive: story.json is not valid JSON"); + } var loadedModel = new PackDiagramModel(json.title, json.version, (json.description || ''), (json.nightModeAvailable || false)); diff --git a/web-ui/javascript/src/utils/reader.test.js b/web-ui/javascript/src/utils/reader.test.js new file mode 100644 index 000000000..67c40c3b2 --- /dev/null +++ b/web-ui/javascript/src/utils/reader.test.js @@ -0,0 +1,30 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +import JSZip from 'jszip'; +import { readFromArchive } from './reader'; + +/* + * An uploaded archive is user input. A story.json that is not JSON used to surface as a raw + * SyntaxError from deep inside the promise chain; it is now a rejection that names the problem. + */ +describe('readFromArchive', () => { + it('rejects a story.json that is not JSON with a message that says so', async () => { + const zip = new JSZip(); + zip.file('story.json', '{not json'); + const blob = await zip.generateAsync({ type: 'blob' }); + + await expect(readFromArchive(blob)).rejects.toThrow(/story\.json/); + }); + + it('rejects an archive without story.json rather than throwing on null', async () => { + const zip = new JSZip(); + zip.file('readme.txt', 'nothing here'); + const blob = await zip.generateAsync({ type: 'blob' }); + + await expect(readFromArchive(blob)).rejects.toThrow(/story\.json/); + }); +}); diff --git a/web-ui/javascript/src/utils/storage.js b/web-ui/javascript/src/utils/storage.js index 411cb161a..f0b435c98 100644 --- a/web-ui/javascript/src/utils/storage.js +++ b/web-ui/javascript/src/utils/storage.js @@ -7,3 +7,26 @@ export const LOCAL_STORAGE_ANNOUNCE_OPTOUT = 'announceOptOut'; export const LOCAL_STORAGE_ANNOUNCE_LAST_SHOWN = 'lastAnnounceShown'; export const LOCAL_STORAGE_ALLOW_ENRICHED_BINARY_FORMAT = 'allowEnrichedBinaryFormat'; + +/* + * localStorage is not always there: Safari in private browsing, some embedded browsers and a few + * privacy settings make every access throw — the accessor itself included. The settings reducer + * read it while building its initial state, so the store threw during creation and the application + * never rendered. An absent storage means "no stored value", and that is what these answer. + */ +export function safeGetItem(key) { + try { + return window.localStorage.getItem(key); + } catch (e) { + return null; + } +} + +export function safeSetItem(key, value) { + try { + window.localStorage.setItem(key, value); + return true; + } catch (e) { + return false; + } +} diff --git a/web-ui/javascript/src/utils/storage.test.js b/web-ui/javascript/src/utils/storage.test.js new file mode 100644 index 000000000..c7c5960b5 --- /dev/null +++ b/web-ui/javascript/src/utils/storage.test.js @@ -0,0 +1,46 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +/* + * localStorage is not always there. Safari in private browsing, some embedded browsers and a few + * privacy settings make every access throw, and the settings reducer read it while building its + * initial state — so the store threw during creation and the application never rendered. These + * helpers turn that into "no stored value", which is what an absent storage means. + */ + +const { safeGetItem, safeSetItem } = require('./storage'); + +describe('safeGetItem / safeSetItem', () => { + const realStorage = Object.getOwnPropertyDescriptor(window, 'localStorage'); + + afterEach(() => { + Object.defineProperty(window, 'localStorage', realStorage); + }); + + function replaceStorageWith(impl) { + Object.defineProperty(window, 'localStorage', { configurable: true, get: () => impl }); + } + + it('reads and writes through when storage works', () => { + safeSetItem('k', 'v'); + expect(safeGetItem('k')).toBe('v'); + }); + + it('answers null when reading throws', () => { + replaceStorageWith({ getItem: () => { throw new Error('SecurityError'); } }); + expect(safeGetItem('k')).toBeNull(); + }); + + it('answers null when the storage accessor itself throws', () => { + Object.defineProperty(window, 'localStorage', { configurable: true, get: () => { throw new Error('SecurityError'); } }); + expect(safeGetItem('k')).toBeNull(); + }); + + it('swallows a failing write and says so', () => { + replaceStorageWith({ setItem: () => { throw new Error('QuotaExceededError'); } }); + expect(safeSetItem('k', 'v')).toBe(false); + }); +});