From a9e0e1a81eda97b1dcb9c8715010d92f3f76b010 Mon Sep 17 00:00:00 2001 From: Miau Lightouch <5199594+miaulightouch@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:37:08 +0800 Subject: [PATCH] fix(webrtc): fix GStreamer WHEP interop (#4720) ## Summary This PR fixes GStreamer interoperability issues during WebRTC/WHEP negotiation with ZLMediaServer. GStreamer could fail to establish the connection for two separate reasons: 1. ZLMediaServer generated a non-compliant ICE `ufrag`. The generated value contained `_`, which is not a valid ICE character, so GStreamer rejected the SDP. 2. ZLMediaServer did not correctly handle `bundle-only` offers and could answer an accepted bundled m-line with `port=0`, which caused the later WHEP negotiation to fail. ## Changes - Generate ICE `ufrag` values using ICE-compliant characters only. - Preserve and handle `a=bundle-only` correctly during SDP parsing and answer generation. - Return `port=9` instead of `port=0` for accepted bundled m-lines. - Add regression coverage for `bundle-only` SDP handling. - URL-encode `delete_webrtc` query parameters in the returned `Location` header so ICE-safe identifiers remain round-trippable over HTTP. ## Validation - Built with WebRTC and SCTP enabled. - Added regression test: `test_webrtc_regression` - Verified: - ICE-safe identifier round-trip through `delete_webrtc` - `bundle-only` SDP answer generation --- server/WebApi.cpp | 6 +- tests/CMakeLists.txt | 4 +- tests/test_webrtc_regression.cpp | 115 +++++++++++++++++++++++++++++++ webrtc/Sdp.cpp | 57 ++++++++------- webrtc/Sdp.h | 3 + webrtc/WebRtcTransport.cpp | 7 +- 6 files changed, 165 insertions(+), 27 deletions(-) create mode 100644 tests/test_webrtc_regression.cpp diff --git a/server/WebApi.cpp b/server/WebApi.cpp index 40c03181..97363caa 100755 --- a/server/WebApi.cpp +++ b/server/WebApi.cpp @@ -2180,10 +2180,14 @@ void installWebApi() { WebRtcPluginManager::Instance().negotiateSdp(session, type, *args, [invoker, offer, headerOut, location](const WebRtcInterface &exchanger) mutable { auto &handler = const_cast(exchanger); try { + // Encode query params since transport id/token may contain '+' or '/'. + HttpArgs delete_args; + delete_args["id"] = exchanger.getIdentifier(); + delete_args["token"] = exchanger.deleteRandStr(); // 设置返回类型 [AUTO-TRANSLATED:ffc2a31a] // Set return type headerOut["Content-Type"] = "application/sdp"; - headerOut["Location"] = location + "?id=" + exchanger.getIdentifier() + "&token=" + exchanger.deleteRandStr(); + headerOut["Location"] = location + "?" + delete_args.make(); invoker(201, headerOut, handler.getAnswerSdp(offer)); } catch (std::exception &ex) { headerOut["Content-Type"] = "text/plain"; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6781dfab..2737ff30 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -34,10 +34,10 @@ foreach(TEST_SRC ${TEST_SRC_LIST}) continue() endif() endif() - + if(NOT TARGET ZLMediaKit::WebRTC) # 暂时过滤掉依赖 WebRTC 的测试模块 - if("${TEST_EXE_NAME}" MATCHES "test_rtcp_nack") + if("${TEST_EXE_NAME}" MATCHES "test_rtcp_nack|test_webrtc_regression") continue() endif() endif() diff --git a/tests/test_webrtc_regression.cpp b/tests/test_webrtc_regression.cpp new file mode 100644 index 00000000..f40dd246 --- /dev/null +++ b/tests/test_webrtc_regression.cpp @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2016-present The ZLMediaKit project authors. All Rights Reserved. + * + * This file is part of ZLMediaKit(https://github.com/ZLMediaKit/ZLMediaKit). + * + * Use of this source code is governed by MIT-like license that can be found in the + * LICENSE file in the root of the source tree. All contributing project authors + * may be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include +#include +#include + +#include "Common/Parser.h" +#include "Common/strCoding.h" +#include "Http/HttpClient.h" +#include "../webrtc/Sdp.h" + +using namespace std; +using namespace mediakit; + +namespace { + +void expect(bool cond, const string &msg) { + if (!cond) { + throw runtime_error(msg); + } +} + +string makeBundleOnlyDatachannelOffer() { + return + "v=0\r\n" + "o=- 0 0 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=group:BUNDLE 0\r\n" + "a=msid-semantic: WMS\r\n" + "m=application 0 UDP/DTLS/SCTP webrtc-datachannel\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=ice-ufrag:remoteUfrag\r\n" + "a=ice-pwd:remotePassword1234567890\r\n" + "a=fingerprint:sha-256 " + "00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF:" + "00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF\r\n" + "a=setup:actpass\r\n" + "a=mid:0\r\n" + "a=bundle-only\r\n" + "a=sctp-port:5000\r\n"; +} + +void testBundleOnlyDatachannelAnswer() { + RtcSession offer; + offer.loadFrom(makeBundleOnlyDatachannelOffer()); + offer.checkValid(); + + expect(offer.media.size() == 1, "offer should contain a single application m-line"); + expect(offer.media[0].bundle_only, "offer application m-line should preserve a=bundle-only"); + + SdpAttrFingerprint local_fingerprint; + local_fingerprint.algorithm = "sha-256"; + local_fingerprint.hash = + "FF:EE:DD:CC:BB:AA:99:88:77:66:55:44:33:22:11:00:" + "FF:EE:DD:CC:BB:AA:99:88:77:66:55:44:33:22:11:00"; + + RtcConfigure configure; + configure.setDefaultSetting("localUfrag", "localPassword1234567890", RtpDirection::sendrecv, local_fingerprint); + + auto answer = configure.createAnswer(offer); + expect(answer != nullptr, "createAnswer should return a session"); + expect(answer->media.size() == 1, "answer should contain a single application m-line"); + +#ifdef ENABLE_SCTP + answer->checkValid(); + expect(answer->media[0].port == 9, "bundle-only application m-line should use port 9 in answer"); + expect(answer->group.mids.size() == 1 && answer->group.mids[0] == "0", + "accepted bundle-only application m-line should remain in group:BUNDLE"); +#else + expect(answer->media[0].port == 0, "application m-line should stay rejected when SCTP is disabled"); +#endif +} + +void testDeleteWebrtcLocationQueryRoundTrip() { + const string raw_id = "Ab+/9"; + const string raw_token = "token+/9"; + + HttpArgs args; + args["id"] = raw_id; + args["token"] = raw_token; + auto query = args.make(); + + expect(query.find("id=Ab%2B%2F9") != string::npos, "id should be URL-encoded in delete_webrtc query"); + expect(query.find("token=token%2B%2F9") != string::npos, + "token should be URL-encoded in delete_webrtc query"); + + auto parsed = Parser::parseArgs(query); + expect(strCoding::UrlDecodeComponent(parsed["id"]) == raw_id, "encoded id should round-trip through query parsing"); + expect(strCoding::UrlDecodeComponent(parsed["token"]) == raw_token, + "encoded token should round-trip through query parsing"); +} + +} // namespace + +int main() { + try { + testBundleOnlyDatachannelAnswer(); + testDeleteWebrtcLocationQueryRoundTrip(); + cout << "test_webrtc_regression passed" << endl; + return 0; + } catch (const exception &ex) { + cerr << "test_webrtc_regression failed: " << ex.what() << endl; + return EXIT_FAILURE; + } +} diff --git a/webrtc/Sdp.cpp b/webrtc/Sdp.cpp index a6412e0b..032e3628 100644 --- a/webrtc/Sdp.cpp +++ b/webrtc/Sdp.cpp @@ -799,6 +799,7 @@ void RtcSession::loadFrom(const string &str) { rtc_media.fingerprint = sdp.getItemClass('a', "fingerprint"); } rtc_media.ice_lite = media.getItem('a', "ice-lite").operator bool(); + rtc_media.bundle_only = media.getItem('a', "bundle-only").operator bool(); auto ice_options = media.getItemClass('a', "ice-options"); rtc_media.ice_trickle = ice_options.trickle; rtc_media.ice_renomination = ice_options.renomination; @@ -1650,14 +1651,23 @@ static RtpDirection matchDirection(RtpDirection offer_direction, RtpDirection su } } -static DtlsRole mathDtlsRole(DtlsRole role) { - switch (role) { - case DtlsRole::actpass: - case DtlsRole::active: return DtlsRole::passive; - case DtlsRole::passive: return DtlsRole::active; - default: CHECK(0, "invalid role:", getDtlsRoleString(role)); return DtlsRole::passive; - } -} +static DtlsRole mathDtlsRole(DtlsRole role) { + switch (role) { + case DtlsRole::actpass: + case DtlsRole::active: return DtlsRole::passive; + case DtlsRole::passive: return DtlsRole::active; + default: CHECK(0, "invalid role:", getDtlsRoleString(role)); return DtlsRole::passive; + } +} + +static uint16_t getAnswerMediaPort(const RtcMedia &offer_media) { + // RFC 8843: bundle-only m-lines may use port=0 in the offer but still need a + // real port placeholder in the accepted answer. + if (!offer_media.port && offer_media.bundle_only) { + return 9; + } + return offer_media.port; +} void RtcConfigure::createMediaOffer(const std::shared_ptr &ret) const { int index = 0; @@ -1824,13 +1834,14 @@ void RtcConfigure::matchMedia(const std::shared_ptr &ret, const RtcM RETRY: - if (offer_media.type == TrackApplication) { - RtcMedia answer_media = offer_media; - answer_media.role = mathDtlsRole(offer_media.role); - answer_media.ice_ufrag = configure.ice_ufrag; - answer_media.ice_pwd = configure.ice_pwd; - answer_media.fingerprint = configure.fingerprint; - answer_media.ice_lite = configure.ice_lite; + if (offer_media.type == TrackApplication) { + RtcMedia answer_media = offer_media; + answer_media.port = getAnswerMediaPort(offer_media); + answer_media.role = mathDtlsRole(offer_media.role); + answer_media.ice_ufrag = configure.ice_ufrag; + answer_media.ice_pwd = configure.ice_pwd; + answer_media.fingerprint = configure.fingerprint; + answer_media.ice_lite = configure.ice_lite; #ifdef ENABLE_SCTP answer_media.candidate = configure.candidate; #else @@ -1867,14 +1878,14 @@ RETRY: // All codecs for this media in the offer are not supported continue; } - RtcMedia answer_media; - answer_media.type = offer_media.type; - answer_media.mid = offer_media.mid; - answer_media.proto = offer_media.proto; - answer_media.port = offer_media.port; - answer_media.addr = offer_media.addr; - answer_media.bandwidth = offer_media.bandwidth; - answer_media.rtcp_addr = offer_media.rtcp_addr; + RtcMedia answer_media; + answer_media.type = offer_media.type; + answer_media.mid = offer_media.mid; + answer_media.proto = offer_media.proto; + answer_media.port = getAnswerMediaPort(offer_media); + answer_media.addr = offer_media.addr; + answer_media.bandwidth = offer_media.bandwidth; + answer_media.rtcp_addr = offer_media.rtcp_addr; answer_media.rtcp_mux = offer_media.rtcp_mux && configure.rtcp_mux; answer_media.rtcp_rsize = offer_media.rtcp_rsize && configure.rtcp_rsize; answer_media.ice_trickle = offer_media.ice_trickle && configure.ice_trickle; diff --git a/webrtc/Sdp.h b/webrtc/Sdp.h index 8139f645..149b30ae 100644 --- a/webrtc/Sdp.h +++ b/webrtc/Sdp.h @@ -652,6 +652,9 @@ public: bool rtcp_rsize { false }; SdpAttrRtcp rtcp_addr; + //////// bundle //////// + bool bundle_only { false }; + //////// ice //////// bool ice_trickle { false }; bool ice_lite { false }; diff --git a/webrtc/WebRtcTransport.cpp b/webrtc/WebRtcTransport.cpp index 6cbfadff..7335d9c6 100644 --- a/webrtc/WebRtcTransport.cpp +++ b/webrtc/WebRtcTransport.cpp @@ -137,7 +137,12 @@ static std::string getServerPrefix() { // 拷贝tcp端口 [AUTO-TRANSLATED:23191878] // Copy tcp port memcpy(buf + 6, &(reinterpret_cast(&addr)->sin_port), 2); - auto ret = encodeBase64(string(buf, 8)) + '_'; + // RFC 5245 §15.4: ice-char = ALPHA / DIGIT / "+" / "/" + auto ret = encodeBase64(string(buf, 8)); + // Remove base64 '=' padding (not a valid ice-char) + ret.erase(std::remove(ret.begin(), ret.end(), '='), ret.end()); + // Use '/' separator instead of '_' (not a valid ice-char) + ret += '/'; InfoL << "MediaServer(" << host << ":" << udp_port << ":" << tcp_port << ") prefix: " << ret; return ret; }