diff --git a/CHANGELOG.md b/CHANGELOG.md index fa51a3c085..8ae8f1d258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog +### 3.12.16 + +* Fix `IceServer` crash when client uses ICE renomination ([PR #1182](https://github.com/versatica/mediasoup/pull/1182)). + + ### 3.12.15 * Fix NPM "postinstall" task in Windows ([PR #1187](https://github.com/versatica/mediasoup/pull/1187)). diff --git a/package-lock.json b/package-lock.json index 00a90d66bc..51307cb9e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@mafalda-sfu/mediasoup", - "version": "3.12.15", + "version": "3.12.16", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@mafalda-sfu/mediasoup", - "version": "3.12.15", + "version": "3.12.16", "hasInstallScript": true, "license": "ISC", "dependencies": { @@ -25,8 +25,8 @@ "@types/uuid": "^9.0.6", "@typescript-eslint/eslint-plugin": "^6.8.0", "@typescript-eslint/parser": "^6.8.0", - "eslint": "^8.51.0", - "eslint-plugin-jest": "^27.4.2", + "eslint": "^8.52.0", + "eslint-plugin-jest": "^27.4.3", "jest": "^29.7.0", "marked": "^9.1.2", "open-cli": "^7.2.0", @@ -766,21 +766,21 @@ } }, "node_modules/@eslint/js": { - "version": "8.51.0", - "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.51.0.tgz", - "integrity": "sha512-HxjQ8Qn+4SI3/AFv6sOrDB+g6PpUTDwSJiQqOrnneEk8L71161srI9gjzzZvYVbzHiVg/BvcH95+cK/zfIt4pg==", + "version": "8.52.0", + "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.52.0.tgz", + "integrity": "sha512-mjZVbpaeMZludF2fsWLD0Z9gCref1Tk4i9+wddjRvpUNqqcndPkBD09N/Mapey0b3jaXbLm2kICwFv2E64QinA==", "dev": true, "engines": { "node": "^12.22.0 || ^14.17.0 || >=16.0.0" } }, "node_modules/@humanwhocodes/config-array": { - "version": "0.11.11", - "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.11.tgz", - "integrity": "sha512-N2brEuAadi0CcdeMXUkhbZB84eskAc8MEX1By6qEchoVywSgXPIjou4rYsl0V3Hj0ZnuGycGCjdNgockbzeWNA==", + "version": "0.11.13", + "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.13.tgz", + "integrity": "sha512-JSBDMiDKSzQVngfRjOdFXgFfklaXI4K9nLF49Auh21lmBWRLIK3+xTErTWD4KU54pb6coM6ESE7Awz/FNU3zgQ==", "dev": true, "dependencies": { - "@humanwhocodes/object-schema": "^1.2.1", + "@humanwhocodes/object-schema": "^2.0.1", "debug": "^4.1.1", "minimatch": "^3.0.5" }, @@ -802,9 +802,9 @@ } }, "node_modules/@humanwhocodes/object-schema": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@humanwhocodes/object-schema/-/object-schema-1.2.1.tgz", - "integrity": "sha512-ZnQMnLV4e7hDlUvw8H+U8ASL02SS2Gn6+9Ac3wGGLIe7+je2AeAOxPY+izIPJDfFDb7eDjev0Us8MO1iFRN8hA==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@humanwhocodes/object-schema/-/object-schema-2.0.1.tgz", + "integrity": "sha512-dvuCeX5fC9dXgJn9t+X5atfmgQAzUOWqS1254Gh0m6i8wKd10ebXkfNKiRK+1GWi/yTvvLDHpoxLr0xxxeslWw==", "dev": true }, "node_modules/@istanbuljs/load-nyc-config": { @@ -2129,6 +2129,12 @@ "url": "https://opencollective.com/typescript-eslint" } }, + "node_modules/@ungap/structured-clone": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/@ungap/structured-clone/-/structured-clone-1.2.0.tgz", + "integrity": "sha512-zuVdFrMJiuCDQUMCzQaD6KL28MjnqqN8XnAqiEq9PNm/hCPTSGfrXCOfwj1ow4LFb/tNymJPwsNbVePc1xFqrQ==", + "dev": true + }, "node_modules/acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", @@ -3024,18 +3030,19 @@ } }, "node_modules/eslint": { - "version": "8.51.0", - "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.51.0.tgz", - "integrity": "sha512-2WuxRZBrlwnXi+/vFSJyjMqrNjtJqiasMzehF0shoLaW7DzS3/9Yvrmq5JiT66+pNjiX4UBnLDiKHcWAr/OInA==", + "version": "8.52.0", + "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.52.0.tgz", + "integrity": "sha512-zh/JHnaixqHZsolRB/w9/02akBk9EPrOs9JwcTP2ek7yL5bVvXuRariiaAjjoJ5DvuwQ1WAE/HsMz+w17YgBCg==", "dev": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", "@eslint/eslintrc": "^2.1.2", - "@eslint/js": "8.51.0", - "@humanwhocodes/config-array": "^0.11.11", + "@eslint/js": "8.52.0", + "@humanwhocodes/config-array": "^0.11.13", "@humanwhocodes/module-importer": "^1.0.1", "@nodelib/fs.walk": "^1.2.8", + "@ungap/structured-clone": "^1.2.0", "ajv": "^6.12.4", "chalk": "^4.0.0", "cross-spawn": "^7.0.2", @@ -3078,9 +3085,9 @@ } }, "node_modules/eslint-plugin-jest": { - "version": "27.4.2", - "resolved": "https://registry.npmjs.org/eslint-plugin-jest/-/eslint-plugin-jest-27.4.2.tgz", - "integrity": "sha512-3Nfvv3wbq2+PZlRTf2oaAWXWwbdBejFRBR2O8tAO67o+P8zno+QGbcDYaAXODlreXVg+9gvWhKKmG2rgfb8GEg==", + "version": "27.4.3", + "resolved": "https://registry.npmjs.org/eslint-plugin-jest/-/eslint-plugin-jest-27.4.3.tgz", + "integrity": "sha512-7S6SmmsHsgIm06BAGCAxL+ABd9/IB3MWkz2pudj6Qqor2y1qQpWPfuFU4SG9pWj4xDjF0e+D7Llh5useuSzAZw==", "dev": true, "dependencies": { "@typescript-eslint/utils": "^5.10.0" @@ -7308,18 +7315,18 @@ } }, "@eslint/js": { - "version": "8.51.0", - "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.51.0.tgz", - "integrity": "sha512-HxjQ8Qn+4SI3/AFv6sOrDB+g6PpUTDwSJiQqOrnneEk8L71161srI9gjzzZvYVbzHiVg/BvcH95+cK/zfIt4pg==", + "version": "8.52.0", + "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.52.0.tgz", + "integrity": "sha512-mjZVbpaeMZludF2fsWLD0Z9gCref1Tk4i9+wddjRvpUNqqcndPkBD09N/Mapey0b3jaXbLm2kICwFv2E64QinA==", "dev": true }, "@humanwhocodes/config-array": { - "version": "0.11.11", - "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.11.tgz", - "integrity": "sha512-N2brEuAadi0CcdeMXUkhbZB84eskAc8MEX1By6qEchoVywSgXPIjou4rYsl0V3Hj0ZnuGycGCjdNgockbzeWNA==", + "version": "0.11.13", + "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.13.tgz", + "integrity": "sha512-JSBDMiDKSzQVngfRjOdFXgFfklaXI4K9nLF49Auh21lmBWRLIK3+xTErTWD4KU54pb6coM6ESE7Awz/FNU3zgQ==", "dev": true, "requires": { - "@humanwhocodes/object-schema": "^1.2.1", + "@humanwhocodes/object-schema": "^2.0.1", "debug": "^4.1.1", "minimatch": "^3.0.5" } @@ -7331,9 +7338,9 @@ "dev": true }, "@humanwhocodes/object-schema": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@humanwhocodes/object-schema/-/object-schema-1.2.1.tgz", - "integrity": "sha512-ZnQMnLV4e7hDlUvw8H+U8ASL02SS2Gn6+9Ac3wGGLIe7+je2AeAOxPY+izIPJDfFDb7eDjev0Us8MO1iFRN8hA==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@humanwhocodes/object-schema/-/object-schema-2.0.1.tgz", + "integrity": "sha512-dvuCeX5fC9dXgJn9t+X5atfmgQAzUOWqS1254Gh0m6i8wKd10ebXkfNKiRK+1GWi/yTvvLDHpoxLr0xxxeslWw==", "dev": true }, "@istanbuljs/load-nyc-config": { @@ -8320,6 +8327,12 @@ "eslint-visitor-keys": "^3.3.0" } }, + "@ungap/structured-clone": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/@ungap/structured-clone/-/structured-clone-1.2.0.tgz", + "integrity": "sha512-zuVdFrMJiuCDQUMCzQaD6KL28MjnqqN8XnAqiEq9PNm/hCPTSGfrXCOfwj1ow4LFb/tNymJPwsNbVePc1xFqrQ==", + "dev": true + }, "acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", @@ -8956,18 +8969,19 @@ "dev": true }, "eslint": { - "version": "8.51.0", - "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.51.0.tgz", - "integrity": "sha512-2WuxRZBrlwnXi+/vFSJyjMqrNjtJqiasMzehF0shoLaW7DzS3/9Yvrmq5JiT66+pNjiX4UBnLDiKHcWAr/OInA==", + "version": "8.52.0", + "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.52.0.tgz", + "integrity": "sha512-zh/JHnaixqHZsolRB/w9/02akBk9EPrOs9JwcTP2ek7yL5bVvXuRariiaAjjoJ5DvuwQ1WAE/HsMz+w17YgBCg==", "dev": true, "requires": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", "@eslint/eslintrc": "^2.1.2", - "@eslint/js": "8.51.0", - "@humanwhocodes/config-array": "^0.11.11", + "@eslint/js": "8.52.0", + "@humanwhocodes/config-array": "^0.11.13", "@humanwhocodes/module-importer": "^1.0.1", "@nodelib/fs.walk": "^1.2.8", + "@ungap/structured-clone": "^1.2.0", "ajv": "^6.12.4", "chalk": "^4.0.0", "cross-spawn": "^7.0.2", @@ -9019,9 +9033,9 @@ } }, "eslint-plugin-jest": { - "version": "27.4.2", - "resolved": "https://registry.npmjs.org/eslint-plugin-jest/-/eslint-plugin-jest-27.4.2.tgz", - "integrity": "sha512-3Nfvv3wbq2+PZlRTf2oaAWXWwbdBejFRBR2O8tAO67o+P8zno+QGbcDYaAXODlreXVg+9gvWhKKmG2rgfb8GEg==", + "version": "27.4.3", + "resolved": "https://registry.npmjs.org/eslint-plugin-jest/-/eslint-plugin-jest-27.4.3.tgz", + "integrity": "sha512-7S6SmmsHsgIm06BAGCAxL+ABd9/IB3MWkz2pudj6Qqor2y1qQpWPfuFU4SG9pWj4xDjF0e+D7Llh5useuSzAZw==", "dev": true, "requires": { "@typescript-eslint/utils": "^5.10.0" diff --git a/package.json b/package.json index 9b9f973f53..c86af04bd9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mafalda-sfu/mediasoup", - "version": "3.12.15", + "version": "3.12.16", "description": "Cutting Edge WebRTC Video Conferencing", "contributors": [ "Iñaki Baz Castillo (https://inakibaz.me)", @@ -104,8 +104,8 @@ "@types/uuid": "^9.0.6", "@typescript-eslint/eslint-plugin": "^6.8.0", "@typescript-eslint/parser": "^6.8.0", - "eslint": "^8.51.0", - "eslint-plugin-jest": "^27.4.2", + "eslint": "^8.52.0", + "eslint-plugin-jest": "^27.4.3", "jest": "^29.7.0", "marked": "^9.1.2", "open-cli": "^7.2.0", diff --git a/worker/include/RTC/IceServer.hpp b/worker/include/RTC/IceServer.hpp index 51fa85919d..b213189ff5 100644 --- a/worker/include/RTC/IceServer.hpp +++ b/worker/include/RTC/IceServer.hpp @@ -93,9 +93,11 @@ namespace RTC } bool IsValidTuple(const RTC::TransportTuple* tuple) const; void RemoveTuple(RTC::TransportTuple* tuple); - // This should be just called in 'connected' or completed' state - // and the given tuple must be an already valid tuple. - void ForceSelectedTuple(const RTC::TransportTuple* tuple); + /** + * This should be just called in 'connected' or 'completed' state and the + * given tuple must be an already valid tuple. + */ + void MayForceSelectedTuple(const RTC::TransportTuple* tuple); private: void HandleTuple( @@ -122,8 +124,8 @@ namespace RTC std::string password; std::string oldUsernameFragment; std::string oldPassword; - uint32_t remoteNomination{ 0u }; IceState state{ IceState::NEW }; + uint32_t remoteNomination{ 0u }; std::list tuples; RTC::TransportTuple* selectedTuple{ nullptr }; }; diff --git a/worker/src/RTC/IceServer.cpp b/worker/src/RTC/IceServer.cpp index fde3d25a1b..4cf1a6a924 100644 --- a/worker/src/RTC/IceServer.cpp +++ b/worker/src/RTC/IceServer.cpp @@ -220,9 +220,13 @@ namespace RTC // Authenticate the response. if (this->oldPassword.empty()) + { response->Authenticate(this->password); + } else + { response->Authenticate(this->oldPassword); + } // Send back. response->Serialize(StunSerializeBuffer); @@ -233,7 +237,9 @@ namespace RTC uint32_t nomination{ 0u }; if (packet->HasNomination()) + { nomination = packet->GetNomination(); + } // Handle the tuple. HandleTuple(tuple, packet->HasUseCandidate(), packet->HasNomination(), nomination); @@ -294,7 +300,9 @@ namespace RTC // If not found, ignore. if (!removedTuple) + { return; + } // Notify the listener. this->listener->OnIceServerTupleRemoved(this, removedTuple); @@ -319,24 +327,35 @@ namespace RTC { // Update state. this->state = IceState::DISCONNECTED; + + // Reset remote nomination. + this->remoteNomination = 0u; + // Notify the listener. this->listener->OnIceServerDisconnected(this); } } } - void IceServer::ForceSelectedTuple(const RTC::TransportTuple* tuple) + void IceServer::MayForceSelectedTuple(const RTC::TransportTuple* tuple) { MS_TRACE(); - MS_ASSERT( - this->selectedTuple, "cannot force the selected tuple if there was not a selected tuple"); + if (this->state != IceState::CONNECTED && this->state != IceState::COMPLETED) + { + MS_WARN_TAG(ice, "cannot force selected tuple if not in state 'connected' or 'completed'"); + + return; + } auto* storedTuple = HasTuple(tuple); - MS_ASSERT( - storedTuple, - "cannot force the selected tuple if the given tuple was not already a valid tuple"); + if (!storedTuple) + { + MS_WARN_TAG(ice, "cannot force selected tuple if the given tuple was not already a valid one"); + + return; + } // Mark it as selected tuple. SetSelectedTuple(storedTuple); @@ -351,10 +370,6 @@ namespace RTC { case IceState::NEW: { - // There should be no tuples. - MS_ASSERT( - this->tuples.empty(), "state is 'new' but there are %zu tuples", this->tuples.size()); - // There shouldn't be a selected tuple. MS_ASSERT(!this->selectedTuple, "state is 'new' but there is selected tuple"); @@ -373,8 +388,10 @@ namespace RTC // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update state. this->state = IceState::CONNECTED; + // Notify the listener. this->listener->OnIceServerConnected(this); } @@ -395,11 +412,16 @@ namespace RTC // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update state. this->state = IceState::COMPLETED; + // Update nomination. if (hasNomination && nomination > this->remoteNomination) + { this->remoteNomination = nomination; + } + // Notify the listener. this->listener->OnIceServerCompleted(this); } @@ -410,12 +432,6 @@ namespace RTC case IceState::DISCONNECTED: { - // There should be no tuples. - MS_ASSERT( - this->tuples.empty(), - "state is 'disconnected' but there are %zu tuples", - this->tuples.size()); - // There shouldn't be a selected tuple. MS_ASSERT(!this->selectedTuple, "state is 'disconnected' but there is selected tuple"); @@ -434,8 +450,10 @@ namespace RTC // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update state. this->state = IceState::CONNECTED; + // Notify the listener. this->listener->OnIceServerConnected(this); } @@ -456,11 +474,16 @@ namespace RTC // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update state. this->state = IceState::COMPLETED; + // Update nomination. if (hasNomination && nomination > this->remoteNomination) + { this->remoteNomination = nomination; + } + // Notify the listener. this->listener->OnIceServerCompleted(this); } @@ -479,9 +502,8 @@ namespace RTC if (!hasUseCandidate && !hasNomination) { - // If a new tuple store it. - if (!HasTuple(tuple)) - AddTuple(tuple); + // Store the tuple. + AddTuple(tuple); } else { @@ -493,21 +515,23 @@ namespace RTC hasNomination ? "true" : "false", nomination); - auto* storedTuple = HasTuple(tuple); - - // If a new tuple store it. - if (!storedTuple) - storedTuple = AddTuple(tuple); + // Store the tuple. + auto* storedTuple = AddTuple(tuple); if ((hasNomination && nomination > this->remoteNomination) || !hasNomination) { // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update state. this->state = IceState::COMPLETED; + // Update nomination. if (hasNomination && nomination > this->remoteNomination) + { this->remoteNomination = nomination; + } + // Notify the listener. this->listener->OnIceServerCompleted(this); } @@ -526,25 +550,24 @@ namespace RTC if (!hasUseCandidate && !hasNomination) { - // If a new tuple store it. - if (!HasTuple(tuple)) - AddTuple(tuple); + // Store the tuple. + AddTuple(tuple); } else { - auto* storedTuple = HasTuple(tuple); - - // If a new tuple store it. - if (!storedTuple) - storedTuple = AddTuple(tuple); + // Store the tuple. + auto* storedTuple = AddTuple(tuple); if ((hasNomination && nomination > this->remoteNomination) || !hasNomination) { // Mark it as selected tuple. SetSelectedTuple(storedTuple); + // Update nomination. if (hasNomination && nomination > this->remoteNomination) + { this->remoteNomination = nomination; + } } } @@ -557,15 +580,26 @@ namespace RTC { MS_TRACE(); + auto* storedTuple = HasTuple(tuple); + + if (storedTuple) + { + MS_DEBUG_DEV('tuple already exists'); + + return storedTuple; + } + // Add the new tuple at the beginning of the list. this->tuples.push_front(*tuple); - auto* storedTuple = std::addressof(*this->tuples.begin()); + storedTuple = std::addressof(*this->tuples.begin()); // If it is UDP then we must store the remote address (until now it is // just a pointer that will be freed soon). if (storedTuple->GetProtocol() == TransportTuple::Protocol::UDP) + { storedTuple->StoreUdpRemoteAddress(); + } // Notify the listener. this->listener->OnIceServerTupleAdded(this, storedTuple); @@ -614,14 +648,11 @@ namespace RTC { MS_TRACE(); - // If there is no selected tuple yet then we know that the tuples list - // is empty. - if (!this->selectedTuple) - return nullptr; - - // Check the current selected tuple. - if (this->selectedTuple->Compare(tuple)) + // Check the current selected tuple (if any). + if (this->selectedTuple && this->selectedTuple->Compare(tuple)) + { return this->selectedTuple; + } // Otherwise check other stored tuples. for (const auto& it : this->tuples) @@ -629,7 +660,9 @@ namespace RTC auto* storedTuple = const_cast(std::addressof(it)); if (storedTuple->Compare(tuple)) + { return storedTuple; + } } return nullptr; @@ -641,7 +674,9 @@ namespace RTC // If already the selected tuple do nothing. if (storedTuple == this->selectedTuple) + { return; + } this->selectedTuple = storedTuple; diff --git a/worker/src/RTC/WebRtcTransport.cpp b/worker/src/RTC/WebRtcTransport.cpp index 000b389dbb..4846006957 100644 --- a/worker/src/RTC/WebRtcTransport.cpp +++ b/worker/src/RTC/WebRtcTransport.cpp @@ -1058,7 +1058,7 @@ namespace RTC } // Trick for clients performing aggressive ICE regardless we are ICE-Lite. - this->iceServer->ForceSelectedTuple(tuple); + this->iceServer->MayForceSelectedTuple(tuple); // Check that DTLS status is 'connecting' or 'connected'. if ( @@ -1142,7 +1142,7 @@ namespace RTC } // Trick for clients performing aggressive ICE regardless we are ICE-Lite. - this->iceServer->ForceSelectedTuple(tuple); + this->iceServer->MayForceSelectedTuple(tuple); // Pass the packet to the parent transport. RTC::Transport::ReceiveRtpPacket(packet);