meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.25k stars 2.48k forks source link

Capture potential null access bugs in janus.js #3110

Closed veeting closed 1 year ago

veeting commented 1 year ago

Hi Lorenzo,

We have recently seen some exceptions in our logs that indicate that config.pc was null when trying to call getTransceivers(). We don't know why it is happening, from the logs it seams to only happen on Android devices so far.

This PR adds three null checks to mitigate this potential issues.

Kind regards,

Fabian

januscla commented 1 year ago

Thanks for your contribution, @veeting! Please make sure you sign our CLA, as it's a required step before we can merge this.

veeting commented 1 year ago

Thanks, @atoppi

Your short hand writing is nicer, however, I tried to be consistent with similar checks which already exist in master, for instance:

https://github.com/meetecho/janus-gateway/blob/master/html/janus.js#L1928

Would you prefer me to

  1. leave the code in this PR as is, to match the existing code, or
  2. just rewrite this PR, or
  3. rewrite all occurrences in the existing code to match your suggestion?

I am happy with all, however, I would prefer 1 or 3, to keep the code consistent.

atoppi commented 1 year ago

I see. Okay, let's keep the code consistent: option no. 1

lminiero commented 1 year ago

Merging then, thanks! I'll check if the same fix needs to be packported to 0.x.