sharkdp / numbat

A statically typed programming language for scientific computations with first class support for physical dimensions and units
https://numbat.dev
Apache License 2.0
1.26k stars 53 forks source link

JSON parse error in wasm version causes very long system error to be printed #533

Closed Mads-MMJ closed 1 month ago

Mads-MMJ commented 3 months ago

Description I was playing around with making a generic numbat function for creating list representations of mixed units after reading #364 and #501. When I tried to test it on the Angle Dimension I tried to use the minute and second symbols but used the wrong characters by accident. This however caused a parsing error which took several seconds (roughly 10) to print while read text filled the screen. It looks a lot like an unhandled exception that rose all the way to the user by accident. For the sake of reproducibility, I have included the error and my implementation of the function that caused it. I can however not include the full error due to the GitHub character limit. I do however have the full error and full session history saved and can provide it if needed.

Error

>>> unit_list(12 degrees + 34 minute + 56 second, [1 degree, 1´, 1´´])
error: while parsing
  ┌─ <input:106>:1:59
  │
1 │ unit_list(12 degrees + 34 minute + 56 second, [1 degree, 1´, 1´´])
  │                                                           ^ Unexpected character
: '´'

[USER]: https://numbat.dev/jquery.terminal.min.js: JSON.parse: unexpected
non-whitespace character after JSON data at line 1 column 4 of the JSON data
e/r/<@https://numbat.dev/jquery.terminal.min.js:46:94723
r@https://numbat.dev/jquery.terminal.min.js:46:94472
e@https://numbat.dev/jquery.terminal.min.js:46:95069
e/<@https://numbat.dev/jquery.terminal.min.js:46:95793
process_command/u<@https://numbat.dev/jquery.terminal.min.js:46:79079
map@https://numbat.dev/jquery.min.js:2:3967
process_command@https://numbat.dev/jquery.terminal.min.js:46:78933
e@https://numbat.dev/jquery.terminal.min.js:46:95745
me@https://numbat.dev/jquery.terminal.min.js:46:124374
ENTER@https://numbat.dev/jquery.terminal.min.js:46:45308
it@https://numbat.dev/jquery.terminal.min.js:46:64967
dispatch@https://numbat.dev/jquery.min.js:2:39997
add/v.handle@https://numbat.dev/jquery.min.js:2:37968

Implementation of unit_list

fn unit_trunc<A: Dim>(val: A, u: A) -> A = trunc(val ➞ unit_of(u))

fn unit_list_impl<A: Dim>(val: A, units: List<A>, acc: List<A>) -> List<A> =
    if is_empty(units)
        then reverse(acc)
    else if len(units) == 1
        then reverse(cons(val -> head(units), acc))
    else 
        unit_list_impl(val - unit_val, tail(units), cons(unit_val, acc))
    where unit_val: A = unit_trunc(val, head(units))

fn unit_list<A: Dim>(val: A, units: List<A>) -> List<A> = unit_list_impl(val, units, [])
sharkdp commented 3 months ago

Thank you for reporting this. I can not reproduce this in Firefox. I copied the unit_list implementation and then your unit_list(12 degrees + 34 minute + 56 second, [1 degree, 1´, 1´´]) query to arrive at this, and I see the following output:

image

And no JavaScript errors are appearing. I also tested it in the CLI version and everything works as expected.

Interestingly though, I experienced a potentially related error today. I did something like this …

image

… in a slightly different environment, and only saw List in the output, instead of List<Length>. Turns out that HTML symbols are not properly escaped in error messages (it works fine with actual values). The problem is this function:

https://github.com/sharkdp/numbat/blob/2ead484c7f9748903d9253742fed2b79a6ee81b3/numbat/src/html_formatter.rs#L72

which needs to call html_escape::encode_text on the incoming text.

Maybe something similar happened in your case?! Or it is something else entirely. But then I need some help with reproducing this.

The escaping problem should definitely be fixed though.

sharkdp commented 3 months ago

I was playing around with making a generic numbat function for creating list representations of mixed units after reading #364 and #501.

:star_struck:

sharkdp commented 2 months ago

@Mads-MMJ any feedback on this?

sharkdp commented 1 month ago

I'm closing this due to inactivity. Please feel free to comment in case it should be re-opened.