Feature - Implemented Escape HTML in exception messages

[philmasterplus]

With JavaScript, we can now throw exception objects with arbitrary messages. If these error messages contain text that can be interpreted as HTML, the error message shown in the gCLI is rendered as such. This makes it hard to debug JavaScript errors.

For example, suppose the following code is executed:

throw new Error('<span><span> is invalid');

Currently (r20608) this will be printed as:

JavaScript exception: Error: is invalid (file:/C:/Users/Phil/Documents/KoL/scripts/test.js#1)
at file:/C:/Users/Phil/Documents/KoL/scripts/test.js:1

From the stack trace alone, it is impossible to distinguish a <span> tag from an empty string.

This problem can be exacerbated when the error message contains large HTML markup, e.g. tables. KoLmafia will try to render such messages in the gCLI instead of just printing their output.

To fix this, I suggest making KoLmafia.updateDisplay() escape HTML in messages before it passes them to RequestLogger.printLine(). This is preferable since we only want to escape messages sent to the gCLI. If we escaped the string before passing it it to updateDisplay(), it would also affect messages going to other outlets, such as system tray messages (do we still use those, BTW?). As a bonus, we can now safely use characters like "<" when directly calling KoLmafia.updateDisplay().

Here's a patch that applies the suggested change.

 

[fronobulax]

Rendering HTML sent to the gCLI is a feature used by scriptwriters. Without trying it, it sounds like this patch will break that and I would not want to do that.

Since this is a new problem introduced by JS could it be handled in new code that only deals with JS? Can a stack trace detect it came from JS and escape accordingly? I seem to recall that stack traces go to the session log, at least optionally? Does this go away by inconveniencing JS writers by asking them to look at the session log if the gCLI doesn't make sense?

 

[philmasterplus]

Rendering HTML sent to the gCLI is a feature used by scriptwriters. Without trying it, it sounds like this patch will break that and I would not want to do that.

Since this is a new problem introduced by JS could it be handled in new code that only deals with JS? Can a stack trace detect it came from JS and escape accordingly? I seem to recall that stack traces go to the session log, at least optionally? Does this go away by inconveniencing JS writers by asking them to look at the session log if the gCLI doesn't make sense?

My suggestion doesn't touch print_html(). Script writers can still render HTML in the gCLI when they want to.

This handles the case when they don't want to render HTML--when KoLmafia prints a stack trace.

That said, it is possible to escape HTML before the message is passed to updateDisplay()--which feeds the message to 3 places:

1. The gCLI
2. SystemTrayFrame
3. Status messages shown in various frames

Case 2 is extremely niche and I'm not sure if the functionality is working to begin with. No need to worry about case 3, since it ignores all multiline messages--and stack traces are almost always multiline. That only leaves case 1...so I suppose it's okay if the JS runtime escaped HTML before calling updateDisplay(). Your suggestion makes sense.

Good point on the session logs. I'll have to double check how my proposal (and yours) affects them.

Edit: Nope. Session logs don't store exception stack traces, at least for JavaScript. The debug logs (i.e. DEBUG_201210111.txt) don't help, either; they contain Java stack traces, but not JavaScript. So JavaScript writers are completely dependent on the gCLI printing correct information.

 

[philmasterplus]

Here's an alternative patch that doesn't touch KoLmafia.updateDisplay(). Instead, the JavascriptRuntime class escapes "<" and "&" in for exception messages before passing them to updateDisplay().

 

[fronobulax]

My suggestion doesn't touch print_html(). Script writers can still render HTML in the gCLI when they want to.

But before there was print_html() there was print("<blah>Something</blah>").

There may be a vocabulary issue. What I call the stack trace from Ash definitely prints to both the gCLI and the session log (<char>_<date>.txt).

But when I triggered a stack trace by abort("string") in ash, I saw string in red in the gCLI but not echoed to the session log so there is something that I would hope ended up in both places that doesn't. Some of teh stack trace work only goes back to April 2020 and has my fingerprints on it so I need to fix a bug or understand what is going on.

I'll look at your second patch after I figure out the above.

 

[fronobulax]

Patch applied. I'm remembering why I didn't get the message that triggered an error in the session log before.