Closed
Bug 1413709
Opened 3 years ago
Closed 3 years ago
add tests to detect improper ice restart from answer changing ufrag/pwd
Categories
(Core :: WebRTC: Signaling, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: mjf, Assigned: mjf)
References
Details
Attachments
(1 file)
In Bug 1405940 I fixed a crash that was ultimately the result of an answer (during renegotiation) containing modified ICE credentials causing an ICE restart when the offerer did not indicate an ICE restart.
Comment hidden (mozreview-request) |
Comment 2•3 years ago
|
||
mozreview-review |
Comment on attachment 8924333 [details] Bug 1413709 - add tests to detect improper ice restart by answer. https://reviewboard.mozilla.org/r/195572/#review201042 ::: dom/media/tests/mochitest/test_peerConnection_restartIceBadAnswer.html:31 (Diff revision 1) > + test.chain.insertAfter("PC_LOCAL_GET_ANSWER", > + [ > + function PC_LOCAL_REWRITE_REMOTE_SDP_ICE_CREDS(test) { > + test._remote_answer.sdp = > + test._remote_answer.sdp.replace(/a=ice-pwd:.*\r\n/g, > + "a=ice-pwd:bad-pwd\r\n") > + .replace(/a=ice-ufrag:.*\r\n/g, > + "a=ice-ufrag:bad-ufrag\r\n"); > + } > + ], false, 1 // insert after the second PC_LOCAL_GET_ANSWER > + ); > + > + test.chain.replaceAfter("PC_LOCAL_REWRITE_REMOTE_SDP_ICE_CREDS", > + [ > + function PC_LOCAL_EXPECT_SET_REMOTE_DESCRIPTION_FAIL(test) { > + return test.setRemoteDescription(test.pcLocal, > + test._remote_answer, > + STABLE) > + .then(() => ok(false, "setRemoteDescription must fail"), > + e => is(e.name, "InvalidSessionDescriptionError", > + "setRemoteDescription must fail and did")) > + } > + ], 0 // replace after first (only) PC_LOCAL_REWRITE_REMOTE_SDP_ICE_CREDS > + ); Couldn't we do this with a single replaceAfter? ::: dom/media/tests/mochitest/test_peerConnection_restartIceBadAnswer.html:51 (Diff revision 1) > + return test.setRemoteDescription(test.pcLocal, > + test._remote_answer, > + STABLE) > + .then(() => ok(false, "setRemoteDescription must fail"), > + e => is(e.name, "InvalidSessionDescriptionError", > + "setRemoteDescription must fail and did")) Do we need a ; here? ::: media/webrtc/signaling/gtest/jsep_session_unittest.cpp:123 (Diff revision 1) > } > > void > AddTransportData(JsepSessionImpl& session, TransportData& tdata) > { > // Values here semi-borrowed from JSEP draft. I think this comment needs to move to GenerateNewIceCredentials.
Attachment #8924333 -
Flags: review?(docfaraday) → review+
Comment hidden (mozreview-request) |
Pushed by mfroman@nostrum.com: https://hg.mozilla.org/integration/autoland/rev/6f3b7e0969c9 add tests to detect improper ice restart by answer. r=bwc
Updated•3 years ago
|
Assignee: nobody → mfroman
Comment 5•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6f3b7e0969c9
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•