Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: throw error in setLocalDescription if media line is missing codecs #63

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

brycetham
Copy link
Contributor

This PR fixes an issue in which Firefox will allow setLocalDescription to be called even when all the codecs have been removed from the media line as part of SPARK-465378. This violates RFC8866, which states that at least one codec is required.

A bug has been filed to Firefox about this: https://bugzilla.mozilla.org/show_bug.cgi?id=1857612

Until Firefox fixes this bug, the fix from our end is to add a check in setLocalDescription to check the number of fields in the media line. This is similar to what Google does: https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/pc/webrtc_sdp.cc;l=2682;drc=f5bdc89c7395ed24f1b8d196a3bdd6232d5bf771

If the number of fields is less than 4 (i.e. the codecs are missing), then we throw an error.

@brycetham brycetham requested a review from bbaldino October 12, 2023 14:11
@brycetham
Copy link
Contributor Author

Keep this as draft until webex/web-capabilities#3 is merged.

@brycetham brycetham marked this pull request as ready for review October 12, 2023 17:35
@brycetham brycetham merged commit ec5f257 into main Oct 12, 2023
1 check passed
@brycetham brycetham deleted the btham/sld_media_line_error branch October 12, 2023 17:45
bbaldino pushed a commit that referenced this pull request Oct 12, 2023
## [2.2.1](v2.2.0...v2.2.1) (2023-10-12)

### Bug Fixes

* throw error in setLocalDescription if media line is missing codecs ([#63](#63)) ([ec5f257](ec5f257))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants