-
Notifications
You must be signed in to change notification settings - Fork 458
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
[EXTERNAL] Improved checking for import, moved removeComments #1765
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,17 +149,27 @@ ${tests.trim()};`.trim() | |
} | ||
|
||
const loadAndSanitizeSolution = async () => { | ||
const path = `${solutionPath}/${name}.js` | ||
const rawCode = await read(path, "student solution") | ||
|
||
// this is a very crude and basic removal of comments | ||
// since checking code is only use to prevent cheating | ||
// it's not that important if it doesn't work 100% of the time. | ||
const code = rawCode.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, "").trim() | ||
if (code.includes("import")) fatal("import keyword not allowed") | ||
return { code, rawCode, path } | ||
try { | ||
const path = `${solutionPath}/${name}.js` | ||
const rawCode = await read(path, "student solution") | ||
const sanitizedCode = removeComments(rawCode) | ||
|
||
if (sanitizedCode.includes("import ")) { // space is important as it prevents "imported" or "importance" or other words containing "import" | ||
throw new Error("The use of the 'import' keyword is not allowed.") | ||
} | ||
return { code: sanitizedCode, rawCode, path } | ||
} catch (error) { | ||
console.error(error) | ||
} | ||
} | ||
|
||
const removeComments = (code) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the maintainer may replace this as const removeComments = (code, lang='JavaScript') => |
||
// removes JS single line and multi-line comments only. Not for bash files etc. | ||
// for use with multiple file-types, I suggest writing a removeComments function with language-type as input and then handling accordingly | ||
return code.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, "").trim() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a condition could be added, something like if (lang === 'JavaScript') || (lang === 'JS') { |
||
} | ||
|
||
|
||
const runTests = async ({ url, path, code }) => { | ||
const { setup, tests } = await import(url).catch(err => | ||
fatal(`Unable to execute ${name}, error:\n${stackFmt(err, url)}`), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the maintainer may replace the console.log(error) with fatal(error)
as fatal does precisely that plus whatever is required to be done on top of it