versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.18k stars 1.12k forks source link

Avoid modification of user input data #1285

Closed ibc closed 9 months ago

ibc commented 9 months ago

Fixes #1277

Details

Node

ibc commented 9 months ago

If caller does have a spare value to give away, no extra allocation will be necessary with old API, if there isn't, the caller will have to do explicit clone. But getting value as an argument that you know will always be cloned is anti-pattern in Rust because it introduces unnecessary allocations that caller can't prevent.

Example:

  1. User defines media_codecs in a variable.
  2. User passes media_codecs to router1 = worker.create_router(media_codecs).
  3. media_codecs is internally modified by mediasoup. User doesn't know it.
  4. User passes same media_codecs variable to router2 = worker.create_router(media_codecs). At that time he is passing a modified media_codecs.
  5. Do we really want that?

Basically you are invalidating the whole purpose of this PR by arguing that the user should clone the data by himself if he doesn't want it to be potentially modified by mediasoup. I just don't get it.

nazar-pc commented 9 months ago
  1. User passes same media_codecs variable to router2 = worker.create_router(media_codecs). At that time he is passing a modified media_codecs.

Not possible, ownership of media_codecs will be transferred during the first call. You'll have to clone it during the first call or else it will not compile. This is not TypeScript and not C++.

ibc commented 9 months ago

ownership of media_codecs will be transferred during the first call

Ok, that solves it.

May I know how such a ownership transfer is declared in the code?

nazar-pc commented 9 months ago

May I know how such a ownership transfer is declared in the code?

If you specify a variable - you transfer ownership, if you want to give a reference to the data/borrow it, you use &foo or &mut foo explicitly.

ibc commented 9 months ago

Rust changes reverted.

ibc commented 9 months ago

Rust changes look fine to me 😂

Thanks, I had to investigate and learn a lot to write them properly.