From f5feff386a00c171920f5fcf08ffaec79f081adf Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Wed, 27 Nov 2019 02:51:43 +0100 Subject: [PATCH] fix: stop user code after 500ms of execution Code like `var xs = []; while(true){ xs.push(1) }` can quickly run the browser out of memory causing it to crash. These changes stop user loops from running indefinitely so that common mistakes will no longer cause the browser to crash. Also, the user is informed if a long running loop is detected (js and jsx challenges) during preview or testing. Before this there was no protection for js challenges and no information was given to the user if they had created such a loop. Co-Authored-By: Tom <20648924+moT01@users.noreply.github.com> Co-Authored-By: mrugesh <1884376+raisedadead@users.noreply.github.com> Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com> --- client/package-lock.json | 10 +-- client/package.json | 2 +- .../Challenges/rechallenge/transformers.js | 83 +++++++++++++------ .../redux/execute-challenge-saga.js | 15 +++- .../src/templates/Challenges/utils/build.js | 12 +-- 5 files changed, 83 insertions(+), 39 deletions(-) diff --git a/client/package-lock.json b/client/package-lock.json index 82e62f55b78..950d9991bf1 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -1236,6 +1236,11 @@ "prop-types": "^15.5.10" } }, + "@freecodecamp/loop-protect": { + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/@freecodecamp/loop-protect/-/loop-protect-2.2.1.tgz", + "integrity": "sha512-px2gy/jHfMyTFOAY+c5IiNuBJCP+B0vC20SGdaS0YgnCJov82bewHDqE9a2fci4XYTxjxyJpuTKZMelLxDcyJg==" + }, "@freecodecamp/react-bootstrap": { "version": "0.32.3", "resolved": "https://registry.npmjs.org/@freecodecamp/react-bootstrap/-/react-bootstrap-0.32.3.tgz", @@ -15513,11 +15518,6 @@ "resolved": "https://registry.npmjs.org/longest-streak/-/longest-streak-2.0.3.tgz", "integrity": "sha512-9lz5IVdpwsKLMzQi0MQ+oD9EA0mIGcWYP7jXMTZVXP8D42PwuAk+M/HBFYQoxt1G5OR8m7aSIgb1UymfWGBWEw==" }, - "loop-protect": { - "version": "2.1.6", - "resolved": "https://registry.npmjs.org/loop-protect/-/loop-protect-2.1.6.tgz", - "integrity": "sha512-eGNk917T5jQ9A/ER/zJlEXCGD/NQepYyLnLBgVPSuspHauG2HUiDx5oKDSpyVQOzGb+yUKMA1k41+Old2ZmcRQ==" - }, "loose-envify": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.3.1.tgz", diff --git a/client/package.json b/client/package.json index e08641d2e94..ccfe9790040 100644 --- a/client/package.json +++ b/client/package.json @@ -14,6 +14,7 @@ "@fortawesome/free-regular-svg-icons": "^5.11.2", "@fortawesome/free-solid-svg-icons": "^5.11.2", "@fortawesome/react-fontawesome": "^0.1.4", + "@freecodecamp/loop-protect": "^2.2.1", "@freecodecamp/react-bootstrap": "^0.32.3", "@reach/router": "^1.2.1", "algoliasearch": "^3.35.1", @@ -40,7 +41,6 @@ "gatsby-transformer-remark": "^2.6.30", "jquery": "^3.4.1", "lodash": "^4.17.15", - "loop-protect": "^2.1.6", "monaco-editor": "^0.18.1", "monaco-editor-webpack-plugin": "^1.7.0", "nanoid": "^1.2.2", diff --git a/client/src/templates/Challenges/rechallenge/transformers.js b/client/src/templates/Challenges/rechallenge/transformers.js index bb80a89ae0c..a47a1e4f0d5 100644 --- a/client/src/templates/Challenges/rechallenge/transformers.js +++ b/client/src/templates/Challenges/rechallenge/transformers.js @@ -13,7 +13,7 @@ import { import * as Babel from '@babel/standalone'; import presetEnv from '@babel/preset-env'; import presetReact from '@babel/preset-react'; -import protect from 'loop-protect'; +import protect from '@freecodecamp/loop-protect'; import * as vinyl from '../utils/polyvinyl.js'; import createWorker from '../utils/worker-executor'; @@ -23,7 +23,26 @@ import createWorker from '../utils/worker-executor'; import { filename as sassCompile } from '../../../../config/sass-compile'; const protectTimeout = 100; -Babel.registerPlugin('loopProtection', protect(protectTimeout)); +const testProtectTimeout = 1500; +const loopsPerTimeoutCheck = 2000; + +function loopProtectCB(line) { + console.log( + `Potential infinite loop detected on line ${line}. Tests may fail if this is not changed.` + ); +} + +function testLoopProtectCB(line) { + console.log( + `Potential infinite loop detected on line ${line}. Tests may be failing because of this.` + ); +} + +Babel.registerPlugin('loopProtection', protect(protectTimeout, loopProtectCB)); +Babel.registerPlugin( + 'testLoopProtection', + protect(testProtectTimeout, testLoopProtectCB, loopsPerTimeoutCheck) +); const babelOptionsJSX = { plugins: ['loopProtection'], @@ -31,9 +50,15 @@ const babelOptionsJSX = { }; const babelOptionsJS = { + plugins: ['testLoopProtection'], presets: [presetEnv] }; +const babelOptionsJSPreview = { + ...babelOptionsJS, + plugins: ['loopProtection'] +}; + const babelTransformCode = options => code => Babel.transform(code, options).code; @@ -69,28 +94,31 @@ function tryTransform(wrap = identity) { }; } -export const babelTransformer = cond([ - [ - testJS, - flow( - partial( - vinyl.transformHeadTailAndContents, - tryTransform(babelTransformCode(babelOptionsJS)) +const babelTransformer = (preview = false) => + cond([ + [ + testJS, + flow( + partial( + vinyl.transformHeadTailAndContents, + tryTransform( + babelTransformCode(preview ? babelOptionsJSPreview : babelOptionsJS) + ) + ) ) - ) - ], - [ - testJSX, - flow( - partial( - vinyl.transformHeadTailAndContents, - tryTransform(babelTransformCode(babelOptionsJSX)) - ), - partial(vinyl.setExt, 'js') - ) - ], - [stubTrue, identity] -]); + ], + [ + testJSX, + flow( + partial( + vinyl.transformHeadTailAndContents, + tryTransform(babelTransformCode(babelOptionsJSX)) + ), + partial(vinyl.setExt, 'js') + ) + ], + [stubTrue, identity] + ]); const sassWorker = createWorker(sassCompile); async function transformSASS(element) { @@ -141,7 +169,14 @@ export const htmlTransformer = cond([ export const transformers = [ replaceNBSP, - babelTransformer, + babelTransformer(), + composeHTML, + htmlTransformer +]; + +export const transformersPreview = [ + replaceNBSP, + babelTransformer(true), composeHTML, htmlTransformer ]; diff --git a/client/src/templates/Challenges/redux/execute-challenge-saga.js b/client/src/templates/Challenges/redux/execute-challenge-saga.js index 477cf0dbf5a..54970ee7263 100644 --- a/client/src/templates/Challenges/redux/execute-challenge-saga.js +++ b/client/src/templates/Challenges/redux/execute-challenge-saga.js @@ -33,6 +33,9 @@ import { isJavaScriptChallenge } from '../utils/build'; +// How long before bailing out of a preview. +const previewTimeout = 2500; + export function* executeChallengeSaga() { const isBuildEnabled = yield select(isBuildEnabledSelector); if (!isBuildEnabled) { @@ -90,9 +93,9 @@ function* takeEveryConsole(channel) { }); } -function* buildChallengeData(challengeData) { +function* buildChallengeData(challengeData, preview) { try { - return yield call(buildChallenge, challengeData); + return yield call(buildChallenge, challengeData, preview); } catch (e) { yield put(disableBuildOnError()); throw e; @@ -155,7 +158,7 @@ function* previewChallengeSaga() { const challengeData = yield select(challengeDataSelector); if (canBuildChallenge(challengeData)) { - const buildData = yield buildChallengeData(challengeData); + const buildData = yield buildChallengeData(challengeData, true); // evaluate the user code in the preview frame or in the worker if (challengeHasPreview(challengeData)) { const document = yield getContext('document'); @@ -163,10 +166,14 @@ function* previewChallengeSaga() { } else if (isJavaScriptChallenge(challengeData)) { const runUserCode = getTestRunner(buildData, { proxyLogger }); // without a testString the testRunner just evaluates the user's code - yield call(runUserCode, null, 5000); + yield call(runUserCode, null, previewTimeout); } } } catch (err) { + if (err === 'timeout') { + // eslint-disable-next-line no-ex-assign + err = `The code you have written is taking longer than the ${previewTimeout}ms our challenges allow. You may have created an infinite loop or need to write a more efficient algorithm`; + } console.log(err); yield put(updateConsole(escape(err))); } diff --git a/client/src/templates/Challenges/utils/build.js b/client/src/templates/Challenges/utils/build.js index e38188c7e5a..0121cd957fe 100644 --- a/client/src/templates/Challenges/utils/build.js +++ b/client/src/templates/Challenges/utils/build.js @@ -1,4 +1,4 @@ -import { transformers } from '../rechallenge/transformers'; +import { transformers, transformersPreview } from '../rechallenge/transformers'; import { cssToHtml, jsToHtml, concatHtml } from '../rechallenge/builders.js'; import { challengeTypes } from '../../../../utils/challengeTypes'; import createWorker from './worker-executor'; @@ -76,11 +76,11 @@ export function canBuildChallenge(challengeData) { return buildFunctions.hasOwnProperty(challengeType); } -export async function buildChallenge(challengeData) { +export async function buildChallenge(challengeData, preview = false) { const { challengeType } = challengeData; let build = buildFunctions[challengeType]; if (build) { - return build(challengeData); + return build(challengeData, preview); } throw new Error(`Cannot build challenge of type ${challengeType}`); } @@ -137,8 +137,10 @@ export function buildDOMChallenge({ files, required = [], template = '' }) { })); } -export function buildJSChallenge({ files }) { - const pipeLine = composeFunctions(...transformers); +export function buildJSChallenge({ files }, preview = false) { + const pipeLine = preview + ? composeFunctions(...transformersPreview) + : composeFunctions(...transformers); const finalFiles = Object.keys(files) .map(key => files[key]) .map(pipeLine);