Skip to content

Commit

Permalink
C++: Fix DataProducer::Send invalid memory access (#1190)
Browse files Browse the repository at this point in the history
'subchannels' are optional and hence they need to be checked before
accessing them.

Remove test expectation that assumed we were receiving all messages in
incremental mode. TS tests failure was being masked by [1], I don't know
why Rust wasn't failing but definitely consumers are not receiving
everything sent by the producer since in the tests both are paused at
some time.

[1]: #1188
  • Loading branch information
jmillan authored Oct 20, 2023
1 parent b706ae7 commit c889e1a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
2 changes: 1 addition & 1 deletion node/src/tests/test-DirectTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ test('dataProducer.send() succeeds', async () =>
expect(ppid).toBe(53); // PPID of WebRTC DataChannel binary.
}

expect(id).toBe(++lastRecvMessageId);
++lastRecvMessageId;
});
});

Expand Down
2 changes: 0 additions & 2 deletions rust/tests/integration/direct_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ fn get_stats_succeeds() {
}

#[test]
#[ignore]
fn send_succeeds() {
future::block_on(async move {
let (_worker, _router, transport) = init().await;
Expand Down Expand Up @@ -226,7 +225,6 @@ fn send_succeeds() {
}

last_recv_message_id.fetch_add(1, Ordering::SeqCst);
assert_eq!(id, last_recv_message_id.load(Ordering::SeqCst));

if id == num_messages {
let _ = received_messages_tx.lock().take().unwrap().send(());
Expand Down
11 changes: 7 additions & 4 deletions worker/src/RTC/DataProducer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,14 @@ namespace RTC

std::vector<uint16_t> subchannels;

subchannels.reserve(body->subchannels()->size());

for (const auto subchannel : *body->subchannels())
if (flatbuffers::IsFieldPresent(body, FBS::DataProducer::SendNotification::VT_SUBCHANNELS))
{
subchannels.emplace_back(subchannel);
subchannels.reserve(body->subchannels()->size());

for (const auto subchannel : *body->subchannels())
{
subchannels.emplace_back(subchannel);
}
}

std::optional<uint16_t> requiredSubchannel{ std::nullopt };
Expand Down

0 comments on commit c889e1a

Please sign in to comment.