Exception flag or similar to prevent exposure of backtrace

[redbullmarky]

When using FLAG_PROPAGATE_PHP_EXCEPTIONS (which is the only way afaik to catch PHP exceptions in the JS) and catching an error thrown by PHP in your JS, you have full access to all of PHP's exception methods allowing you to view a full trace. So if you're running a sort of sandbox environment, it's probably exposing more than you'd like. Code:

<?php
class myv8 extends V8Js
{
	public function throwException(string $message) {
		throw new Exception($message);
	}
}

$v8 = new myv8();
$v8->executeString('
	try {
		PHP.throwException("Oops");
	}
	catch (e) {
		var_dump(e.getTrace());
	}

	print("done");
', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);

Unfortunately, it looks like PHP's Exception class methods are mostly final too so cannot be overridden to prevent access. The only way we've found is to (in boilerplate JS executed before custom JS) essentially wrap all functions available on the PHP object and wrapping them, with the addition of a try/catch that rethrows the string of an exception rather than the full thing. A bit like:

// inaccessible boilerplate JS that runs prior to usercode
let originalThrowException = PHP.throwException;
PHP.throwException = () => {
	try {
		return originalThrowException.call(PHP, ...arguments);
	}
	catch (e) {
		throw e.getMessage();
	}
}

So I think we need some way of changing this behaviour. Maybe an extra FLAG that gets the text from an exception and throws that in place of the PHP exception instance.

Thoughts?

[stesie]

Hej Mark,

first of all, I don't think this qualifies as a bug. After all it works as designed :-)
Default behaviour is to not expose exceptions, just silently abort execution. Years later the opt-in behaviour of exposing exceptions as-is was introduced. But I see your need, so ...

Providing (yet) another flag that just throws the result of getMessage feels a bit too specific to me. After all there might be reasons to have the exception type/class accessible or some kind of getCode, getKey etc. Contrary simply forbidding access to getTrace and getTraceAsString might be too little, as some might feel bad about getFile or getLine as well.

Therefore having some sort of proxy object would be what I'd strive for, that's automatically created by the extension.

Something like this:

<?php
class myv8 extends V8Js
{
	public function throwException(string $message) {
		throw new Exception($message);
	}
}

class ExceptionProxy {
	private $ex;

	public function __construct(Throwable $ex) {
		$this->ex = $ex;
	}

	public function getMessage() {
		return $this->ex->getMessage();
	}
}

$v8 = new myv8();
$v8->registerExceptionProxy(ExceptionProxy::class);

$v8->executeString('
	try {
		PHP.throwException("Oops");
	}
	catch (e) {
		var_dump(e.getMessage()); // calls ExceptionProxy::getMessage
		var_dump(e.getTrace()); // fails
	}
', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);

Then it just has to be decided what should happen if __construct throws an exception. That could mean that the exception shouldn't be exposed to JS. But that feels a bit sketchy.

[redbullmarky]

The original title of this issue was 'Custom exceptions' where I thought adding an alternative would be better, I guess I just thought that if there's code in V8Js somewhere relying on checking that an object implements a PHP base exception, it'd no longer work. So I thought perhaps a flag such as FLAG_PROPAGATE_PHP_EXCEPTION_STRING (terrible name but you get the gist) might be an easy solve.

But to be honest, I could work with either approach - the most important thing is just not exposing internals to JS-land, but still allowing the user to catch errors that may have been thrown within one of a whole suite of provided functions.

[stesie]

Closed via #487

[redbullmarky]

@stesie this one will need to go into php8 branch too, right?