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

REPL panics related to invalid unicode #7953

Closed
solson opened this issue Oct 13, 2020 · 4 comments
Closed

REPL panics related to invalid unicode #7953

solson opened this issue Oct 13, 2020 · 4 comments
Labels
bug Something isn't working correctly

Comments

@solson
Copy link
Contributor

solson commented Oct 13, 2020

I found a way to trip two different Rust panic messages in the REPL:

$ deno --version
deno 1.4.6
v8 8.7.220.3
typescript 4.0.3

$ deno
Deno 1.4.6
exit using ctrl+d or close()
> '\u{1f3b5}'
"🎵"
> '\u{1f3b5}'[0]
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("unexpected end of hex escape", line: 1, column: 60)', cli/inspector.rs:859:54
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
fish: “deno” terminated by signal SIGABRT (Abort)

$ deno
Deno 1.4.6
exit using ctrl+d or close()
> '\u{1f3b5}'[1]
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("lone leading surrogate in hex escape", line: 1, column: 59)', cli/inspector.rs:859:54
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
fish: “deno” terminated by signal SIGABRT (Abort)

Note that these panics only occur in the REPL, and regular Deno code is unaffected as far as I can tell.

cc @caspervonb

@caspervonb
Copy link
Contributor

caspervonb commented Oct 13, 2020

Thanks for the heads up 👍

For reference, It's from a StringBuffer unwrap in the inspector so it only happens for local inspector message responses that contains invalid unicode.

deno/cli/inspector.rs

Lines 854 to 868 in 1956cb8

fn send_response(
&mut self,
call_id: i32,
message: v8::UniquePtr<v8::inspector::StringBuffer>,
) {
let raw_message = message.unwrap().string().to_string();
let message = serde_json::from_str(&raw_message).unwrap();
self
.response_tx_map
.remove(&call_id)
.unwrap()
.send(message)
.unwrap();
}

@bartlomieju bartlomieju added the bug Something isn't working correctly label Oct 13, 2020
@benjamingr
Copy link
Contributor

Hey, I want to try and take a look at this as it's a relatively small fix that will make it learn some rust :]

@benjamingr
Copy link
Contributor

Checked this for a bit, I think this actually isn't actually a small fix at all to do well. The "invalid" JSON is coming from the CDP itself, Chrome's JSON parser and other parsers I've found happily parse this and it seems like an issue with serde_json itself.

I will however take the opportunity to brush up on rust error handling and match expressions and try to solve the issue ad-hoc :]

@bartlomieju
Copy link
Member

Fixed in #8759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants