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

Logging strings with surrogate code points crashes deno #12226

Closed
rotu opened this issue Sep 26, 2021 · 5 comments
Closed

Logging strings with surrogate code points crashes deno #12226

rotu opened this issue Sep 26, 2021 · 5 comments
Assignees
Labels
bug Something isn't working correctly

Comments

@rotu
Copy link
Contributor

rotu commented Sep 26, 2021

Logging a string with an unpaired surrogate unit (console.log("\uDE00")) causes Deno to crash. Similarly, when an object contains such a string, logging it also causes a crash (console.log({x:"\uDE00"}))

Note that alert and Deno.core.print do not crash in this situation

RUST_BACKTRACE=1 deno 
Deno 1.14.1
exit using ctrl+d or close()
> console.log("\uDE00")
�
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("lone leading surrogate in hex escape", line: 1, column: 100)', /Users/ry/src/deno/core/inspector.rs:718:52
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::unwrap
   4: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   7: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   8: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   9: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  10: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  11: deno::tools::repl::run::{{closure}}
  12: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  13: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
  14: deno::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@rotu
Copy link
Contributor Author

rotu commented Sep 26, 2021

Possibly related is that tab-complete does not work for such binary strings.

e.g. if I have const a="a"; const b="\uDE00", then tab completing a.ind tab autocompletes to a.indexOf but b.ind does not.

@rotu rotu changed the title Logging binary strings crashes deno Logging strings with surrogate code points crashes deno Sep 26, 2021
@littledivy
Copy link
Member

Related serde_json issue: serde-rs/json#495

@bartlomieju bartlomieju added bug Something isn't working correctly upstream Changes in upstream are required to solve these issues labels Sep 26, 2021
@lucacasonato lucacasonato self-assigned this Nov 22, 2021
@lucacasonato
Copy link
Member

My tenative plan to fix this:

  1. Allow lone surrogates to be deserialized into ByteBufs by serde (Deserialize lone surrogates into byte bufs serde-rs/json#828)
  2. Change our inspector implementation to deserialize into a serde_json::value::RawValue rather than a serde_json::Value
  3. Add a LossyString type which has a serde::Deserialize implementation that replaces lone surrogate pairs with replacement characters using APIs from encoding_rs
  4. Use LossyString to deserialize inspector messages for the REPL

Let's see how it goes.

@lucacasonato
Copy link
Member

All upstream changes have landed in serde and are released now. This is blocked on me putting in the work to hook this all up correctly now.

@marvinhagemeister
Copy link
Contributor

Tried to reproduce this with Deno 1.42.4 and I'm unable to. This is issue seems to be resolved 🎉

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
5 participants