From 14b651c0db9853fa09928c28a2d7ae9ee12c66c8 Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Thu, 12 Oct 2023 09:31:46 -0400 Subject: [PATCH 1/2] fix: throw error in setLocalDescription if media line is missing codecs --- package.json | 1 + src/mocks/rtc-peer-connection-stub.ts | 3 +++ src/peer-connection.spec.ts | 34 +++++++++++++++++++++++++++ src/peer-connection.ts | 16 +++++++++++++ yarn.lock | 5 ++++ 5 files changed, 59 insertions(+) diff --git a/package.json b/package.json index 0eeff04..9ee23ab 100644 --- a/package.json +++ b/package.json @@ -107,6 +107,7 @@ }, "dependencies": { "@webex/ts-events": "^1.1.0", + "@webex/web-capabilities": "^1.0.0", "@webex/web-media-effects": "^2.7.0", "events": "^3.3.0", "js-logger": "^1.6.1", diff --git a/src/mocks/rtc-peer-connection-stub.ts b/src/mocks/rtc-peer-connection-stub.ts index 6d950d9..a34124f 100644 --- a/src/mocks/rtc-peer-connection-stub.ts +++ b/src/mocks/rtc-peer-connection-stub.ts @@ -13,6 +13,9 @@ class RTCPeerConnectionStub { getStats(): Promise { return new Promise(() => {}); } + setLocalDescription(): Promise { + return new Promise(() => {}); + } onconnectionstatechange: () => void = () => {}; oniceconnectionstatechange: () => void = () => {}; } diff --git a/src/peer-connection.spec.ts b/src/peer-connection.spec.ts index e2ad793..f061166 100644 --- a/src/peer-connection.spec.ts +++ b/src/peer-connection.spec.ts @@ -1,3 +1,4 @@ +import { BrowserInfo } from '@webex/web-capabilities'; import { ConnectionState, ConnectionStateHandler } from './connection-state-handler'; import { mocked } from './mocks/mock'; import { RTCPeerConnectionStub } from './mocks/rtc-peer-connection-stub'; @@ -245,4 +246,37 @@ describe('PeerConnection', () => { connectionStateHandlerListener(ConnectionState.Connecting); }); }); + + describe('setLocalDescription', () => { + let mockPc: RTCPeerConnectionStub; + let setLocalDescriptionSpy: jest.SpyInstance; + let pc: PeerConnection; + + beforeEach(() => { + jest.clearAllMocks(); + mockPc = mocked(new RTCPeerConnectionStub(), true); + mockCreateRTCPeerConnection.mockReturnValueOnce(mockPc as unknown as RTCPeerConnection); + setLocalDescriptionSpy = jest.spyOn(mockPc, 'setLocalDescription'); + pc = new PeerConnection(); + }); + + it('sets the local description with an SDP offer', async () => { + expect.hasAssertions(); + const description = { type: 'offer', sdp: 'fake sdp' } as RTCSessionDescriptionInit; + pc.setLocalDescription(description); + expect(setLocalDescriptionSpy).toHaveBeenCalledWith(description); + }); + it('sets the local description with no SDP offer', async () => { + expect.hasAssertions(); + pc.setLocalDescription(); + expect(setLocalDescriptionSpy).toHaveBeenCalledWith(undefined); + }); + it('throws an error when the SDP has an invalid media line on Firefox', async () => { + expect.hasAssertions(); + jest.spyOn(BrowserInfo, 'isFirefox').mockReturnValue(true); + await expect( + pc.setLocalDescription({ type: 'offer', sdp: 'm=video 9 UDP/TLS/RTP' }) + ).rejects.toThrow(Error); + }); + }); }); diff --git a/src/peer-connection.ts b/src/peer-connection.ts index c100345..4ec0d98 100644 --- a/src/peer-connection.ts +++ b/src/peer-connection.ts @@ -1,3 +1,4 @@ +import { BrowserInfo } from '@webex/web-capabilities'; import { ConnectionState, ConnectionStateHandler } from './connection-state-handler'; import { EventEmitter, EventMap } from './event-emitter'; import { createRTCPeerConnection } from './rtc-peer-connection-factory'; @@ -199,6 +200,21 @@ class PeerConnection extends EventEmitter { async setLocalDescription( description?: RTCSessionDescription | RTCSessionDescriptionInit ): Promise { + // In Firefox, setLocalDescription will not throw an error if an m-line has no codecs, even + // though it violates https://datatracker.ietf.org/doc/html/rfc8866. See + // https://bugzilla.mozilla.org/show_bug.cgi?id=1857612. So, we check the media lines here to + // preemptively throw an error on Firefox. + if (BrowserInfo.isFirefox()) { + description?.sdp + ?.split(/(\r\n|\r|\n)/) + .filter((line) => line.startsWith('m')) + .forEach((mediaLine) => { + if (mediaLine.split(' ').length < 4) { + throw new Error(`Invalid media line ${mediaLine}, expected at least 4 fields`); + } + }); + } + return this.pc.setLocalDescription(description); } diff --git a/yarn.lock b/yarn.lock index 9f2a6c9..239749a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2685,6 +2685,11 @@ events "^3.3.0" typed-emitter "^2.1.0" +"@webex/web-capabilities@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@webex/web-capabilities/-/web-capabilities-1.0.0.tgz#b69f3ba492669431a486f79053146948af582de5" + integrity sha512-EIejiH9kdvpp2Cm0nq6DSusZMsiqNfJCbtmS0aiLAXPcC9gTcr7zDhqamnrbIc1PPsK08cJG7f2QDkH6BfFaPg== + "@webex/web-media-effects@^2.7.0": version "2.7.0" resolved "https://registry.yarnpkg.com/@webex/web-media-effects/-/web-media-effects-2.7.0.tgz#f203f9512fb95d0639c74c10f39eda2e58663739" From 896e1fadf5dc07738339c2d8ca452b1369d78cc0 Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Thu, 12 Oct 2023 13:33:53 -0400 Subject: [PATCH 2/2] chore: update web-capabilities version --- package.json | 2 +- yarn.lock | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 9ee23ab..3b3e499 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ }, "dependencies": { "@webex/ts-events": "^1.1.0", - "@webex/web-capabilities": "^1.0.0", + "@webex/web-capabilities": "^1.1.0", "@webex/web-media-effects": "^2.7.0", "events": "^3.3.0", "js-logger": "^1.6.1", diff --git a/yarn.lock b/yarn.lock index 239749a..6e94f87 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2685,10 +2685,12 @@ events "^3.3.0" typed-emitter "^2.1.0" -"@webex/web-capabilities@^1.0.0": - version "1.0.0" - resolved "https://registry.yarnpkg.com/@webex/web-capabilities/-/web-capabilities-1.0.0.tgz#b69f3ba492669431a486f79053146948af582de5" - integrity sha512-EIejiH9kdvpp2Cm0nq6DSusZMsiqNfJCbtmS0aiLAXPcC9gTcr7zDhqamnrbIc1PPsK08cJG7f2QDkH6BfFaPg== +"@webex/web-capabilities@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@webex/web-capabilities/-/web-capabilities-1.1.0.tgz#7e0e7c8b67ee9cb6b795fecf099ff748ef7d13c6" + integrity sha512-Sh++KBHhy8VmEQZtSFuKrvq82SgEILNkAfWvoJGWpCpV9+VirEpUdCYxwBv03V+DF2EcxmOcRt/n8bzjIG1b/A== + dependencies: + bowser "^2.11.0" "@webex/web-media-effects@^2.7.0": version "2.7.0" @@ -3281,6 +3283,11 @@ bottleneck@^2.18.1: resolved "https://registry.yarnpkg.com/bottleneck/-/bottleneck-2.19.5.tgz#5df0b90f59fd47656ebe63c78a98419205cadd91" integrity sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw== +bowser@^2.11.0: + version "2.11.0" + resolved "https://registry.yarnpkg.com/bowser/-/bowser-2.11.0.tgz#5ca3c35757a7aa5771500c70a73a9f91ef420a8f" + integrity sha512-AlcaJBi/pqqJBIQ8U9Mcpc9i8Aqxn88Skv5d+xBX006BY5u8N3mGLHa5Lgppa7L/HfwgwLgZ6NYs+Ag6uUmJRA== + brace-expansion@^1.1.7: version "1.1.11" resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.11.tgz#3c7fcbf529d87226f3d2f52b966ff5271eb441dd"