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

Runtime error in script run via eval crashes the app #127

Closed
scripting opened this issue Aug 25, 2019 · 4 comments

Comments

@scripting
Copy link
Owner

commented Aug 25, 2019

PagePark allows pages to be defined by script, the way it works is if a file has a .js extension it runs it through an eval inside a try.

The idea is that if your script has an error, we can catch it so it doesn't crash PagePark.

But it doesn't get caught and PagePark crashes.

Not sure what script it's running, I'm adding some debugging code to list that to the console, but in the meantime I wonder if there is a legit way in JS to catch these errors. I'd like to log them, and of course PP should keep running.

PS: Please save the lectures about security. I have decades of experience with this. Thanks. :-)

@danmactough

This comment has been minimized.

Copy link

commented Aug 25, 2019

But it doesn't get caught and PagePark crashes.

If the JavaScript getting eval'd has any asynchronous code, try/catch won't catch that.

Example:

> // Non-async code
undefined
> s = "throw new Error('eval me')"
'throw new Error(\'eval me\')'
> try { eval(s) } catch (e) { console.error(`An error was caught: ${e.message}`) }
An error was caught: eval me
undefined
> // Async code
undefined
> s = "process.nextTick(() => { throw new Error('eval me') } )"
'process.nextTick(() => { throw new Error(\'eval me\') } )'
> try { eval(s) } catch (e) { console.error(`An error was caught: ${e.message}`) }
undefined
> Error: eval me
    at process.nextTick (eval at <anonymous> (repl:1:7), <anonymous>:1:32)
    at _combinedTickCallback (internal/process/next_tick.js:132:7)
    at process._tickDomainCallback (internal/process/next_tick.js:219:9)

I'm confused to hear that PagePark crashes, though.

@scripting

This comment has been minimized.

Copy link
Owner Author

commented Aug 25, 2019

Dan that makes perfect sense. I should have included the stack crawl when PP crashes to give a clue. I added some debugging code and trapped the problem. It's a missing callback on a fs.writeFile call in the script that was being eval'd. Here's the actual script.

if (urlFeed !== undefined) {
	var now = new Date (), fname = "domains/ping.1999.io/pingers.json";
	console.log ("ping.1999.io: urlFeed == " + urlFeed);
	fs.readFile (fname, function (err, data) {
		var jstruct = new Object ();
		if (!err) {
			jstruct = JSON.parse (data.toString ());
			}
		if (jstruct [urlFeed] === undefined) {
			jstruct [urlFeed] = {
				ct: 1,
				when: now
				}
			}
		else {
			jstruct [urlFeed].ct++;
			jstruct [urlFeed].when = now
			}
		fs.writeFile (fname, JSON.stringify (jstruct, undefined, 4));
		});
	}
"Thanks for the ping!";

@scripting scripting closed this Aug 26, 2019

@scripting

This comment has been minimized.

Copy link
Owner Author

commented Aug 26, 2019

BTW, normally a problem like this would have been found when the ping-handler script was written. But the requirement that the callback be defined when calling fs.writeFile was introduced in a version of Node that came after it was written, deployed and forgotten.

@danmactough

This comment has been minimized.

Copy link

commented Aug 26, 2019

But the requirement that the callback be defined when calling fs.writeFile was introduced in a version of Node that came after it was written, deployed and forgotten.

oh-ffs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.