Panic in Rtc:accepts() caused by ICE code

[OxleyS]

Under certain circumstances which I am not totally sure of, the Rtc::accepts() function panics:

thread '<unnamed>' panicked at /Users/sean/.cargo/registry/src/index.crates.io-6f17d22bba15001f/str0m-0.4.1/src/ice/agent.rs:324:18:
Remote ICE credentials
stack backtrace:
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:196:5
   3: core::panicking::panic_str
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:171:5
   4: core::option::expect_failed
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/option.rs:1980:5
   5: core::option::Option<T>::expect
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/option.rs:894:21
   6: str0m::ice::agent::IceAgent::stun_credentials
             at /Users/sean/.cargo/registry/src/index.crates.io-6f17d22bba15001f/str0m-0.4.1/src/ice/agent.rs:321:24
   7: str0m::ice::agent::IceAgent::accepts_message
             at /Users/sean/.cargo/registry/src/index.crates.io-6f17d22bba15001f/str0m-0.4.1/src/ice/agent.rs:778:29
   8: str0m::Rtc::accepts
             at /Users/sean/.cargo/registry/src/index.crates.io-6f17d22bba15001f/str0m-0.4.1/src/lib.rs:1495:20
<...my project's stack frames omitted...>

The offender seems to be the stun_credentials() call, which is documented to panic on missing remote credentials, but this is not checked for before calling.

Unfortunately, I don't think I can provide repro code for you anytime soon, as I was in the process of smashing out a proof-of-concept and the current code is in no position to be reduced.

[algesten]

Hi @OxleyS, welcome to str0m!

How did you set up this proof of concept? SDP or the lower level APIs? The error message is maybe too opaque, but it's a config fault to not have the remote ice credentials set when starting to accept STUN traffic. For SDP this should be impossible, but for the direct API it probably is possible to misconfigure.

[OxleyS]

Hi algesten, my proof-of-concept is hacking on the chat example from this repo, swapping out the web interface + data channels with a Websocket-based thingy to do negotiation and ICE candidate trickle. I'm using sdp_api() when setting the remote offer, and never call direct_api(). I have confirmed that accept_offer() isn't returning Err or anything.

I do agree with your assessment that it would seem impossible to run into this case, as remote credentials should be set once the remote offer is processed, correct? I'll keep digging on my end and see what I can find.

[OxleyS]

I think I understand the bug now. My code uses the same demultiplexing thing that the chat example uses. However, unlike the example, I add clients (and their corresponding Rtc instances) to the clients list before I've received an initial offer.

The STUN traffic may be intended for clients[1], but if clients[0] hasn't had a remote offer set yet, when we call clients[0].accepts() it panics because there's no remote creds. It should probably just return false in this case.

[algesten]

Ahhh. Good find! Yeah, I agree. If this was direct api, it would be another matter, but you found a way to trigger this with SDP API only.

Returning false seems like the simplest solve. PR welcome or I fix later.

Thanks!