SDPUtil.findLine() returns `false` on failure while tpc._remoteTrackAdded expects it to be `undefined`

[UzEE]

This Issue tracker is only for reporting bugs and tracking code related issues.

Before posting, please make sure you check community.jitsi.org to see if the same or similar bugs have already been discussed. General questions, installation help, and feature requests can also be posted to community.jitsi.org.

Description

I'm debugging a scenario where I'm trying to get lib-jitsi-meet working in a native iOS app and came across this issue. SDPUtils.findLine() returns false if the needle isn't found in the haystack. However, in TraceablePeerConnection.prototype._remoteTrackAdded, the following snippet expects findLine() to return undefined if the line isn't found.

mediaLines = remoteSDP.media.filter(mls => {
    const msid = SDPUtil.findLine(mls, 'a=msid');

    return typeof msid !== 'undefined' && streamId === msid.substring(7).split(' ')[0];
});

This causes an error instead of just skipping the line in the filter because false.substring doesn't exist.

Current behavior

SDPUtil.findLine() returns false if the line isn't found, while at least one of it's usage expects it to return undefined instead.

Expected Behavior

mediaLines filter function should be checking for false instead of undefined if the lines aren't found.

Possible Solution

The following code should probably be something like this:

mediaLines = remoteSDP.media.filter(mls => {
    const msid = SDPUtil.findLine(mls, 'a=msid');

    return typeof msid === 'boolean' && msid !== false && streamId === msid.substring(7).split(' ')[0];
});

Environment details

custom Jitsi implementation, iOS 13.5 (using WebRTC M69 wrapped by a Cordova plugin)

---

NOTE: that we're still trying to solve other issues apart from this that prevents our custom solution from working on iOS since it currently tries to incorrectly use Unified Plan because the library correctly detects it's running on Safari but the underlaying WebRTC implementation is from Chrome M69 which defaults to Plan B.

[sapkra]

Have you already tried this patch? https://github.com/jitsi/lib-jitsi-meet/issues/595#issuecomment-624112557