[P2P] Modernize STUN and P2P code to use std::span This change replaces (const void*, size_t) and (const char*, size_t) patterns with std::span<const uint8_t> across the STUN and P2P codebases. Key changes: - Modernized api/transport/stun.h (StunMessage, StunByteStringAttribute). - Modernized p2p/base/port_interface.h, port.h and subclasses (UDPPort, TCPPort, TurnPort, TestPort). - Modernized p2p/base/connection.h and stun_request.h. - Added [[deprecated]] shims for all public API changes to maintain backward compatibility. - Updated all internal callers and unit tests. - Fixed a hardening assertion crash in api/transport/stun_unittest.cc. Bug: webrtc:42225170 Change-Id: I3ef7b21996b0c17a6292bce75be983112c9e6649 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/466420 Reviewed-by: Evan Shrubsole <eshr@webrtc.org> Commit-Queue: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47572}
diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn index c83d96a..e0f7e52 100644 --- a/api/transport/BUILD.gn +++ b/api/transport/BUILD.gn
@@ -115,8 +115,10 @@ "../../rtc_base:logging", "../../rtc_base:net_helpers", "../../rtc_base:socket_address", + "../../rtc_base:span_helpers", "../../rtc_base:stringutils", "../../system_wrappers:metrics", + "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/strings:string_view", ] } @@ -173,6 +175,7 @@ "../../rtc_base:byte_order", "../../rtc_base:ip_address", "../../rtc_base:socket_address", + "../../rtc_base:span_helpers", "../../system_wrappers:metrics", "../../test:test_support", "//testing/gtest",
diff --git a/api/transport/DEPS b/api/transport/DEPS index 7da3887..4f9869f 100644 --- a/api/transport/DEPS +++ b/api/transport/DEPS
@@ -4,6 +4,7 @@ "+rtc_base/ip_address.h", "+rtc_base/net_helpers.h", "+rtc_base/socket_address.h", + "+rtc_base/span_helpers.h", ], "data_channel_transport_interface\\.h": [ "+rtc_base/ssl_stream_adapter.h",
diff --git a/api/transport/stun.cc b/api/transport/stun.cc index 2849a5a..ef5ea5d 100644 --- a/api/transport/stun.cc +++ b/api/transport/stun.cc
@@ -11,6 +11,7 @@ #include "api/transport/stun.h" #include <algorithm> // IWYU pragma: keep +#include <array> #include <cstdint> #include <cstring> #include <functional> @@ -33,6 +34,7 @@ #include "rtc_base/message_digest.h" #include "rtc_base/net_helpers.h" #include "rtc_base/socket_address.h" +#include "rtc_base/span_helpers.h" #include "system_wrappers/include/metrics.h" using ::webrtc::ByteBufferReader; @@ -253,17 +255,17 @@ << "Usage error: Verification should only be done once"; password_ = password; if (GetByteString(STUN_ATTR_MESSAGE_INTEGRITY)) { - if (ValidateMessageIntegrityOfType( - STUN_ATTR_MESSAGE_INTEGRITY, kStunMessageIntegritySize, - buffer_.c_str(), buffer_.size(), password)) { + if (ValidateMessageIntegrityOfType(STUN_ATTR_MESSAGE_INTEGRITY, + kStunMessageIntegritySize, buffer_, + password)) { integrity_ = IntegrityStatus::kIntegrityOk; } else { integrity_ = IntegrityStatus::kIntegrityBad; } } else if (GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32)) { - if (ValidateMessageIntegrityOfType( - STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32, kStunMessageIntegrity32Size, - buffer_.c_str(), buffer_.size(), password)) { + if (ValidateMessageIntegrityOfType(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32, + kStunMessageIntegrity32Size, buffer_, + password)) { integrity_ = IntegrityStatus::kIntegrityOk; } else { integrity_ = IntegrityStatus::kIntegrityBad; @@ -348,20 +350,17 @@ } bool StunMessage::ValidateMessageIntegrityForTesting( - const char* data, - size_t size, - const std::string& password) { - return ValidateMessageIntegrityOfType(STUN_ATTR_MESSAGE_INTEGRITY, - kStunMessageIntegritySize, data, size, - password); + const std::string& password, + std::span<const uint8_t> data) { + return ValidateMessageIntegrityOfType( + STUN_ATTR_MESSAGE_INTEGRITY, kStunMessageIntegritySize, data, password); } bool StunMessage::ValidateMessageIntegrity32ForTesting( - const char* data, - size_t size, - const std::string& password) { + const std::string& password, + std::span<const uint8_t> data) { return ValidateMessageIntegrityOfType(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32, - kStunMessageIntegrity32Size, data, size, + kStunMessageIntegrity32Size, data, password); } @@ -369,40 +368,35 @@ // procedure outlined in RFC 5389, section 15.4. bool StunMessage::ValidateMessageIntegrityOfType(int mi_attr_type, size_t mi_attr_size, - const char* data, - size_t size, + std::span<const uint8_t> data, const std::string& password) { RTC_DCHECK(mi_attr_size <= kStunMessageIntegritySize); // Verifying the size of the message. - if ((size % 4) != 0 || size < kStunHeaderSize) { + if ((data.size() % 4) != 0 || data.size() < kStunHeaderSize) { return false; } - std::span<const uint8_t> data_view(reinterpret_cast<const uint8_t*>(data), - size); - // Getting the message length from the STUN header. - uint16_t msg_length = GetBE16(data_view.subspan(2, 2)); - if (size != (msg_length + kStunHeaderSize)) { + uint16_t msg_length = GetBE16(data.subspan(2, 2)); + if (data.size() != (msg_length + kStunHeaderSize)) { return false; } // Finding Message Integrity attribute in stun message. size_t current_pos = kStunHeaderSize; bool has_message_integrity_attr = false; - while (current_pos + 4 <= size) { + while (current_pos + 4 <= data.size()) { uint16_t attr_type, attr_length; // Getting attribute type and length. - attr_type = GetBE16(data_view.subspan(current_pos, 2)); - attr_length = - GetBE16(data_view.subspan(current_pos + sizeof(attr_type), 2)); + attr_type = GetBE16(data.subspan(current_pos, 2)); + attr_length = GetBE16(data.subspan(current_pos + sizeof(attr_type), 2)); // If M-I, sanity check it, and break out. if (attr_type == mi_attr_type) { if (attr_length != mi_attr_size || current_pos + sizeof(attr_type) + sizeof(attr_length) + attr_length > - size) { + data.size()) { return false; } has_message_integrity_attr = true; @@ -422,14 +416,14 @@ // Getting length of the message to calculate Message Integrity. size_t mi_pos = current_pos; - std::unique_ptr<char[]> temp_data(new char[current_pos]); - memcpy(temp_data.get(), data, current_pos); - if (size > mi_pos + kStunAttributeHeaderSize + mi_attr_size) { + std::unique_ptr<uint8_t[]> temp_data(new uint8_t[current_pos]); + memcpy(temp_data.get(), data.data(), current_pos); + if (data.size() > mi_pos + kStunAttributeHeaderSize + mi_attr_size) { // Stun message has other attributes after message integrity. // Adjust the length parameter in stun message to calculate HMAC. size_t extra_offset = - size - (mi_pos + kStunAttributeHeaderSize + mi_attr_size); - size_t new_adjusted_len = size - extra_offset - kStunHeaderSize; + data.size() - (mi_pos + kStunAttributeHeaderSize + mi_attr_size); + size_t new_adjusted_len = data.size() - extra_offset - kStunHeaderSize; // Writing new length of the STUN message @ Message Length in temp buffer. // 0 1 2 3 @@ -437,22 +431,22 @@ // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // |0 0| STUN Message Type | Message Length | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - SetBE16( - std::span<uint8_t>(reinterpret_cast<uint8_t*>(temp_data.get() + 2), 2), - static_cast<uint16_t>(new_adjusted_len)); + SetBE16(std::span<uint8_t>(temp_data.get() + 2, 2), + static_cast<uint16_t>(new_adjusted_len)); } - char hmac[kStunMessageIntegritySize]; - size_t ret = ComputeHmac(DIGEST_SHA_1, password.c_str(), password.size(), - temp_data.get(), mi_pos, hmac, sizeof(hmac)); - RTC_DCHECK(ret == sizeof(hmac)); - if (ret != sizeof(hmac)) { + std::array<uint8_t, kStunMessageIntegritySize> hmac; + size_t ret = ComputeHmac(DIGEST_SHA_1, AsUint8Span(password), + std::span(temp_data.get(), mi_pos), hmac); + if (ret != hmac.size()) { + RTC_DCHECK_NOTREACHED() << "hmac return != hmac.size()"; return false; } // Comparing the calculated HMAC with the one present in the message. - return memcmp(data + current_pos + kStunAttributeHeaderSize, hmac, - mi_attr_size) == 0; + return std::ranges::equal( + data.subspan(current_pos + kStunAttributeHeaderSize, mi_attr_size), + std::span(hmac).first(mi_attr_size)); } bool StunMessage::AddMessageIntegrity(absl::string_view password) { @@ -483,18 +477,18 @@ int msg_len_for_hmac = static_cast<int>( buf.Length() - kStunAttributeHeaderSize - msg_integrity_attr->length()); - char hmac[kStunMessageIntegritySize]; - size_t ret = ComputeHmac(DIGEST_SHA_1, key.data(), key.size(), buf.Data(), - msg_len_for_hmac, hmac, sizeof(hmac)); - RTC_DCHECK(ret == sizeof(hmac)); - if (ret != sizeof(hmac)) { + std::array<uint8_t, kStunMessageIntegritySize> hmac; + size_t ret = ComputeHmac(DIGEST_SHA_1, AsUint8Span(key), + std::span(buf.Data(), msg_len_for_hmac), hmac); + if (ret != hmac.size()) { + RTC_DCHECK_NOTREACHED(); RTC_LOG(LS_ERROR) << "HMAC computation failed. Message-Integrity " "has dummy value."; return false; } // Insert correct HMAC into the attribute. - msg_integrity_attr->CopyBytes(hmac, attr_size); + msg_integrity_attr->CopyBytes(std::span(hmac).first(attr_size)); password_ = std::string(key); integrity_ = IntegrityStatus::kIntegrityOk; return true; @@ -503,35 +497,33 @@ // Verifies a message is in fact a STUN message, by performing the checks // outlined in RFC 5389, section 7.3, including the FINGERPRINT check detailed // in section 15.5. -bool StunMessage::ValidateFingerprint(const char* data, size_t size) { +bool StunMessage::ValidateFingerprint(std::span<const uint8_t> data) { // Check the message length. size_t fingerprint_attr_size = kStunAttributeHeaderSize + StunUInt32Attribute::SIZE; - if (size % 4 != 0 || size < kStunHeaderSize + fingerprint_attr_size) + if (data.size() % 4 != 0 || + data.size() < kStunHeaderSize + fingerprint_attr_size) return false; - std::span<const uint8_t> data_view(reinterpret_cast<const uint8_t*>(data), - size); - // Skip the rest if the magic cookie isn't present. size_t magic_cookie_offset = kStunTransactionIdOffset - kStunMagicCookieLength; - if (GetBE32(data_view.subspan(magic_cookie_offset, 4)) != kStunMagicCookie) + if (GetBE32(data.subspan(magic_cookie_offset, 4)) != kStunMagicCookie) return false; // Check the fingerprint type and length. - size_t fingerprint_attr_offset = size - fingerprint_attr_size; - if (GetBE16(data_view.subspan(fingerprint_attr_offset, 2)) != + size_t fingerprint_attr_offset = data.size() - fingerprint_attr_size; + if (GetBE16(data.subspan(fingerprint_attr_offset, 2)) != STUN_ATTR_FINGERPRINT || - GetBE16(data_view.subspan(fingerprint_attr_offset + sizeof(uint16_t), - 2)) != StunUInt32Attribute::SIZE) + GetBE16(data.subspan(fingerprint_attr_offset + sizeof(uint16_t), 2)) != + StunUInt32Attribute::SIZE) return false; // Check the fingerprint value. uint32_t fingerprint = GetBE32( - data_view.subspan(fingerprint_attr_offset + kStunAttributeHeaderSize, 4)); + data.subspan(fingerprint_attr_offset + kStunAttributeHeaderSize, 4)); return ((fingerprint ^ STUN_FINGERPRINT_XOR_VALUE) == - ComputeCrc32(data, size - fingerprint_attr_size)); + ComputeCrc32(data.first(data.size() - fingerprint_attr_size))); } // static @@ -540,22 +532,18 @@ } bool StunMessage::IsStunMethod(std::span<int> methods, - const char* data, - size_t size) { + std::span<const uint8_t> data) { // Check the message length. - if (size % 4 != 0 || size < kStunHeaderSize) + if (data.size() % 4 != 0 || data.size() < kStunHeaderSize) return false; - std::span<const uint8_t> data_view(reinterpret_cast<const uint8_t*>(data), - size); - // Skip the rest if the magic cookie isn't present. size_t magic_cookie_offset = kStunTransactionIdOffset - kStunMagicCookieLength; - if (GetBE32(data_view.subspan(magic_cookie_offset, 4)) != kStunMagicCookie) + if (GetBE32(data.subspan(magic_cookie_offset, 4)) != kStunMagicCookie) return false; - int method = GetBE16(data_view); + int method = GetBE16(data); for (int m : methods) { if (m == method) { return true; @@ -588,7 +576,7 @@ bool StunMessage::Read(ByteBufferReader* buf) { // Keep a copy of the buffer data around for later verification. - buffer_.assign(reinterpret_cast<const char*>(buf->Data()), buf->Length()); + buffer_.assign(buf->DataView().begin(), buf->DataView().end()); if (!buf->ReadUInt16(&type_)) { return false; @@ -1117,10 +1105,9 @@ } StunByteStringAttribute::StunByteStringAttribute(uint16_t type, - const void* bytes, - size_t length) + std::span<const uint8_t> bytes) : StunAttribute(type, 0), bytes_(nullptr) { - CopyBytes(bytes, length); + CopyBytes(bytes); } StunByteStringAttribute::StunByteStringAttribute( @@ -1131,7 +1118,7 @@ for (const auto& value : values) { writer.WriteUInt32(value); } - CopyBytes(writer.Data(), writer.Length()); + CopyBytes(writer.DataView()); } StunByteStringAttribute::StunByteStringAttribute(uint16_t type, uint16_t length) @@ -1160,17 +1147,15 @@ } void StunByteStringAttribute::CopyBytes(absl::string_view bytes) { + CopyBytes(AsUint8Span(bytes)); +} + +void StunByteStringAttribute::CopyBytes(std::span<const uint8_t> bytes) { uint8_t* new_bytes = new uint8_t[bytes.size()]; memcpy(new_bytes, bytes.data(), bytes.size()); SetBytes(new_bytes, bytes.size()); } -void StunByteStringAttribute::CopyBytes(const void* bytes, size_t length) { - uint8_t* new_bytes = new uint8_t[length]; - memcpy(new_bytes, bytes, length); - SetBytes(new_bytes, length); -} - uint8_t StunByteStringAttribute::GetByte(size_t index) const { RTC_DCHECK(bytes_ != nullptr); RTC_DCHECK(index < length()); @@ -1419,14 +1404,14 @@ input += ':'; input += password; - char digest[MessageDigest::kMaxSize]; - size_t size = ComputeDigest(DIGEST_MD5, input.c_str(), input.size(), digest, - sizeof(digest)); + std::array<uint8_t, MessageDigest::kMaxSize> digest; + size_t size = ComputeDigest(DIGEST_MD5, AsUint8Span(input), digest); + if (size == 0) { return false; } - *hash = std::string(digest, size); + *hash = std::string(AsStringView(std::span(digest).first(size))); return true; }
diff --git a/api/transport/stun.h b/api/transport/stun.h index f9e0a8f..e12b01d 100644 --- a/api/transport/stun.h +++ b/api/transport/stun.h
@@ -23,12 +23,14 @@ #include <string> #include <vector> +#include "absl/base/macros.h" #include "absl/strings/string_view.h" #include "rtc_base/byte_buffer.h" #include "rtc_base/checks.h" #include "rtc_base/ip_address.h" #include "rtc_base/net_helpers.h" #include "rtc_base/socket_address.h" +#include "rtc_base/span_helpers.h" namespace webrtc { @@ -256,11 +258,20 @@ // Verify that a buffer has stun magic cookie and one of the specified // methods. Note that it does not check for the existance of FINGERPRINT. static bool IsStunMethod(std::span<int> methods, + std::span<const uint8_t> data); + ABSL_DEPRECATE_AND_INLINE() + static bool IsStunMethod(std::span<int> methods, const char* data, - size_t size); + size_t size) { + return IsStunMethod(methods, AsUint8Span(std::span(data, size))); + } // Verifies that a given buffer is STUN by checking for a correct FINGERPRINT. - static bool ValidateFingerprint(const char* data, size_t size); + static bool ValidateFingerprint(std::span<const uint8_t> data); + ABSL_DEPRECATE_AND_INLINE() + static bool ValidateFingerprint(const char* data, size_t size) { + return ValidateFingerprint(AsUint8Span(std::span(data, size))); + } // Generates a new 12 byte (RFC5389) transaction id. static std::string GenerateTransactionId(); @@ -295,13 +306,28 @@ std::function<bool(int type)> attribute_type_mask) const; // Expose raw-buffer ValidateMessageIntegrity function for testing. + static bool ValidateMessageIntegrityForTesting(const std::string& password, + std::span<const uint8_t> data); + ABSL_DEPRECATE_AND_INLINE() static bool ValidateMessageIntegrityForTesting(const char* data, size_t size, - const std::string& password); + const std::string& password) { + return ValidateMessageIntegrityForTesting( + password, AsUint8Span(std::span(data, size))); + } + // Expose raw-buffer ValidateMessageIntegrity function for testing. - static bool ValidateMessageIntegrity32ForTesting(const char* data, - size_t size, - const std::string& password); + static bool ValidateMessageIntegrity32ForTesting( + const std::string& password, + std::span<const uint8_t> data); + ABSL_DEPRECATE_AND_INLINE() + static bool ValidateMessageIntegrity32ForTesting( + const char* data, + size_t size, + const std::string& password) { + return ValidateMessageIntegrity32ForTesting( + password, AsUint8Span(std::span(data, size))); + } protected: // Verifies that the given attribute is allowed for this message. @@ -318,8 +344,7 @@ absl::string_view key); static bool ValidateMessageIntegrityOfType(int mi_attr_type, size_t mi_attr_size, - const char* data, - size_t size, + std::span<const uint8_t> data, const std::string& password); uint16_t type_ = STUN_INVALID_MESSAGE_TYPE; @@ -328,7 +353,7 @@ uint32_t reduced_transaction_id_ = 0; uint32_t stun_magic_cookie_ = kStunMagicCookie; // The original buffer for messages created by Read(). - std::string buffer_; + std::vector<uint8_t> buffer_; IntegrityStatus integrity_ = IntegrityStatus::kNotSet; std::string password_; }; @@ -506,7 +531,12 @@ public: explicit StunByteStringAttribute(uint16_t type); StunByteStringAttribute(uint16_t type, absl::string_view str); - StunByteStringAttribute(uint16_t type, const void* bytes, size_t length); + StunByteStringAttribute(uint16_t type, std::span<const uint8_t> bytes); + ABSL_DEPRECATE_AND_INLINE() + StunByteStringAttribute(uint16_t type, const void* bytes, size_t length) + : StunByteStringAttribute( + type, + AsUint8Span(std::span(static_cast<const char*>(bytes), length))) {} StunByteStringAttribute(uint16_t type, const std::vector<uint32_t>& values); StunByteStringAttribute(uint16_t type, uint16_t length); ~StunByteStringAttribute() override; @@ -531,7 +561,11 @@ std::optional<std::vector<uint32_t>> GetUInt32Vector() const; - void CopyBytes(const void* bytes, size_t length); + void CopyBytes(std::span<const uint8_t> bytes); + ABSL_DEPRECATE_AND_INLINE() + void CopyBytes(const void* bytes, size_t length) { + CopyBytes(AsUint8Span(std::span(static_cast<const char*>(bytes), length))); + } void CopyBytes(absl::string_view bytes); uint8_t GetByte(size_t index) const;
diff --git a/api/transport/stun_unittest.cc b/api/transport/stun_unittest.cc index 21d26ee..8da2425 100644 --- a/api/transport/stun_unittest.cc +++ b/api/transport/stun_unittest.cc
@@ -10,6 +10,8 @@ #include "api/transport/stun.h" +#include <algorithm> +#include <array> #include <cstdint> #include <cstring> #include <memory> @@ -23,6 +25,7 @@ #include "rtc_base/byte_order.h" #include "rtc_base/ip_address.h" #include "rtc_base/socket_address.h" +#include "rtc_base/span_helpers.h" #include "system_wrappers/include/metrics.h" #include "test/gmock.h" #include "test/gtest.h" @@ -30,6 +33,9 @@ namespace webrtc { namespace { + +using ::testing::ElementsAreArray; + // Sample STUN packets with various attributes // Gathered by wiresharking pjproject's pjnath test programs // pjproject available at www.pjsip.org @@ -222,21 +228,18 @@ // Software name (response): "test vector" (without quotes) // Username: "evtj:h6vY" (without quotes) // Password: "VOkJxbRl1RmTxUk/WvJxBt" (without quotes) -constexpr uint8_t kRfc5769SampleMsgTransactionId[] = { - 0xb7, 0xe7, 0xa7, 0x01, 0xbc, 0x34, 0xd6, 0x86, 0xfa, 0x87, 0xdf, 0xae -}; +constexpr auto kRfc5769SampleMsgTransactionId = std::to_array<uint8_t>( + {0xb7, 0xe7, 0xa7, 0x01, 0xbc, 0x34, 0xd6, 0x86, 0xfa, 0x87, 0xdf, 0xae}); constexpr char kRfc5769SampleMsgClientSoftware[] = "STUN test client"; constexpr char kRfc5769SampleMsgServerSoftware[] = "test vector"; constexpr char kRfc5769SampleMsgUsername[] = "evtj:h6vY"; constexpr char kRfc5769SampleMsgPassword[] = "VOkJxbRl1RmTxUk/WvJxBt"; -const SocketAddress kRfc5769SampleMsgMappedAddress( - "192.0.2.1", 32853); +const SocketAddress kRfc5769SampleMsgMappedAddress("192.0.2.1", 32853); const SocketAddress kRfc5769SampleMsgIPv6MappedAddress( "2001:db8:1234:5678:11:2233:4455:6677", 32853); -constexpr uint8_t kRfc5769SampleMsgWithAuthTransactionId[] = { - 0x78, 0xad, 0x34, 0x33, 0xc6, 0xad, 0x72, 0xc0, 0x29, 0xda, 0x41, 0x2e -}; +constexpr auto kRfc5769SampleMsgWithAuthTransactionId = std::to_array<uint8_t>( + {0x78, 0xad, 0x34, 0x33, 0xc6, 0xad, 0x72, 0xc0, 0x29, 0xda, 0x41, 0x2e}); constexpr char kRfc5769SampleMsgWithAuthUsername[] = "\xe3\x83\x9e\xe3\x83\x88\xe3\x83\xaa\xe3\x83\x83\xe3\x82\xaf\xe3\x82\xb9"; constexpr char kRfc5769SampleMsgWithAuthPassword[] = "TheMatrIX"; @@ -469,14 +472,12 @@ // A transaction ID without the 'magic cookie' portion // pjnat's test programs use this transaction ID a lot. -constexpr uint8_t kTestTransactionId1[] = {0x029, 0x01f, 0x0cd, 0x07c, - 0x0ba, 0x058, 0x0ab, 0x0d7, - 0x0f2, 0x041, 0x001, 0x000}; +constexpr auto kTestTransactionId1 = std::to_array<uint8_t>( + {0x29, 0x1f, 0xcd, 0x7c, 0xba, 0x58, 0xab, 0xd7, 0xf2, 0x41, 0x01, 0x00}); // They use this one sometimes too. -constexpr uint8_t kTestTransactionId2[] = {0x0e3, 0x0a9, 0x046, 0x0e1, - 0x07c, 0x000, 0x0c2, 0x062, - 0x054, 0x008, 0x001, 0x000}; +constexpr auto kTestTransactionId2 = std::to_array<uint8_t>( + {0xe3, 0xa9, 0x46, 0xe1, 0x7c, 0x00, 0xc2, 0x62, 0x54, 0x08, 0x01, 0x00}); const in6_addr kIPv6TestAddress1 = { {{0x24, 0x01, 0xfa, 0x00, 0x00, 0x04, 0x10, 0x00, 0xbe, 0x30, 0x5b, 0xff, @@ -502,6 +503,7 @@ constexpr int kTestMessagePort2 = 47233; constexpr int kTestMessagePort3 = 56743; constexpr int kTestMessagePort4 = 40444; + } // namespace class StunTest : public ::testing::Test { @@ -514,12 +516,12 @@ } void CheckStunTransactionID(const StunMessage& msg, - const uint8_t* expectedID, - size_t length) { - ASSERT_EQ(length, msg.transaction_id().size()); - ASSERT_EQ(length == kStunTransactionIdLength + 4, msg.IsLegacy()); - ASSERT_EQ(length == kStunTransactionIdLength, !msg.IsLegacy()); - ASSERT_EQ(0, memcmp(msg.transaction_id().c_str(), expectedID, length)); + std::span<const uint8_t> expectedID) { + ASSERT_EQ(expectedID.size(), msg.transaction_id().size()); + ASSERT_EQ(expectedID.size() == kStunTransactionIdLength + 4, + msg.IsLegacy()); + ASSERT_EQ(expectedID.size() == kStunTransactionIdLength, !msg.IsLegacy()); + EXPECT_THAT(msg.transaction_id(), ElementsAreArray(expectedID)); } void CheckStunAddressAttribute(const StunAddressAttribute* addr, @@ -544,19 +546,18 @@ } size_t ReadStunMessageTestCase(StunMessage* msg, - const uint8_t* testcase, - size_t size) { - ByteBufferReader buf(std::span(testcase, size)); + std::span<const uint8_t> data) { + ByteBufferReader buf(data); if (msg->Read(&buf)) { // Returns the size the stun message should report itself as being - return (size - 20); + return (data.size() - 20); } else { return 0; } } }; -#define ReadStunMessage(X, Y) ReadStunMessageTestCase(X, Y, sizeof(Y)); +#define ReadStunMessage(X, Y) ReadStunMessageTestCase(X, Y); // Test that the GetStun*Type and IsStun*Type methods work as expected. TEST_F(StunTest, MessageTypes) { @@ -586,7 +587,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kStunMessageWithIPv4MappedAddress); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_MAPPED_ADDRESS); IPAddress test_address(kIPv4TestAddress1); @@ -599,7 +600,7 @@ StunMessage msg2; size_t size = ReadStunMessage(&msg, kStunMessageWithIPv4XorMappedAddress); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); @@ -612,7 +613,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kStunMessageWithIPv6MappedAddress); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); IPAddress test_address(kIPv6TestAddress1); @@ -625,7 +626,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kStunMessageWithIPv6MappedAddress); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); IPAddress test_address(kIPv6TestAddress1); @@ -641,7 +642,7 @@ IPAddress test_address(kIPv6TestAddress1); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId2); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); @@ -654,9 +655,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kRfc5769SampleRequest); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId, - kStunTransactionIdLength); - + CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId); const StunByteStringAttribute* software = msg.GetByteString(STUN_ATTR_SOFTWARE); ASSERT_TRUE(software != nullptr); @@ -681,9 +680,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kRfc5769SampleResponse); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId, - kStunTransactionIdLength); - + CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId); const StunByteStringAttribute* software = msg.GetByteString(STUN_ATTR_SOFTWARE); ASSERT_TRUE(software != nullptr); @@ -704,9 +701,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kRfc5769SampleResponseIPv6); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId, - kStunTransactionIdLength); - + CheckStunTransactionID(msg, kRfc5769SampleMsgTransactionId); const StunByteStringAttribute* software = msg.GetByteString(STUN_ATTR_SOFTWARE); ASSERT_TRUE(software != nullptr); @@ -727,9 +722,7 @@ StunMessage msg; size_t size = ReadStunMessage(&msg, kRfc5769SampleRequestLongTermAuth); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kRfc5769SampleMsgWithAuthTransactionId, - kStunTransactionIdLength); - + CheckStunTransactionID(msg, kRfc5769SampleMsgWithAuthTransactionId); const StunByteStringAttribute* username = msg.GetByteString(STUN_ATTR_USERNAME); ASSERT_TRUE(username != nullptr); @@ -752,16 +745,15 @@ // kStunMessageWithIPv4MappedAddress, but with a different value where the // magic cookie was. TEST_F(StunTest, ReadLegacyMessage) { - uint8_t rfc3489_packet[sizeof(kStunMessageWithIPv4MappedAddress)]; - memcpy(rfc3489_packet, kStunMessageWithIPv4MappedAddress, - sizeof(kStunMessageWithIPv4MappedAddress)); + auto rfc3489_packet = std::to_array(kStunMessageWithIPv4MappedAddress); // Overwrite the magic cookie here. memcpy(&rfc3489_packet[4], "ABCD", 4); StunMessage msg; size_t size = ReadStunMessage(&msg, rfc3489_packet); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, &rfc3489_packet[4], kStunTransactionIdLength + 4); + CheckStunTransactionID( + msg, std::span(rfc3489_packet).subspan(4, kStunTransactionIdLength + 4)); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_MAPPED_ADDRESS); IPAddress test_address(kIPv4TestAddress1); @@ -776,7 +768,7 @@ IPAddress test_address(kIPv6TestAddress1); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId2); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); @@ -823,7 +815,7 @@ IPAddress test_address(kIPv4TestAddress1); CheckStunHeader(msg, STUN_BINDING_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); const StunAddressAttribute* addr = msg.GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); @@ -905,11 +897,8 @@ IPAddress test_ip(kIPv6TestAddress1); - StunMessage msg( - STUN_BINDING_REQUEST, - std::string(reinterpret_cast<const char*>(kTestTransactionId1), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_REQUEST, AsStringView(kTestTransactionId1)); + CheckStunTransactionID(msg, kTestTransactionId1); auto addr = StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS); SocketAddress test_addr(test_ip, kTestMessagePort2); @@ -933,11 +922,8 @@ IPAddress test_ip(kIPv4TestAddress1); - StunMessage msg( - STUN_BINDING_RESPONSE, - std::string(reinterpret_cast<const char*>(kTestTransactionId1), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_RESPONSE, AsStringView(kTestTransactionId1)); + CheckStunTransactionID(msg, kTestTransactionId1); auto addr = StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS); SocketAddress test_addr(test_ip, kTestMessagePort4); @@ -961,11 +947,8 @@ IPAddress test_ip(kIPv6TestAddress1); - StunMessage msg( - STUN_BINDING_RESPONSE, - std::string(reinterpret_cast<const char*>(kTestTransactionId2), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_RESPONSE, AsStringView(kTestTransactionId2)); + CheckStunTransactionID(msg, kTestTransactionId2); auto addr = StunAttribute::CreateXorAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); SocketAddress test_addr(test_ip, kTestMessagePort1); @@ -990,11 +973,8 @@ IPAddress test_ip(kIPv4TestAddress1); - StunMessage msg( - STUN_BINDING_RESPONSE, - std::string(reinterpret_cast<const char*>(kTestTransactionId1), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_RESPONSE, AsStringView(kTestTransactionId1)); + CheckStunTransactionID(msg, kTestTransactionId1); auto addr = StunAttribute::CreateXorAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); SocketAddress test_addr(test_ip, kTestMessagePort3); @@ -1019,7 +999,7 @@ size_t size = ReadStunMessage(&msg, kStunMessageWithByteStringAttribute); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId2); const StunByteStringAttribute* username = msg.GetByteString(STUN_ATTR_USERNAME); ASSERT_TRUE(username != nullptr); @@ -1032,7 +1012,7 @@ ReadStunMessage(&msg, kStunMessageWithPaddedByteStringAttribute); ASSERT_NE(0U, size); CheckStunHeader(msg, STUN_BINDING_REQUEST, size); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId2); const StunByteStringAttribute* username = msg.GetByteString(STUN_ATTR_USERNAME); ASSERT_TRUE(username != nullptr); @@ -1044,7 +1024,7 @@ size_t size = ReadStunMessage(&msg, kStunMessageWithErrorAttribute); CheckStunHeader(msg, STUN_BINDING_ERROR_RESPONSE, size); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + CheckStunTransactionID(msg, kTestTransactionId1); const StunErrorCodeAttribute* errorcode = msg.GetErrorCode(); ASSERT_TRUE(errorcode != nullptr); EXPECT_EQ(kTestErrorClass, errorcode->eclass()); @@ -1089,11 +1069,9 @@ TEST_F(StunTest, WriteMessageWithAnErrorCodeAttribute) { size_t size = sizeof(kStunMessageWithErrorAttribute); - StunMessage msg( - STUN_BINDING_ERROR_RESPONSE, - std::string(reinterpret_cast<const char*>(kTestTransactionId1), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId1, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_ERROR_RESPONSE, + AsStringView(kTestTransactionId1)); + CheckStunTransactionID(msg, kTestTransactionId1); auto errorcode = StunAttribute::CreateErrorCode(); errorcode->SetCode(kTestErrorCode); errorcode->SetReason(kTestErrorReason); @@ -1110,11 +1088,8 @@ TEST_F(StunTest, WriteMessageWithAUInt16ListAttribute) { size_t size = sizeof(kStunMessageWithUInt16ListAttribute); - StunMessage msg( - STUN_BINDING_REQUEST, - std::string(reinterpret_cast<const char*>(kTestTransactionId2), - kStunTransactionIdLength)); - CheckStunTransactionID(msg, kTestTransactionId2, kStunTransactionIdLength); + StunMessage msg(STUN_BINDING_REQUEST, AsStringView(kTestTransactionId2)); + CheckStunTransactionID(msg, kTestTransactionId2); auto list = StunAttribute::CreateUnknownAttributes(); list->AddType(0x1U); list->AddType(0x1000U); @@ -1155,25 +1130,19 @@ TEST_F(StunTest, ValidateMessageIntegrity) { // Try the messages from RFC 5769. EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleRequest), - sizeof(kRfc5769SampleRequest), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kRfc5769SampleRequest)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleRequest), - sizeof(kRfc5769SampleRequest), "InvalidPassword")); + "InvalidPassword", kRfc5769SampleRequest)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleResponse), - sizeof(kRfc5769SampleResponse), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kRfc5769SampleResponse)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleResponse), - sizeof(kRfc5769SampleResponse), "InvalidPassword")); + "InvalidPassword", kRfc5769SampleResponse)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleResponseIPv6), - sizeof(kRfc5769SampleResponseIPv6), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kRfc5769SampleResponseIPv6)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleResponseIPv6), - sizeof(kRfc5769SampleResponseIPv6), "InvalidPassword")); + "InvalidPassword", kRfc5769SampleResponseIPv6)); // We first need to compute the key for the long-term authentication HMAC. std::string key; @@ -1181,53 +1150,44 @@ kRfc5769SampleMsgWithAuthRealm, kRfc5769SampleMsgWithAuthPassword, &key); EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleRequestLongTermAuth), - sizeof(kRfc5769SampleRequestLongTermAuth), key)); + key, kRfc5769SampleRequestLongTermAuth)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kRfc5769SampleRequestLongTermAuth), - sizeof(kRfc5769SampleRequestLongTermAuth), "InvalidPassword")); + "InvalidPassword", kRfc5769SampleRequestLongTermAuth)); // Try some edge cases. EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithZeroLength), - sizeof(kStunMessageWithZeroLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithZeroLength)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithExcessLength), - sizeof(kStunMessageWithExcessLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithExcessLength)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithSmallLength), - sizeof(kStunMessageWithSmallLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithSmallLength)); // Again, but with the lengths matching what is claimed in the headers. + auto GetSubspan = [](std::span<const uint8_t> view) { + size_t claimed_len = kStunHeaderSize + GetBE16(view.subspan(2, 2)); + return view.subspan(0, std::min(claimed_len, view.size())); + }; EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithZeroLength), - kStunHeaderSize + GetBE16(kZeroLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kZeroLenView))); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithExcessLength), - kStunHeaderSize + GetBE16(kExcessLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kExcessLenView))); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithSmallLength), - kStunHeaderSize + GetBE16(kSmallLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kSmallLenView))); // Check that a too-short HMAC doesn't cause buffer overflow. EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(kStunMessageWithBadHmacAtEnd), - sizeof(kStunMessageWithBadHmacAtEnd), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithBadHmacAtEnd)); // Test that munging a single bit anywhere in the message causes the // message-integrity check to fail, unless it is after the M-I attribute. - char buf[sizeof(kRfc5769SampleRequest)]; - memcpy(buf, kRfc5769SampleRequest, sizeof(kRfc5769SampleRequest)); - for (size_t i = 0; i < sizeof(buf); ++i) { + auto buf = std::to_array(kRfc5769SampleRequest); + for (size_t i = 0; i < buf.size(); ++i) { buf[i] ^= 0x01; if (i > 0) buf[i - 1] ^= 0x01; - EXPECT_EQ(i >= sizeof(buf) - 8, + EXPECT_EQ(i >= buf.size() - 8, StunMessage::ValidateMessageIntegrityForTesting( - buf, sizeof(buf), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf)); } } @@ -1248,8 +1208,7 @@ ByteBufferWriter buf1; EXPECT_TRUE(msg.Write(&buf1)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf1.DataView())); IceMessage msg2; ByteBufferReader buf2(kRfc5769SampleResponseWithoutMI); @@ -1264,61 +1223,51 @@ ByteBufferWriter buf3; EXPECT_TRUE(msg2.Write(&buf3)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(buf3.Data()), buf3.Length(), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf3.DataView())); } // Check our STUN message validation code against the RFC5769 test messages. TEST_F(StunTest, ValidateMessageIntegrity32) { // Try the messages from RFC 5769. EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kSampleRequestMI32), - sizeof(kSampleRequestMI32), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kSampleRequestMI32)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kSampleRequestMI32), - sizeof(kSampleRequestMI32), "InvalidPassword")); + "InvalidPassword", kSampleRequestMI32)); // Try some edge cases. EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithZeroLength), - sizeof(kStunMessageWithZeroLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithZeroLength)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithExcessLength), - sizeof(kStunMessageWithExcessLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithExcessLength)); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithSmallLength), - sizeof(kStunMessageWithSmallLength), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithSmallLength)); // Again, but with the lengths matching what is claimed in the headers. + auto GetSubspan = [](std::span<const uint8_t> view) { + size_t claimed_len = kStunHeaderSize + GetBE16(view.subspan(2, 2)); + return view.subspan(0, std::min(claimed_len, view.size())); + }; EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithZeroLength), - kStunHeaderSize + GetBE16(kZeroLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kZeroLenView))); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithExcessLength), - kStunHeaderSize + GetBE16(kExcessLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kExcessLenView))); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithSmallLength), - kStunHeaderSize + GetBE16(kSmallLenView.subspan(2, 2)), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, GetSubspan(kSmallLenView))); // Check that a too-short HMAC doesn't cause buffer overflow. EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(kStunMessageWithBadHmacAtEnd), - sizeof(kStunMessageWithBadHmacAtEnd), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, kStunMessageWithBadHmacAtEnd)); // Test that munging a single bit anywhere in the message causes the // message-integrity check to fail, unless it is after the M-I attribute. - char buf[sizeof(kSampleRequestMI32)]; - memcpy(buf, kSampleRequestMI32, sizeof(kSampleRequestMI32)); - for (size_t i = 0; i < sizeof(buf); ++i) { + auto buf = std::to_array(kSampleRequestMI32); + for (size_t i = 0; i < buf.size(); ++i) { buf[i] ^= 0x01; if (i > 0) buf[i - 1] ^= 0x01; - EXPECT_EQ(i >= sizeof(buf) - 8, + EXPECT_EQ(i >= buf.size() - 8, StunMessage::ValidateMessageIntegrity32ForTesting( - buf, sizeof(buf), kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf)); } } @@ -1337,8 +1286,7 @@ ByteBufferWriter buf1; EXPECT_TRUE(msg.Write(&buf1)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf1.DataView())); IceMessage msg2; ByteBufferReader buf2(kRfc5769SampleResponseWithoutMI); @@ -1353,8 +1301,7 @@ ByteBufferWriter buf3; EXPECT_TRUE(msg2.Write(&buf3)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(buf3.Data()), buf3.Length(), - kRfc5769SampleMsgPassword)); + kRfc5769SampleMsgPassword, buf3.DataView())); } // Validate that the message validates if both MESSAGE-INTEGRITY-32 and @@ -1363,7 +1310,7 @@ TEST_F(StunTest, AddMessageIntegrity32AndMessageIntegrity) { IceMessage msg; auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("keso", sizeof("keso")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 's', 'o', '\0'})); msg.AddAttribute(std::move(attr)); msg.AddMessageIntegrity32("password1"); msg.AddMessageIntegrity("password2"); @@ -1371,51 +1318,38 @@ ByteBufferWriter buf1; EXPECT_TRUE(msg.Write(&buf1)); EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password1")); - EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password2")); + "password1", buf1.DataView())); + EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting("password2", + buf1.DataView())); EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password2")); + "password2", buf1.DataView())); EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password1")); + "password1", buf1.DataView())); } // Check our STUN message validation code against the RFC5769 test messages. TEST_F(StunTest, ValidateFingerprint) { - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kRfc5769SampleRequest), - sizeof(kRfc5769SampleRequest))); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kRfc5769SampleResponse), - sizeof(kRfc5769SampleResponse))); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kRfc5769SampleResponseIPv6), - sizeof(kRfc5769SampleResponseIPv6))); + EXPECT_TRUE(StunMessage::ValidateFingerprint(kRfc5769SampleRequest)); + EXPECT_TRUE(StunMessage::ValidateFingerprint(kRfc5769SampleResponse)); + EXPECT_TRUE(StunMessage::ValidateFingerprint(kRfc5769SampleResponseIPv6)); - EXPECT_FALSE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kStunMessageWithZeroLength), - sizeof(kStunMessageWithZeroLength))); - EXPECT_FALSE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kStunMessageWithExcessLength), - sizeof(kStunMessageWithExcessLength))); - EXPECT_FALSE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(kStunMessageWithSmallLength), - sizeof(kStunMessageWithSmallLength))); + EXPECT_FALSE(StunMessage::ValidateFingerprint(kStunMessageWithZeroLength)); + EXPECT_FALSE(StunMessage::ValidateFingerprint(kStunMessageWithExcessLength)); + EXPECT_FALSE(StunMessage::ValidateFingerprint(kStunMessageWithSmallLength)); // Test that munging a single bit anywhere in the message causes the // fingerprint check to fail. - char buf[sizeof(kRfc5769SampleRequest)]; - memcpy(buf, kRfc5769SampleRequest, sizeof(kRfc5769SampleRequest)); - for (size_t i = 0; i < sizeof(buf); ++i) { + auto buf = std::to_array(kRfc5769SampleRequest); + for (size_t i = 0; i < buf.size(); ++i) { buf[i] ^= 0x01; if (i > 0) buf[i - 1] ^= 0x01; - EXPECT_FALSE(StunMessage::ValidateFingerprint(buf, sizeof(buf))); + EXPECT_FALSE(StunMessage::ValidateFingerprint(buf)); } // Put them all back to normal and the check should pass again. - buf[sizeof(buf) - 1] ^= 0x01; - EXPECT_TRUE(StunMessage::ValidateFingerprint(buf, sizeof(buf))); + buf[buf.size() - 1] ^= 0x01; + EXPECT_TRUE(StunMessage::ValidateFingerprint(buf)); } TEST_F(StunTest, AddFingerprint) { @@ -1426,8 +1360,7 @@ ByteBufferWriter buf1; EXPECT_TRUE(msg.Write(&buf1)); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(buf1.Data()), buf1.Length())); + EXPECT_TRUE(StunMessage::ValidateFingerprint(buf1.DataView())); } // Test that we can remove attribute from a message. @@ -1439,7 +1372,7 @@ { auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("kes", sizeof("kes")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 's'})); msg.AddAttribute(std::move(attr)); } @@ -1448,22 +1381,21 @@ auto attr = msg.RemoveAttribute(STUN_ATTR_USERNAME); ASSERT_NE(attr, nullptr); EXPECT_EQ(attr->type(), STUN_ATTR_USERNAME); - EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(attr.get()) - ->string_view() - .data()); + EXPECT_EQ("kes", + static_cast<StunByteStringAttribute*>(attr.get())->string_view()); EXPECT_LT(msg.length(), len); } // Now add same attribute type twice. { auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("kes", sizeof("kes")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 's'})); msg.AddAttribute(std::move(attr)); } { auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("kenta", sizeof("kenta")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 'n', 't', 'a'})); msg.AddAttribute(std::move(attr)); } @@ -1472,9 +1404,8 @@ auto attr = msg.RemoveAttribute(STUN_ATTR_USERNAME); ASSERT_NE(attr, nullptr); EXPECT_EQ(attr->type(), STUN_ATTR_USERNAME); - EXPECT_STREQ("kenta", static_cast<StunByteStringAttribute*>(attr.get()) - ->string_view() - .data()); + EXPECT_EQ("kenta", + static_cast<StunByteStringAttribute*>(attr.get())->string_view()); } // Remove should remove the last added occurrence. @@ -1482,9 +1413,8 @@ auto attr = msg.RemoveAttribute(STUN_ATTR_USERNAME); ASSERT_NE(attr, nullptr); EXPECT_EQ(attr->type(), STUN_ATTR_USERNAME); - EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(attr.get()) - ->string_view() - .data()); + EXPECT_EQ("kes", + static_cast<StunByteStringAttribute*>(attr.get())->string_view()); } // Removing something that does exist should return nullptr. @@ -1496,7 +1426,7 @@ StunMessage msg; auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("kes", sizeof("kes")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 's'})); msg.AddAttribute(std::move(attr)); size_t len = msg.length(); @@ -1513,13 +1443,13 @@ for (auto buffer_ptr : buffer_ptrs) { { // Test StunByteStringAttribute. auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); - attr->CopyBytes("kes", sizeof("kes")); + attr->CopyBytes(std::to_array<uint8_t>({'k', 'e', 's'})); auto copy = CopyStunAttribute(*attr, buffer_ptr); ASSERT_EQ(copy->value_type(), STUN_VALUE_BYTE_STRING); - EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(copy.get()) - ->string_view() - .data()); + EXPECT_EQ( + "kes", + static_cast<StunByteStringAttribute*>(copy.get())->string_view()); } { // Test StunAddressAttribute. @@ -1681,8 +1611,7 @@ EXPECT_TRUE(msg.Write(&out)); ASSERT_EQ(size, out.Length()); - size_t read_size = ReadStunMessageTestCase( - &msg, reinterpret_cast<const uint8_t*>(out.Data()), out.Length()); + size_t read_size = ReadStunMessageTestCase(&msg, out.DataView()); ASSERT_EQ(read_size + 20, size); CheckStunHeader(msg, STUN_BINDING_REQUEST, read_size); const StunUInt16ListAttribute* types = @@ -1697,16 +1626,14 @@ TEST_F(StunTest, IsStunMethod) { int methods[] = {STUN_BINDING_REQUEST}; - EXPECT_TRUE(StunMessage::IsStunMethod( - methods, reinterpret_cast<const char*>(kRfc5769SampleRequest), - sizeof(kRfc5769SampleRequest))); + EXPECT_TRUE(StunMessage::IsStunMethod(methods, kRfc5769SampleRequest)); } TEST_F(StunTest, SizeRestrictionOnAttributes) { StunMessage msg(STUN_BINDING_REQUEST, "ABCDEFGHIJKL"); auto long_username = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); std::string long_string(509, 'x'); - long_username->CopyBytes(long_string.c_str(), long_string.size()); + long_username->CopyBytes(long_string); msg.AddAttribute(std::move(long_username)); ByteBufferWriter out; ASSERT_FALSE(msg.Write(&out)); @@ -1756,8 +1683,7 @@ // Taken from the RFC 5769 sample request. std::vector<uint32_t> expected_integrity_vector = { 0x9aeaa70c, 0xbfd8cb56, 0x781ef2b5, 0xb2d3f249, 0xc1b571a2}; - EXPECT_THAT(*integrity_vector, - ::testing::ElementsAreArray(expected_integrity_vector)); + EXPECT_THAT(*integrity_vector, ElementsAreArray(expected_integrity_vector)); } } // namespace webrtc
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 937dc57..81f4b17 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn
@@ -211,6 +211,7 @@ "../rtc_base:rtc_numerics", "../rtc_base:socket", "../rtc_base:socket_address", + "../rtc_base:span_helpers", "../rtc_base:stringutils", "../rtc_base:timeutils", "../rtc_base:weak_ptr", @@ -566,6 +567,7 @@ "../rtc_base:net_helpers", "../rtc_base:network", "../rtc_base:socket_address", + "../rtc_base:span_helpers", "../rtc_base:stringutils", "../rtc_base:weak_ptr", "../rtc_base/network:received_packet", @@ -622,7 +624,9 @@ "../rtc_base:network", "../rtc_base:socket", "../rtc_base:socket_address", + "../rtc_base:span_helpers", "../rtc_base/network:sent_packet", + "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/strings:string_view", ] @@ -857,6 +861,7 @@ "../rtc_base/network:sent_packet", "../system_wrappers:metrics", "//third_party/abseil-cpp/absl/algorithm:container", + "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/strings:string_view", @@ -949,6 +954,7 @@ "../rtc_base:macromagic", "../rtc_base:network_route", "../rtc_base:socket", + "../rtc_base:span_helpers", "../rtc_base:task_queue_for_test", "../rtc_base:timeutils", "../rtc_base/network:received_packet",
diff --git a/p2p/base/async_stun_tcp_socket.h b/p2p/base/async_stun_tcp_socket.h index c1e1924..a0150a89 100644 --- a/p2p/base/async_stun_tcp_socket.h +++ b/p2p/base/async_stun_tcp_socket.h
@@ -32,6 +32,9 @@ AsyncStunTCPSocket(const AsyncStunTCPSocket&) = delete; AsyncStunTCPSocket& operator=(const AsyncStunTCPSocket&) = delete; + using AsyncPacketSocket::Send; + using AsyncPacketSocket::SendTo; + int Send(const void* pv, size_t cb, const AsyncSocketPacketOptions& options) override;
diff --git a/p2p/base/async_stun_tcp_socket_unittest.cc b/p2p/base/async_stun_tcp_socket_unittest.cc index 7857cc3..e1d06b9 100644 --- a/p2p/base/async_stun_tcp_socket_unittest.cc +++ b/p2p/base/async_stun_tcp_socket_unittest.cc
@@ -10,12 +10,16 @@ #include "p2p/base/async_stun_tcp_socket.h" +#include <array> #include <cstddef> +#include <cstdint> #include <cstring> #include <list> #include <memory> +#include <span> #include <string> #include <utility> +#include <vector> #include "absl/memory/memory.h" #include "api/environment/environment.h" @@ -34,36 +38,49 @@ namespace webrtc { +using ::testing::ElementsAreArray; +using ::testing::IsEmpty; using ::testing::NotNull; +using ::testing::SizeIs; -static unsigned char kStunMessageWithZeroLength[] = { +static constexpr auto kStunMessageWithZeroLength = std::to_array<uint8_t>({ 0x00, 0x01, 0x00, 0x00, // length of 0 (last 2 bytes) 0x21, 0x12, 0xA4, 0x42, '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', -}; +}); -static unsigned char kTurnChannelDataMessageWithZeroLength[] = { - 0x40, 0x00, 0x00, 0x00, // length of 0 (last 2 bytes) -}; +static constexpr auto kTurnChannelDataMessageWithZeroLength = + std::to_array<uint8_t>({ + 0x40, 0x00, 0x00, 0x00, // length of 0 (last 2 bytes) + }); -static unsigned char kTurnChannelDataMessage[] = { +static constexpr auto kTurnChannelDataMessage = std::to_array<uint8_t>({ 0x40, 0x00, 0x00, 0x10, 0x21, 0x12, 0xA4, 0x42, '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', -}; +}); -static unsigned char kStunMessageWithInvalidLength[] = { +static auto kStunMessageWithInvalidLength = std::to_array<uint8_t>({ 0x00, 0x01, 0x00, 0x10, 0x21, 0x12, 0xA4, 0x42, '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', -}; +}); -static unsigned char kTurnChannelDataMessageWithInvalidLength[] = { +static auto kTurnChannelDataMessageWithInvalidLength = std::to_array<uint8_t>({ 0x80, 0x00, 0x00, 0x20, 0x21, 0x12, 0xA4, 0x42, '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', -}; +}); -static unsigned char kTurnChannelDataMessageWithOddLength[] = { - 0x40, 0x00, 0x00, 0x05, 0x21, 0x12, 0xA4, 0x42, '0', -}; +static constexpr auto kTurnChannelDataMessageWithOddLength = + std::to_array<uint8_t>({ + 0x40, + 0x00, + 0x00, + 0x05, + 0x21, + 0x12, + 0xA4, + 0x42, + '0', + }); static const SocketAddress kClientAddr("11.11.11.11", 0); static const SocketAddress kServerAddr("22.22.22.22", 0); @@ -114,8 +131,7 @@ void OnReadPacket(AsyncPacketSocket* /* socket */, const ReceivedIpPacket& packet) { recv_packets_.push_back( - std::string(reinterpret_cast<const char*>(packet.payload().data()), - packet.payload().size())); + std::vector<uint8_t>(packet.payload().begin(), packet.payload().end())); } void OnSentPacket(AsyncPacketSocket* /* socket */, @@ -132,22 +148,11 @@ }); } - bool Send(const void* data, size_t len) { + bool Send(std::span<const uint8_t> data) { AsyncSocketPacketOptions options; - int ret = - send_socket_->Send(reinterpret_cast<const char*>(data), len, options); + int ret = send_socket_->Send(data, options); vss_->ProcessMessagesUntilIdle(); - return (ret == static_cast<int>(len)); - } - - bool CheckData(const void* data, int len) { - bool ret = false; - if (!recv_packets_.empty()) { - std::string packet = recv_packets_.front(); - recv_packets_.pop_front(); - ret = (memcmp(data, packet.c_str(), len) == 0); - } - return ret; + return (ret == static_cast<int>(data.size())); } std::unique_ptr<VirtualSocketServer> vss_; @@ -155,164 +160,157 @@ std::unique_ptr<AsyncStunTCPSocket> send_socket_; std::unique_ptr<AsyncListenSocket> listen_socket_; std::unique_ptr<AsyncPacketSocket> recv_socket_; - std::list<std::string> recv_packets_; + std::list<std::vector<uint8_t>> recv_packets_; int sent_packets_ = 0; }; +static constexpr uint8_t kTurnChannelDataMarker = 0x40; +static constexpr size_t kMaxTurnPacketSize = 65539; +static constexpr size_t kMaxStunPacketSize = 65552; + // Testing a stun packet sent/recv properly. TEST_F(AsyncStunTCPSocketTest, TestSingleStunPacket) { - EXPECT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); - EXPECT_EQ(1u, recv_packets_.size()); - EXPECT_TRUE(CheckData(kStunMessageWithZeroLength, - sizeof(kStunMessageWithZeroLength))); + EXPECT_TRUE(Send(kStunMessageWithZeroLength)); + ASSERT_THAT(recv_packets_, SizeIs(1u)); + EXPECT_THAT(recv_packets_.front(), + ElementsAreArray(kStunMessageWithZeroLength)); } // Verify sending multiple packets. TEST_F(AsyncStunTCPSocketTest, TestMultipleStunPackets) { - EXPECT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); - EXPECT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); - EXPECT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); - EXPECT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); - EXPECT_EQ(4u, recv_packets_.size()); + EXPECT_TRUE(Send(kStunMessageWithZeroLength)); + EXPECT_TRUE(Send(kStunMessageWithZeroLength)); + EXPECT_TRUE(Send(kStunMessageWithZeroLength)); + EXPECT_TRUE(Send(kStunMessageWithZeroLength)); + ASSERT_THAT(recv_packets_, SizeIs(4u)); } TEST_F(AsyncStunTCPSocketTest, ProcessInputHandlesMultiplePackets) { send_socket_->RegisterReceivedPacketCallback( [&](AsyncPacketSocket* /* socket */, const ReceivedIpPacket& packet) { - recv_packets_.push_back( - std::string(reinterpret_cast<const char*>(packet.payload().data()), - packet.payload().size())); + recv_packets_.push_back(std::vector<uint8_t>(packet.payload().begin(), + packet.payload().end())); }); Buffer buffer; - buffer.AppendData(kStunMessageWithZeroLength, - sizeof(kStunMessageWithZeroLength)); + buffer.AppendData(kStunMessageWithZeroLength); // ChannelData message MUST be padded to // a multiple of four bytes. - const unsigned char kTurnChannelData[] = { - 0x40, 0x00, 0x00, 0x04, 0x21, 0x12, 0xA4, 0x42, - }; - buffer.AppendData(kTurnChannelData, sizeof(kTurnChannelData)); + static constexpr auto kTurnChannelData = std::to_array<uint8_t>({ + 0x40, + 0x00, + 0x00, + 0x04, + 0x21, + 0x12, + 0xA4, + 0x42, + }); + buffer.AppendData(kTurnChannelData); send_socket_->ProcessInput(buffer); - EXPECT_EQ(2u, recv_packets_.size()); - EXPECT_TRUE(CheckData(kStunMessageWithZeroLength, - sizeof(kStunMessageWithZeroLength))); - EXPECT_TRUE(CheckData(kTurnChannelData, sizeof(kTurnChannelData))); + ASSERT_THAT(recv_packets_, SizeIs(2u)); + EXPECT_THAT(recv_packets_.front(), + ElementsAreArray(kStunMessageWithZeroLength)); + recv_packets_.pop_front(); + EXPECT_THAT(recv_packets_.front(), ElementsAreArray(kTurnChannelData)); } // Verifying TURN channel data message with zero length. TEST_F(AsyncStunTCPSocketTest, TestTurnChannelDataWithZeroLength) { - EXPECT_TRUE(Send(kTurnChannelDataMessageWithZeroLength, - sizeof(kTurnChannelDataMessageWithZeroLength))); - EXPECT_EQ(1u, recv_packets_.size()); - EXPECT_TRUE(CheckData(kTurnChannelDataMessageWithZeroLength, - sizeof(kTurnChannelDataMessageWithZeroLength))); + EXPECT_TRUE(Send(kTurnChannelDataMessageWithZeroLength)); + ASSERT_THAT(recv_packets_, SizeIs(1u)); + EXPECT_THAT(recv_packets_.front(), + ElementsAreArray(kTurnChannelDataMessageWithZeroLength)); } // Verifying TURN channel data message. TEST_F(AsyncStunTCPSocketTest, TestTurnChannelData) { - EXPECT_TRUE(Send(kTurnChannelDataMessage, sizeof(kTurnChannelDataMessage))); - EXPECT_EQ(1u, recv_packets_.size()); - EXPECT_TRUE( - CheckData(kTurnChannelDataMessage, sizeof(kTurnChannelDataMessage))); + EXPECT_TRUE(Send(kTurnChannelDataMessage)); + ASSERT_THAT(recv_packets_, SizeIs(1u)); + EXPECT_THAT(recv_packets_.front(), ElementsAreArray(kTurnChannelDataMessage)); } // Verifying TURN channel messages which needs padding handled properly. TEST_F(AsyncStunTCPSocketTest, TestTurnChannelDataPadding) { - EXPECT_TRUE(Send(kTurnChannelDataMessageWithOddLength, - sizeof(kTurnChannelDataMessageWithOddLength))); - EXPECT_EQ(1u, recv_packets_.size()); - EXPECT_TRUE(CheckData(kTurnChannelDataMessageWithOddLength, - sizeof(kTurnChannelDataMessageWithOddLength))); + EXPECT_TRUE(Send(kTurnChannelDataMessageWithOddLength)); + ASSERT_THAT(recv_packets_, SizeIs(1u)); + EXPECT_THAT(recv_packets_.front(), + ElementsAreArray(kTurnChannelDataMessageWithOddLength)); } // Verifying stun message with invalid length. TEST_F(AsyncStunTCPSocketTest, TestStunInvalidLength) { - EXPECT_FALSE(Send(kStunMessageWithInvalidLength, - sizeof(kStunMessageWithInvalidLength))); - EXPECT_EQ(0u, recv_packets_.size()); + EXPECT_FALSE(Send(kStunMessageWithInvalidLength)); + ASSERT_THAT(recv_packets_, IsEmpty()); // Modify the message length to larger value. kStunMessageWithInvalidLength[2] = 0xFF; kStunMessageWithInvalidLength[3] = 0xFF; - EXPECT_FALSE(Send(kStunMessageWithInvalidLength, - sizeof(kStunMessageWithInvalidLength))); + EXPECT_FALSE(Send(kStunMessageWithInvalidLength)); // Modify the message length to smaller value. kStunMessageWithInvalidLength[2] = 0x00; kStunMessageWithInvalidLength[3] = 0x01; - EXPECT_FALSE(Send(kStunMessageWithInvalidLength, - sizeof(kStunMessageWithInvalidLength))); + EXPECT_FALSE(Send(kStunMessageWithInvalidLength)); } // Verifying TURN channel data message with invalid length. TEST_F(AsyncStunTCPSocketTest, TestTurnChannelDataWithInvalidLength) { - EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength, - sizeof(kTurnChannelDataMessageWithInvalidLength))); + EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength)); // Modify the length to larger value. kTurnChannelDataMessageWithInvalidLength[2] = 0xFF; kTurnChannelDataMessageWithInvalidLength[3] = 0xF0; - EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength, - sizeof(kTurnChannelDataMessageWithInvalidLength))); + EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength)); // Modify the length to smaller value. kTurnChannelDataMessageWithInvalidLength[2] = 0x00; kTurnChannelDataMessageWithInvalidLength[3] = 0x00; - EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength, - sizeof(kTurnChannelDataMessageWithInvalidLength))); + EXPECT_FALSE(Send(kTurnChannelDataMessageWithInvalidLength)); } // Verifying a small buffer handled (dropped) properly. This will be // a common one for both stun and turn. TEST_F(AsyncStunTCPSocketTest, TestTooSmallMessageBuffer) { - char data[1]; - EXPECT_FALSE(Send(data, sizeof(data))); + auto data = std::to_array<uint8_t>({0}); + EXPECT_FALSE(Send(data)); } // Verifying a legal large turn message. TEST_F(AsyncStunTCPSocketTest, TestMaximumSizeTurnPacket) { - unsigned char packet[65539]; - packet[0] = 0x40; + std::vector<uint8_t> packet(kMaxTurnPacketSize, 0); + packet[0] = kTurnChannelDataMarker; packet[1] = 0x00; packet[2] = 0xFF; packet[3] = 0xFF; - EXPECT_TRUE(Send(packet, sizeof(packet))); + EXPECT_TRUE(Send(packet)); } // Verifying a legal large stun message. TEST_F(AsyncStunTCPSocketTest, TestMaximumSizeStunPacket) { - unsigned char packet[65552]; + std::vector<uint8_t> packet(kMaxStunPacketSize, 0); packet[0] = 0x00; packet[1] = 0x01; packet[2] = 0xFF; packet[3] = 0xFC; - EXPECT_TRUE(Send(packet, sizeof(packet))); + EXPECT_TRUE(Send(packet)); } // Test that a turn message is sent completely even if it exceeds the socket // send buffer capacity. TEST_F(AsyncStunTCPSocketTest, TestWithSmallSendBuffer) { vss_->set_send_buffer_capacity(1); - Send(kTurnChannelDataMessageWithOddLength, - sizeof(kTurnChannelDataMessageWithOddLength)); - EXPECT_EQ(1u, recv_packets_.size()); - EXPECT_TRUE(CheckData(kTurnChannelDataMessageWithOddLength, - sizeof(kTurnChannelDataMessageWithOddLength))); + Send(kTurnChannelDataMessageWithOddLength); + ASSERT_THAT(recv_packets_, SizeIs(1u)); + EXPECT_THAT(recv_packets_.front(), + ElementsAreArray(kTurnChannelDataMessageWithOddLength)); } // Test that SignalSentPacket is fired when a packet is sent. TEST_F(AsyncStunTCPSocketTest, SignalSentPacketFiredWhenPacketSent) { - ASSERT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); + ASSERT_TRUE(Send(kStunMessageWithZeroLength)); EXPECT_EQ(1, sent_packets_); // Send another packet for good measure. - ASSERT_TRUE( - Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength))); + ASSERT_TRUE(Send(kStunMessageWithZeroLength)); EXPECT_EQ(2, sent_packets_); } @@ -321,8 +319,8 @@ TEST_F(AsyncStunTCPSocketTest, SignalSentPacketNotFiredWhenPacketNotSent) { // Attempt to send a packet that's too small; since it isn't sent, // SignalSentPacket shouldn't fire. - char data[1]; - ASSERT_FALSE(Send(data, sizeof(data))); + auto data = std::to_array<uint8_t>({0}); + ASSERT_FALSE(Send(data)); EXPECT_EQ(0, sent_packets_); }
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc index 6bb2e57..a3c9b7a 100644 --- a/p2p/base/connection.cc +++ b/p2p/base/connection.cc
@@ -248,8 +248,8 @@ pruned_(false), use_candidate_attr_(false), requests_(port_->thread(), - [this](const void* data, size_t size, StunRequest* request) { - OnSendStunPacket(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + OnSendStunPacket(data, request); }), rtt_(kDefaultRtt), last_ping_sent_(Timestamp::Zero()), @@ -457,15 +457,13 @@ rtt_estimate_.SetHalfTime(field_trials->rtt_estimate_halftime_ms); } -void Connection::OnSendStunPacket(const void* data, - size_t size, +void Connection::OnSendStunPacket(std::span<const uint8_t> data, StunRequest* req) { RTC_DCHECK_RUN_ON(network_thread_); AsyncSocketPacketOptions options(port_->StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kIceConnectivityCheck; - auto err = - port_->SendTo(data, size, remote_candidate_.address(), options, false); + auto err = port_->SendTo(data, remote_candidate_.address(), options, false); if (err < 0) { RTC_LOG(LS_WARNING) << ToString() << ": Failed to send STUN ping " @@ -487,19 +485,12 @@ received_packet_callback_ = nullptr; } -void Connection::OnReadPacket(const char* data, - size_t size, - int64_t packet_time_us) { - OnReadPacket(ReceivedIpPacket::CreateFromLegacy(data, size, packet_time_us)); -} void Connection::OnReadPacket(const ReceivedIpPacket& packet) { RTC_DCHECK_RUN_ON(network_thread_); std::unique_ptr<IceMessage> msg; std::string remote_ufrag; const SocketAddress& addr(remote_candidate_.address()); - if (!port_->GetStunMessage( - reinterpret_cast<const char*>(packet.payload().data()), - packet.payload().size(), addr, &msg, &remote_ufrag)) { + if (!port_->GetStunMessage(packet.payload(), addr, &msg, &remote_ufrag)) { // The packet did not parse as a valid STUN message // This is a data packet, pass it along. last_data_received_ = env_.clock().CurrentTime(); @@ -875,7 +866,7 @@ AsyncSocketPacketOptions options(port_->StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kIceConnectivityCheckResponse; - auto err = port_->SendTo(buf.Data(), buf.Length(), addr, options, false); + auto err = port_->SendTo(buf.DataView(), addr, options, false); if (err < 0) { RTC_LOG(LS_ERROR) << ToString() << ": Failed to send " << StunMethodToString(response.type()) @@ -1911,22 +1902,20 @@ const Candidate& remote_candidate) : Connection(env, std::move(port), index, remote_candidate) {} -int ProxyConnection::Send(const void* data, - size_t size, +int ProxyConnection::Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) { RTC_DCHECK(port() != nullptr) << ToDebugId() << ": port_ null in Send()"; if (port() == nullptr) return SOCKET_ERROR; mutable_stats().sent_total_packets++; - int sent = - port()->SendTo(data, size, remote_candidate().address(), options, true); + int sent = port()->SendTo(data, remote_candidate().address(), options, true); Timestamp now = env().clock().CurrentTime(); if (sent <= 0) { RTC_DCHECK(sent < 0); error_ = port()->GetError(); mutable_stats().sent_discarded_packets++; - mutable_stats().sent_discarded_bytes += size; + mutable_stats().sent_discarded_bytes += data.size(); } else { AddSentBytesToStats(sent, now); }
diff --git a/p2p/base/connection.h b/p2p/base/connection.h index fae6b28..32f330c 100644 --- a/p2p/base/connection.h +++ b/p2p/base/connection.h
@@ -16,10 +16,12 @@ #include <functional> #include <memory> #include <optional> +#include <span> #include <string> #include <utility> #include <vector> +#include "absl/base/macros.h" #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/candidate.h" @@ -46,6 +48,7 @@ #include "rtc_base/network.h" #include "rtc_base/network/received_packet.h" #include "rtc_base/numerics/event_based_exponential_moving_average.h" +#include "rtc_base/span_helpers.h" #include "rtc_base/system/rtc_export.h" #include "rtc_base/thread_annotations.h" #include "rtc_base/weak_ptr.h" @@ -159,10 +162,17 @@ // The connection can send and receive packets asynchronously. This matches // the interface of AsyncPacketSocket, which may use UDP or TCP under the // covers. - virtual int Send(const void* data, - size_t size, + virtual int Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) = 0; + ABSL_DEPRECATE_AND_INLINE() + int Send(const void* data, + size_t size, + const AsyncSocketPacketOptions& options) { + return Send(AsUint8Span(std::span(static_cast<const char*>(data), size)), + options); + } + // Error if Send() returns < 0 virtual int GetError() = 0; @@ -184,9 +194,12 @@ } // Called when a packet is received on this connection. void OnReadPacket(const ReceivedIpPacket& packet); - [[deprecated("Pass a ReceivedIpPacket")]] void - OnReadPacket(const char* data, size_t size, int64_t packet_time_us); + ABSL_DEPRECATE_AND_INLINE() + void OnReadPacket(const char* data, size_t size, int64_t packet_time_us) { + OnReadPacket( + ReceivedIpPacket::CreateFromLegacy(data, size, packet_time_us)); + } // Called when the socket is currently able to send. void OnReadyToSend(); @@ -425,8 +438,7 @@ const Candidate& candidate); // Called back when StunRequestManager has a stun packet to send - void OnSendStunPacket(const void* data, size_t size, StunRequest* req); - + void OnSendStunPacket(std::span<const uint8_t> data, StunRequest* req); // Callbacks from ConnectionRequest virtual void OnConnectionRequestResponse(StunRequest* req, StunMessage* response); @@ -613,8 +625,7 @@ size_t index, const Candidate& remote_candidate); - int Send(const void* data, - size_t size, + int Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) override; int GetError() override;
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 76b5006..4edd58d 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc
@@ -1645,7 +1645,8 @@ last_sent_packet_id_ = options.packet_id; AsyncSocketPacketOptions modified_options(options); modified_options.info_signaled_after_sent.packet_type = PacketType::kData; - int sent = selected_connection_->Send(data, len, modified_options); + int sent = selected_connection_->Send( + std::span(reinterpret_cast<const uint8_t*>(data), len), modified_options); if (sent <= 0) { RTC_DCHECK(sent < 0); error_ = selected_connection_->GetError();
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 8b596e0..1df544f 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -89,7 +89,6 @@ #include "rtc_base/socket_address.h" #include "rtc_base/socket_server.h" #include "rtc_base/thread.h" -#include "rtc_base/time_utils.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" #include "test/create_test_environment.h" @@ -977,8 +976,7 @@ const ReceivedIpPacket& packet) { std::list<std::string>& packets = GetPacketList(transport); packets.push_front( - std::string(reinterpret_cast<const char*>(packet.payload().data()), - packet.payload().size())); + std::string(packet.payload().begin(), packet.payload().end())); } void OnRoleConflict(IceTransportInternal* channel) { @@ -3597,8 +3595,7 @@ msg.AddFingerprint(); ByteBufferWriter buf; msg.Write(&buf); - conn->OnReadPacket(ReceivedIpPacket::CreateFromLegacy( - reinterpret_cast<const char*>(buf.Data()), buf.Length(), TimeMicros())); + conn->OnReadPacket(ReceivedIpPacket(buf.DataView(), SocketAddress())); } void ReceivePingOnConnection(Connection* conn,
diff --git a/p2p/base/port.cc b/p2p/base/port.cc index f3200dd..500de62 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc
@@ -49,6 +49,7 @@ #include "rtc_base/network/received_packet.h" #include "rtc_base/network/sent_packet.h" #include "rtc_base/socket_address.h" +#include "rtc_base/span_helpers.h" #include "rtc_base/string_encode.h" #include "rtc_base/string_utils.h" #include "rtc_base/strings/string_builder.h" @@ -382,12 +383,11 @@ void Port::OnReadPacket(const ReceivedIpPacket& packet, ProtocolType proto) { RTC_DCHECK_RUN_ON(thread_); - const char* data = reinterpret_cast<const char*>(packet.payload().data()); - size_t size = packet.payload().size(); const SocketAddress& addr = packet.source_address(); + std::span<const uint8_t> data = packet.payload(); // If the user has enabled port packets, just hand this over. if (enable_port_packets_) { - NotifyReadPacket(this, data, size, addr); + NotifyReadPacket(this, data, addr); return; } @@ -395,7 +395,7 @@ // send back a proper binding response. std::unique_ptr<IceMessage> msg; std::string remote_username; - if (!GetStunMessage(data, size, addr, &msg, &remote_username)) { + if (!GetStunMessage(data, addr, &msg, &remote_username)) { RTC_LOG(LS_ERROR) << ToString() << ": Received non-STUN packet from unknown address: " << addr.ToSensitiveString(); @@ -449,8 +449,7 @@ candidates_.push_back(local); } -bool Port::GetStunMessage(const char* data, - size_t size, +bool Port::GetStunMessage(std::span<const uint8_t> data, const SocketAddress& addr, std::unique_ptr<IceMessage>* out_msg, std::string* out_username) { @@ -467,15 +466,15 @@ // Except GOOG_PING_REQUEST/RESPONSE that does not send fingerprint. int types[] = {GOOG_PING_REQUEST, GOOG_PING_RESPONSE, GOOG_PING_ERROR_RESPONSE}; - if (!StunMessage::IsStunMethod(types, data, size) && - !StunMessage::ValidateFingerprint(data, size)) { + if (!StunMessage::IsStunMethod(types, data) && + !StunMessage::ValidateFingerprint(data)) { return false; } // Parse the request message. If the packet is not a complete and correct // STUN message, then ignore it. std::unique_ptr<IceMessage> stun_msg(new IceMessage()); - ByteBufferReader buf(std::span(reinterpret_cast<const uint8_t*>(data), size)); + ByteBufferReader buf(data); if (!stun_msg->Read(&buf) || (buf.Length() > 0)) { return false; } @@ -800,7 +799,7 @@ AsyncSocketPacketOptions options(StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kIceConnectivityCheckResponse; - SendTo(buf.Data(), buf.Length(), addr, options, false); + SendTo(buf.DataView(), addr, options, false); RTC_LOG(LS_INFO) << ToString() << ": Sending STUN " << StunMethodToString(response.type()) << ": reason=" << reason << " to " @@ -838,7 +837,7 @@ AsyncSocketPacketOptions options(StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kIceConnectivityCheckResponse; - SendTo(buf.Data(), buf.Length(), addr, options, false); + SendTo(buf.DataView(), addr, options, false); RTC_LOG(LS_ERROR) << ToString() << ": Sending STUN binding error: reason=" << STUN_ERROR_UNKNOWN_ATTRIBUTE << " to " << addr.ToSensitiveString(); @@ -1115,29 +1114,35 @@ unknown_address_callbacks_.Send(port, address, proto, msg, rf, port_muxed); } -[[deprecated]] void Port::SubscribeReadPacket( +// deprecated +void Port::SubscribeReadPacket( absl::AnyInvocable< void(PortInterface*, const char*, size_t, const SocketAddress&)> callback) { - read_packet_callbacks_.AddReceiver(std::move(callback)); + SubscribeReadPacket( + nullptr, [cb = std::move(callback)](PortInterface* port, + std::span<const uint8_t> data, + const SocketAddress& addr) mutable { + cb(port, AsCharSpan(data).data(), data.size(), addr); + }); } void Port::SubscribeReadPacket( const void* tag, - absl::AnyInvocable< - void(PortInterface*, const char*, size_t, const SocketAddress&)> - callback) { + absl::AnyInvocable<void(PortInterface*, + std::span<const uint8_t>, + const SocketAddress&)> callback) { read_packet_callbacks_.AddReceiver(tag, std::move(callback)); } void Port::NotifyReadPacket(PortInterface* port, - const char* data, - size_t size, + std::span<const uint8_t> data, const SocketAddress& remote_address) { - read_packet_callbacks_.Send(port, data, size, remote_address); + read_packet_callbacks_.Send(port, data, remote_address); } [[deprecated]] void Port::SubscribeSentPacket( + absl::AnyInvocable<void(const SentPacketInfo&)> callback) { sent_packet_callbacks_.AddReceiver(std::move(callback)); }
diff --git a/p2p/base/port.h b/p2p/base/port.h index 8fe4b00..871e13d 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h
@@ -18,6 +18,7 @@ #include <memory> #include <optional> #include <set> +#include <span> #include <string> #include <utility> #include <vector> @@ -189,6 +190,13 @@ IceRole GetIceRole() const override; void SetIceRole(IceRole role) override; + /* + int SendTo(std::span<const uint8_t> data, + const SocketAddress& addr, + const AsyncSocketPacketOptions& options, + bool payload) override = 0; + */ + void SetIceTiebreaker(uint64_t tiebreaker) override; uint64_t IceTiebreaker() const override; @@ -434,19 +442,21 @@ const std::string& rf, bool port_muxed) override; - [[deprecated("Use SubscribeReadPacket(const void* tag, ...)")]] - void SubscribeReadPacket( + // This function causes strange linker behavior if it's inlined, + // otherwise it would have been ABSL_DEPRECATE_AND_INLINE. + [[deprecated("Use tagged version with span")]] void SubscribeReadPacket( absl::AnyInvocable< void(PortInterface*, const char*, size_t, const SocketAddress&)> callback) override; + void SubscribeReadPacket( const void* tag, - absl::AnyInvocable< - void(PortInterface*, const char*, size_t, const SocketAddress&)> - callback) override; + absl::AnyInvocable<void(PortInterface*, + std::span<const uint8_t>, + const SocketAddress&)> callback) override; + void NotifyReadPacket(PortInterface* prot, - const char* data, - size_t size, + std::span<const uint8_t> data, const SocketAddress& remote_address) override; [[deprecated("Use SubscribeSentPacket(const void* tag, ...)")]] @@ -497,12 +507,10 @@ // with this port's username fragment, msg will contain the parsed STUN // message. Otherwise, the function may send a STUN response internally. // remote_username contains the remote fragment of the STUN username. - bool GetStunMessage(const char* data, - size_t size, + bool GetStunMessage(std::span<const uint8_t> data, const SocketAddress& addr, std::unique_ptr<IceMessage>* out_msg, std::string* out_username) override; - // Checks if the address in addr is compatible with the port's ip. bool IsCompatibleAddress(const SocketAddress& addr); @@ -622,7 +630,7 @@ const std::string&, bool> unknown_address_callbacks_; - CallbackList<PortInterface*, const char*, size_t, const SocketAddress&> + CallbackList<PortInterface*, std::span<const uint8_t>, const SocketAddress&> read_packet_callbacks_; CallbackList<const SentPacketInfo&> sent_packet_callbacks_;
diff --git a/p2p/base/port_interface.h b/p2p/base/port_interface.h index 0f9d76a..6ff4c4c 100644 --- a/p2p/base/port_interface.h +++ b/p2p/base/port_interface.h
@@ -16,9 +16,12 @@ #include <functional> #include <memory> #include <optional> +#include <span> #include <string> +#include <utility> #include <vector> +#include "absl/base/macros.h" #include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/candidate.h" @@ -32,6 +35,7 @@ #include "rtc_base/network/sent_packet.h" #include "rtc_base/socket.h" #include "rtc_base/socket_address.h" +#include "rtc_base/span_helpers.h" namespace webrtc { @@ -48,7 +52,7 @@ virtual ~PortInterface(); virtual IceCandidateType Type() const = 0; - virtual const ::webrtc::Network* Network() const = 0; + virtual const Network* Network() const = 0; // Methods to set/get ICE role and tiebreaker values. virtual void SetIceRole(IceRole role) = 0; @@ -87,11 +91,33 @@ // Sends the given packet to the given address, provided that the address is // that of a connection or an address that has sent to us already. - virtual int SendTo(const void* data, - size_t size, + virtual int SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, - bool payload) = 0; + bool payload) { + // This function has a default implementation in order to support + // downstream code that has subclasses with the old SendTo method. + // If a subclass implements neither, this will cause a recursion, + // which should be easy to detect. + // TODO: bugs.webrtc.org/42225170 - make pure virtual when the function + // below has been removed. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + return SendTo(data.data(), data.size(), addr, options, payload); +#pragma clang diagnostic pop + } + + // This function is not marked ABSL_DEPRECATE_AND_INLINE() because + // blindly inlining it will lead to unexpected behavior in subclasses. + [[deprecated("Use version with span")]] virtual int SendTo( + const void* data, + size_t size, + const SocketAddress& addr, + const AsyncSocketPacketOptions& options, + bool payload) { + return SendTo(AsUint8Span(std::span(static_cast<const char*>(data), size)), + addr, options, payload); + } // Indicates that we received a successful STUN binding request from an // address that doesn't correspond to any current connection. To turn this @@ -143,20 +169,36 @@ // unknown address). Calling this method turns off delivery of packets // through this port. virtual void EnablePortPackets() = 0; - [[deprecated("Use SubscribeReadPacket(const void* tag, ...)")]] + virtual void SubscribeReadPacket( + const void* tag, + absl::AnyInvocable<void(PortInterface*, + std::span<const uint8_t>, + const SocketAddress&)> callback) = 0; + + ABSL_DEPRECATE_AND_INLINE() virtual void SubscribeReadPacket( absl::AnyInvocable< void(PortInterface*, const char*, size_t, const SocketAddress&)> - callback) = 0; + callback) { + SubscribeReadPacket(nullptr, std::move(callback)); + } + + ABSL_DEPRECATE_AND_INLINE() virtual void SubscribeReadPacket( const void* tag, absl::AnyInvocable< void(PortInterface*, const char*, size_t, const SocketAddress&)> - callback) = 0; + callback) { + SubscribeReadPacket( + tag, [cb = std::move(callback)](PortInterface* port, + std::span<const uint8_t> data, + const SocketAddress& addr) mutable { + cb(port, AsCharSpan(data).data(), data.size(), addr); + }); + } virtual void NotifyReadPacket(PortInterface* port_interface, - const char*, - size_t, - const SocketAddress&) = 0; + std::span<const uint8_t> data, + const SocketAddress& addr) = 0; // Emitted each time a packet is sent on this port. [[deprecated("Use SubscribeSentPacket(const void* tag, ...)")]] @@ -208,8 +250,7 @@ // with this port's username fragment, msg will contain the parsed STUN // message. Otherwise, the function may send a STUN response internally. // remote_username contains the remote fragment of the STUN username. - virtual bool GetStunMessage(const char* data, - size_t size, + virtual bool GetStunMessage(std::span<const uint8_t> data, const SocketAddress& addr, std::unique_ptr<IceMessage>* out_msg, std::string* out_username) = 0;
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index 93ad08c..7ebe8dc 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc
@@ -10,12 +10,14 @@ #include "p2p/base/port.h" +#include <array> #include <cstddef> #include <cstdint> #include <cstring> #include <list> #include <memory> #include <optional> +#include <span> #include <string> #include <utility> #include <vector> @@ -106,7 +108,7 @@ constexpr int kTiebreaker2 = 22222; constexpr int kTiebreakerDefault = 44444; -constexpr char kTestData[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; +constexpr uint8_t kTestData[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; Candidate GetCandidate(Port* port) { RTC_DCHECK_GE(port->Candidates().size(), 1); @@ -136,10 +138,8 @@ const SocketAddress& addr, std::unique_ptr<IceMessage>* out_msg, std::string* out_username) { - return port->GetStunMessage(reinterpret_cast<const char*>(buf->Data()), - buf->Length(), addr, out_msg, out_username); + return port->GetStunMessage(buf->DataView(), addr, out_msg, out_username); } - void SendPingAndReceiveResponse(Connection* lconn, TestPort* lport, Connection* rconn, @@ -243,9 +243,9 @@ void OnPortComplete(Port* port) { complete_count_++; } void SetIceMode(IceMode ice_mode) { ice_mode_ = ice_mode; } - int SendData(const char* data, size_t len) { + int SendData(std::span<const uint8_t> data) { AsyncSocketPacketOptions options; - return conn_->Send(data, len, options); + return conn_->Send(data, options); } void OnUnknownAddress(PortInterface* port, @@ -714,8 +714,7 @@ if (send_after_disconnected) { // First SendData after disconnect should fail but will trigger // reconnect. - EXPECT_EQ(-1, - ch1.SendData(kTestData, static_cast<int>(strlen(kTestData)))); + EXPECT_EQ(-1, ch1.SendData(kTestData)); } if (ping_after_disconnected) { @@ -1108,8 +1107,7 @@ std::unique_ptr<AsyncPacketSocket> next_client_tcp_socket) { next_client_tcp_socket_ = std::move(next_client_tcp_socket); } - std::unique_ptr<webrtc::AsyncDnsResolverInterface> CreateAsyncDnsResolver() - override { + std::unique_ptr<AsyncDnsResolverInterface> CreateAsyncDnsResolver() override { return nullptr; } @@ -1649,9 +1647,7 @@ lport->Reset(); auto buf = std::make_unique<ByteBufferWriter>(); WriteStunMessage(*modified_req, buf.get()); - conn1->OnReadPacket(ReceivedIpPacket::CreateFromLegacy( - reinterpret_cast<const char*>(buf->Data()), buf->Length(), - /*packet_time_us=*/-1)); + conn1->OnReadPacket(ReceivedIpPacket(buf->DataView(), SocketAddress())); ASSERT_THAT( WaitUntil([&] { return lport->last_stun_msg(); }, NotNull(), {.timeout = kDefaultTimeout, .clock = &time_controller_}), @@ -2033,9 +2029,7 @@ EXPECT_TRUE(msg->GetByteString(STUN_ATTR_ICE_CONTROLLED) == nullptr); EXPECT_TRUE(msg->GetByteString(STUN_ATTR_USE_CANDIDATE) != nullptr); EXPECT_TRUE(msg->GetUInt32(STUN_ATTR_FINGERPRINT) != nullptr); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(lport->last_stun_buf().data()), - lport->last_stun_buf().size())); + EXPECT_TRUE(StunMessage::ValidateFingerprint(lport->last_stun_buf())); // Request should not include ping count. ASSERT_TRUE(msg->GetUInt32(STUN_ATTR_RETRANSMIT_COUNT) == nullptr); @@ -2069,9 +2063,7 @@ EXPECT_EQ(StunMessage::IntegrityStatus::kIntegrityOk, msg->ValidateMessageIntegrity("rpass")); EXPECT_TRUE(msg->GetUInt32(STUN_ATTR_FINGERPRINT) != nullptr); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(lport->last_stun_buf().data()), - lport->last_stun_buf().size())); + EXPECT_TRUE(StunMessage::ValidateFingerprint(lport->last_stun_buf())); // No USERNAME or PRIORITY in ICE responses. EXPECT_TRUE(msg->GetByteString(STUN_ATTR_USERNAME) == nullptr); EXPECT_TRUE(msg->GetByteString(STUN_ATTR_PRIORITY) == nullptr); @@ -2100,9 +2092,7 @@ EXPECT_EQ(StunMessage::IntegrityStatus::kIntegrityOk, msg->ValidateMessageIntegrity("rpass")); EXPECT_TRUE(msg->GetUInt32(STUN_ATTR_FINGERPRINT) != nullptr); - EXPECT_TRUE(StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(lport->last_stun_buf().data()), - lport->last_stun_buf().size())); + EXPECT_TRUE(StunMessage::ValidateFingerprint(lport->last_stun_buf())); // No USERNAME with ICE. EXPECT_TRUE(msg->GetByteString(STUN_ATTR_USERNAME) == nullptr); EXPECT_TRUE(msg->GetByteString(STUN_ATTR_PRIORITY) == nullptr); @@ -2700,9 +2690,7 @@ modified_response->AddFingerprint(); ByteBufferWriter buf; WriteStunMessage(*modified_response, &buf); - lconn->OnReadPacket(ReceivedIpPacket::CreateFromLegacy( - reinterpret_cast<const char*>(buf.Data()), buf.Length(), - /*packet_time_us=*/-1)); + lconn->OnReadPacket(ReceivedIpPacket(buf.DataView(), SocketAddress())); // Response should have been ignored, leaving us unwritable still. EXPECT_FALSE(lconn->writable()); } @@ -3152,10 +3140,9 @@ IsRtcOk()); // Data should be sendable before the connection is accepted. - char data[] = "abcd"; - int data_size = std::ssize(data); + auto data = std::to_array<uint8_t>({'a', 'b', 'c', 'd', '\0'}); AsyncSocketPacketOptions options; - EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options)); + EXPECT_EQ(static_cast<int>(data.size()), ch1.conn()->Send(data, options)); // Accept the connection to return the binding response, transition to // writable, and allow data to be sent. @@ -3165,7 +3152,7 @@ Eq(Connection::STATE_WRITABLE), {.timeout = kDefaultTimeout, .clock = &time_controller_}), IsRtcOk()); - EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options)); + EXPECT_EQ(static_cast<int>(data.size()), ch1.conn()->Send(data, options)); // Ask the connection to update state as if enough time has passed to lose // full writability and 5 pings went unresponded to. We'll accomplish the @@ -3180,7 +3167,7 @@ EXPECT_EQ(Connection::STATE_WRITE_UNRELIABLE, ch1.conn()->write_state()); // Data should be able to be sent in this state. - EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options)); + EXPECT_EQ(static_cast<int>(data.size()), ch1.conn()->Send(data, options)); // And now allow the other side to process the pings and send binding // responses. @@ -3200,7 +3187,7 @@ // Even if the connection has timed out, the Connection shouldn't block // the sending of data. - EXPECT_EQ(data_size, ch1.conn()->Send(data, data_size, options)); + EXPECT_EQ(static_cast<int>(data.size()), ch1.conn()->Send(data, options)); ch1.Stop(); ch2.Stop();
diff --git a/p2p/base/stun_dictionary.cc b/p2p/base/stun_dictionary.cc index 548cf40..cc28d8b 100644 --- a/p2p/base/stun_dictionary.cc +++ b/p2p/base/stun_dictionary.cc
@@ -347,7 +347,7 @@ } } return std::make_unique<StunByteStringAttribute>(STUN_ATTR_GOOG_DELTA, - buf.Data(), buf.Length()); + buf.DataView()); } // Apply a delta ack, i.e prune list of pending changes.
diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc index c440624..e35c9ca 100644 --- a/p2p/base/stun_port.cc +++ b/p2p/base/stun_port.cc
@@ -15,6 +15,7 @@ #include <functional> #include <memory> #include <optional> +#include <span> #include <utility> #include <vector> @@ -184,8 +185,8 @@ : Port(args, type), request_manager_( args.network_thread, - [this](const void* data, size_t size, StunRequest* request) { - SendStunRequest(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + SendStunRequest(data, request); }), socket_(socket), error_(0), @@ -202,8 +203,8 @@ : Port(args, type, min_port, max_port), request_manager_( args.network_thread, - [this](const void* data, size_t size, StunRequest* request) { - SendStunRequest(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + SendStunRequest(data, request); }), socket_(nullptr), error_(0), @@ -307,21 +308,20 @@ return conn; } -int UDPPort::SendTo(const void* data, - size_t size, +int UDPPort::SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool /* payload */) { AsyncSocketPacketOptions modified_options(options); CopyPortInformationToPacketInfo(&modified_options.info_signaled_after_sent); - int sent = socket_->SendTo(data, size, addr, modified_options); + int sent = socket_->SendTo(data.data(), data.size(), addr, modified_options); if (sent < 0) { error_ = socket_->GetError(); // Rate limiting added for crbug.com/856088. // TODO(webrtc:9622): Use general rate limiting mechanism once it exists. if (send_error_count_ < kSendErrorLogLimit) { ++send_error_count_; - RTC_LOG(LS_ERROR) << ToString() << ": UDP send of " << size + RTC_LOG(LS_ERROR) << ToString() << ": UDP send of " << data.size() << " bytes to host " << addr.ToSensitiveNameAndAddressString() << " failed with error " << error_; @@ -631,11 +631,11 @@ } } -void UDPPort::SendStunRequest(const void* data, size_t size, StunRequest* req) { +void UDPPort::SendStunRequest(std::span<const uint8_t> data, StunRequest* req) { StunBindingRequest* sreq = static_cast<StunBindingRequest*>(req); AsyncSocketPacketOptions options(StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kStunMessage; - SendTo(data, size, sreq->server_addr(), options, /*payload=*/true); + SendTo(data, sreq->server_addr(), options, /*payload=*/true); stats_.stun_binding_requests_sent++; }
diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h index 684cbeb..3baaf13 100644 --- a/p2p/base/stun_port.h +++ b/p2p/base/stun_port.h
@@ -17,6 +17,7 @@ #include <map> #include <memory> #include <optional> +#include <span> #include "absl/memory/memory.h" #include "absl/strings/string_view.h" @@ -128,14 +129,11 @@ bool emit_local_for_anyaddress); bool Init(); - int SendTo(const void* data, - size_t size, + int SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool payload) override; - void UpdateNetworkCost() override; - DiffServCodePoint StunDscpValue() const override; void OnLocalAddressReady(AsyncPacketSocket* socket, @@ -211,7 +209,7 @@ absl::string_view reason); // Sends STUN requests to the server. - void SendStunRequest(const void* data, size_t size, StunRequest* req); + void SendStunRequest(std::span<const uint8_t> data, StunRequest* req); // TODO(mallinaht): Move this up to Port when SignalAddressReady is // changed to SignalPortReady.
diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc index 1a18d2b..eb60eb6 100644 --- a/p2p/base/stun_request.cc +++ b/p2p/base/stun_request.cc
@@ -55,7 +55,7 @@ StunRequestManager::StunRequestManager( TaskQueueBase* thread, - std::function<void(const void*, size_t, StunRequest*)> send_packet) + std::function<void(std::span<const uint8_t>, StunRequest*)> send_packet) : thread_(thread), send_packet_(std::move(send_packet)) {} StunRequestManager::~StunRequestManager() = default; @@ -244,11 +244,10 @@ requests_.erase(request->id()); } -void StunRequestManager::SendPacket(const void* data, - size_t size, +void StunRequestManager::SendPacket(std::span<const uint8_t> data, StunRequest* request) { RTC_DCHECK_EQ(this, request->manager()); - send_packet_(data, size, request); + send_packet_(data, request); } StunRequest::StunRequest(const Environment& env, StunRequestManager& manager) @@ -302,7 +301,7 @@ ByteBufferWriter buf; msg_->Write(&buf); - manager_.SendPacket(buf.Data(), buf.Length(), this); + manager_.SendPacket(buf.DataView(), this); OnSent(); SendDelayed(TimeDelta::Millis(resend_delay()));
diff --git a/p2p/base/stun_request.h b/p2p/base/stun_request.h index ccd284a..3044a03 100644 --- a/p2p/base/stun_request.h +++ b/p2p/base/stun_request.h
@@ -44,7 +44,7 @@ public: StunRequestManager( TaskQueueBase* thread, - std::function<void(const void*, size_t, StunRequest*)> send_packet); + std::function<void(std::span<const uint8_t>, StunRequest*)> send_packet); ~StunRequestManager(); // Starts sending the given request (perhaps after a delay). @@ -79,7 +79,7 @@ TaskQueueBase* network_thread() const { return thread_; } - void SendPacket(const void* data, size_t size, StunRequest* request); + void SendPacket(std::span<const uint8_t> data, StunRequest* request); private: typedef std::map<std::string, std::unique_ptr<StunRequest>, std::less<>> @@ -87,7 +87,8 @@ TaskQueueBase* const thread_; RequestMap requests_ RTC_GUARDED_BY(thread_); - const std::function<void(const void*, size_t, StunRequest*)> send_packet_; + const std::function<void(std::span<const uint8_t>, StunRequest*)> + send_packet_; }; // Represents an individual request to be sent. The STUN message can either be
diff --git a/p2p/base/stun_request_unittest.cc b/p2p/base/stun_request_unittest.cc index a0998a3..0ec28aa 100644 --- a/p2p/base/stun_request_unittest.cc +++ b/p2p/base/stun_request_unittest.cc
@@ -12,7 +12,9 @@ #include <array> #include <cstddef> +#include <cstdint> #include <memory> +#include <span> #include <string> #include <utility> @@ -49,8 +51,8 @@ : time_controller_(Timestamp::Seconds(12345)), env_(CreateTestEnvironment({.time = &time_controller_})), manager_(time_controller_.GetMainThread(), - [this](const void* data, size_t size, StunRequest* request) { - OnSendPacket(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + OnSendPacket(data, request); }), request_count_(0), response_(nullptr), @@ -60,7 +62,7 @@ std::unique_ptr<StunRequestThunker> CreateStunRequest(); - void OnSendPacket(const void* data, size_t size, StunRequest* req) { + void OnSendPacket(std::span<const uint8_t> data, StunRequest* req) { request_count_++; }
diff --git a/p2p/base/tcp_port.cc b/p2p/base/tcp_port.cc index 87ef5bf..39253c6 100644 --- a/p2p/base/tcp_port.cc +++ b/p2p/base/tcp_port.cc
@@ -70,6 +70,7 @@ #include <cstddef> #include <cstdint> #include <list> +#include <span> #include <utility> #include "absl/algorithm/container.h" @@ -198,8 +199,7 @@ } } -int TCPPort::SendTo(const void* data, - size_t size, +int TCPPort::SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool payload) { @@ -236,13 +236,13 @@ } AsyncSocketPacketOptions modified_options(options); CopyPortInformationToPacketInfo(&modified_options.info_signaled_after_sent); - int sent = socket->Send(data, size, modified_options); + int sent = socket->Send(data.data(), data.size(), modified_options); if (sent < 0) { error_ = socket->GetError(); // Error from this code path for a Connection (instead of from a bare // socket) will not trigger reconnecting. In theory, this shouldn't matter // as OnClose should always be called and set connected to false. - RTC_LOG(LS_ERROR) << ToString() << ": TCP send of " << size + RTC_LOG(LS_ERROR) << ToString() << ": TCP send of " << data.size() << " bytes failed with error " << error_; } return sent; @@ -388,8 +388,7 @@ RTC_DCHECK_RUN_ON(network_thread()); } -int TCPConnection::Send(const void* data, - size_t size, +int TCPConnection::Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) { if (!socket_) { error_ = ENOTCONN; @@ -416,7 +415,7 @@ AsyncSocketPacketOptions modified_options(options); tcp_port()->CopyPortInformationToPacketInfo( &modified_options.info_signaled_after_sent); - int sent = socket_->Send(data, size, modified_options); + int sent = socket_->Send(data.data(), data.size(), modified_options); Timestamp now = env().clock().CurrentTime(); if (sent < 0) { mutable_stats().sent_discarded_packets++;
diff --git a/p2p/base/tcp_port.h b/p2p/base/tcp_port.h index 130e249..74cac03 100644 --- a/p2p/base/tcp_port.h +++ b/p2p/base/tcp_port.h
@@ -15,6 +15,7 @@ #include <cstdint> #include <list> #include <memory> +#include <span> #include "absl/memory/memory.h" #include "absl/strings/string_view.h" @@ -81,13 +82,12 @@ bool allow_listen); // Handles sending using the local TCP socket. - int SendTo(const void* data, - size_t size, + int SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool payload) override; - // Accepts incoming TCP connection. + void OnNewConnection(AsyncListenSocket* socket, AsyncPacketSocket* new_socket); @@ -133,8 +133,7 @@ AsyncPacketSocket* socket = nullptr); ~TCPConnection() override; - int Send(const void* data, - size_t size, + int Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) override; int GetError() override;
diff --git a/p2p/base/tcp_port_unittest.cc b/p2p/base/tcp_port_unittest.cc index 97de11c..143220d 100644 --- a/p2p/base/tcp_port_unittest.cc +++ b/p2p/base/tcp_port_unittest.cc
@@ -13,6 +13,7 @@ #include <cstdint> #include <list> #include <memory> +#include <span> #include <string> #include <vector> @@ -305,12 +306,10 @@ SentPacketCounter client_counter(client.get()); SentPacketCounter server_counter(server.get()); - static const char kData[] = "hello"; + static constexpr uint8_t kData[] = {'h', 'e', 'l', 'l', 'o', '\0'}; for (int i = 0; i < 10; ++i) { - client_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()); - server_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()); + client_conn->Send(kData, webrtc::AsyncSocketPacketOptions()); + server_conn->Send(kData, webrtc::AsyncSocketPacketOptions()); } EXPECT_THAT( webrtc::WaitUntil([&] { return client_counter.sent_packets(); }, Eq(10), @@ -363,9 +362,8 @@ webrtc::IsRtcOk()); SentPacketCounter client_counter(client.get()); - static const char kData[] = "hello"; - int result = client_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()); + static constexpr uint8_t kData[] = {'h', 'e', 'l', 'l', 'o', '\0'}; + int result = client_conn->Send(kData, webrtc::AsyncSocketPacketOptions()); EXPECT_EQ(result, 6); // Deleting the server port should break the current connection. @@ -383,8 +381,7 @@ // Sending a packet from the client will trigger a reconnect attempt but the // packet will be discarded. - result = client_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()); + result = client_conn->Send(kData, webrtc::AsyncSocketPacketOptions()); EXPECT_EQ(result, SOCKET_ERROR); ASSERT_THAT( webrtc::WaitUntil([&] { return client_conn->connected(); }, IsTrue(), @@ -394,8 +391,7 @@ EXPECT_TRUE(client_conn->writable()); for (int i = 0; i < 10; ++i) { // All sent packets still fail to send. - EXPECT_EQ(client_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()), + EXPECT_EQ(client_conn->Send(kData, webrtc::AsyncSocketPacketOptions()), SOCKET_ERROR); } // And are not reported as sent. @@ -436,9 +432,7 @@ // After the Stun Ping response has been received, packets can be sent again // and SignalSentPacket should be invoked. for (int i = 0; i < 5; ++i) { - EXPECT_EQ(client_conn->Send(&kData, sizeof(kData), - webrtc::AsyncSocketPacketOptions()), - 6); + EXPECT_EQ(client_conn->Send(kData, webrtc::AsyncSocketPacketOptions()), 6); } EXPECT_THAT(webrtc::WaitUntil( [&] { return client_counter.sent_packets(); }, Eq(2 + 5),
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index a0df6b5..10e9d1e 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc
@@ -199,8 +199,7 @@ void SendChannelBindRequest(int delay); // Sends a packet to the given destination address. // This will wrap the packet in STUN if necessary. - int Send(const void* data, - size_t size, + int Send(std::span<const uint8_t> data, bool payload, const AsyncSocketPacketOptions& options); @@ -246,8 +245,8 @@ stun_dscp_value_(DSCP_NO_CHANGE), request_manager_( args.network_thread, - [this](const void* data, size_t size, StunRequest* request) { - OnSendStunPacket(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + OnSendStunPacket(data, request); }), next_channel_number_(TURN_CHANNEL_NUMBER_START), state_(STATE_CONNECTING), @@ -277,8 +276,8 @@ stun_dscp_value_(DSCP_NO_CHANGE), request_manager_( args.network_thread, - [this](const void* data, size_t size, StunRequest* request) { - OnSendStunPacket(data, size, request); + [this](std::span<const uint8_t> data, StunRequest* request) { + OnSendStunPacket(data, request); }), next_channel_number_(TURN_CHANNEL_NUMBER_START), state_(STATE_CONNECTING), @@ -711,8 +710,7 @@ return error_; } -int TurnPort::SendTo(const void* data, - size_t size, +int TurnPort::SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool payload) { @@ -728,7 +726,7 @@ // Send the actual contents to the server using the usual mechanism. AsyncSocketPacketOptions modified_options(options); CopyPortInformationToPacketInfo(&modified_options.info_signaled_after_sent); - int sent = entry->Send(data, size, payload, modified_options); + int sent = entry->Send(data, payload, modified_options); if (sent <= 0) { error_ = socket_->GetError(); return SOCKET_ERROR; @@ -736,7 +734,7 @@ // The caller of the function is expecting the number of user data bytes, // rather than the size of the packet. - return static_cast<int>(size); + return static_cast<int>(data.size()); } bool TurnPort::CanHandleIncomingPacketsFrom(const SocketAddress& addr) const { @@ -915,14 +913,13 @@ resolver_->Start(address, Network()->family(), std::move(callback)); } -void TurnPort::OnSendStunPacket(const void* data, - size_t size, +void TurnPort::OnSendStunPacket(std::span<const uint8_t> data, StunRequest* request) { RTC_DCHECK(connected()); AsyncSocketPacketOptions options(StunDscpValue()); options.info_signaled_after_sent.packet_type = PacketType::kTurnMessage; CopyPortInformationToPacketInfo(&options.info_signaled_after_sent); - if (Send(data, size, options) < 0) { + if (Send(data, options) < 0) { RTC_LOG(LS_ERROR) << ToString() << ": Failed to send TURN message, error: " << socket_->GetError(); } @@ -1202,10 +1199,10 @@ RTC_DCHECK(success); } -int TurnPort::Send(const void* data, - size_t len, +int TurnPort::Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options) { - return socket_->SendTo(data, len, server_address_.address, options); + return socket_->SendTo(data.data(), data.size(), server_address_.address, + options); } void TurnPort::UpdateHash() { @@ -1348,14 +1345,14 @@ turn_customizer_->MaybeModifyOutgoingStunMessage(this, message); } -bool TurnPort::TurnCustomizerAllowChannelData(const void* data, - size_t size, +bool TurnPort::TurnCustomizerAllowChannelData(std::span<const uint8_t> data, bool payload) { if (turn_customizer_ == nullptr) { return true; } - return turn_customizer_->AllowChannelData(this, data, size, payload); + return turn_customizer_->AllowChannelData(this, data.data(), data.size(), + payload); } void TurnPort::MaybeAddTurnLoggingId(StunMessage* msg) { @@ -1837,20 +1834,19 @@ new TurnChannelBindRequest(port_, this, channel_id_, ext_addr_), delay); } -int TurnEntry::Send(const void* data, - size_t size, +int TurnEntry::Send(std::span<const uint8_t> data, bool payload, const AsyncSocketPacketOptions& options) { ByteBufferWriter buf; if (state_ != STATE_BOUND || - !port_->TurnCustomizerAllowChannelData(data, size, payload)) { + !port_->TurnCustomizerAllowChannelData(data, payload)) { // If we haven't bound the channel yet, we have to use a Send Indication. // The turn_customizer_ can also make us use Send Indication. TurnMessage msg(TURN_SEND_INDICATION); msg.AddAttribute(std::make_unique<StunXorAddressAttribute>( STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_)); msg.AddAttribute( - std::make_unique<StunByteStringAttribute>(STUN_ATTR_DATA, data, size)); + std::make_unique<StunByteStringAttribute>(STUN_ATTR_DATA, data)); port_->TurnCustomizerMaybeModifyOutgoingStunMessage(&msg); @@ -1865,14 +1861,13 @@ } else { // If the channel is bound, we can send the data as a Channel Message. buf.WriteUInt16(channel_id_); - buf.WriteUInt16(static_cast<uint16_t>(size)); - buf.Write( - std::span<const uint8_t>(reinterpret_cast<const uint8_t*>(data), size)); + buf.WriteUInt16(static_cast<uint16_t>(data.size())); + buf.Write(data); } AsyncSocketPacketOptions modified_options(options); modified_options.info_signaled_after_sent.turn_overhead_bytes = - buf.Length() - size; - return port_->Send(buf.Data(), buf.Length(), modified_options); + buf.Length() - data.size(); + return port_->Send(buf.DataView(), modified_options); } void TurnEntry::OnCreatePermissionSuccess() {
diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 28547e8..04a1e34 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h
@@ -16,9 +16,11 @@ #include <map> #include <memory> #include <set> +#include <span> #include <string> #include <vector> +#include "absl/base/macros.h" #include "absl/memory/memory.h" #include "absl/strings/string_view.h" #include "api/async_dns_resolver.h" @@ -155,12 +157,23 @@ void PrepareAddress() override; Connection* CreateConnection(const Candidate& c, PortInterface::CandidateOrigin origin) override; + int SendTo(std::span<const uint8_t> data, + const SocketAddress& addr, + const AsyncSocketPacketOptions& options, + bool payload) override; + + ABSL_DEPRECATE_AND_INLINE() int SendTo(const void* data, size_t size, const SocketAddress& addr, const AsyncSocketPacketOptions& options, - bool payload) override; + bool payload) override { + return SendTo(std::span(reinterpret_cast<const uint8_t*>(data), size), addr, + options, payload); + } + int SetOption(Socket::Option opt, int value) override; + int GetOption(Socket::Option opt, int* value) override; int GetError() override; @@ -266,7 +279,7 @@ void OnLocalNetworkAccessPermissionGranted(); void AddRequestAuthInfo(StunMessage* msg); - void OnSendStunPacket(const void* data, size_t size, StunRequest* request); + void OnSendStunPacket(std::span<const uint8_t> data, StunRequest* request); // Stun address from allocate success response. // Currently used only for testing. void OnStunAddress(const SocketAddress& address); @@ -281,8 +294,7 @@ bool ScheduleRefresh(uint32_t lifetime); void SendRequest(StunRequest* request, int delay); - int Send(const void* data, - size_t size, + int Send(std::span<const uint8_t> data, const AsyncSocketPacketOptions& options); void UpdateHash(); bool UpdateNonce(StunMessage* response); @@ -299,8 +311,7 @@ void MaybeAddTurnLoggingId(StunMessage* message); void TurnCustomizerMaybeModifyOutgoingStunMessage(StunMessage* message); - bool TurnCustomizerAllowChannelData(const void* data, - size_t size, + bool TurnCustomizerAllowChannelData(std::span<const uint8_t> data, bool payload); ProtocolAddress server_address_;
diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index b23f33a..346f4b7 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc
@@ -9,6 +9,8 @@ */ #include "p2p/base/turn_port.h" +#include <algorithm> +#include <array> #include <cstddef> #include <cstdint> #include <list> @@ -739,7 +741,7 @@ ByteBufferWriter buf; msg->Write(&buf); - conn1->Send(buf.Data(), buf.Length(), options); + conn1->Send(buf.DataView(), options); // Now restore the password before continuing. conn1->set_remote_password_for_test(pwd); @@ -801,13 +803,12 @@ // Send some data. size_t num_packets = 256; for (size_t i = 0; i < num_packets; ++i) { - unsigned char buf[256] = {0}; - for (size_t j = 0; j < i + 1; ++j) { - buf[j] = 0xFF - static_cast<unsigned char>(j); - } + std::array<uint8_t, 256> buf; + uint8_t val = 0xFF; + std::generate(buf.begin(), buf.begin() + i + 1, [&val] { return val--; }); options.ect_1 = (i % 2 == 0); - conn1->Send(buf, i + 1, options); - conn2->Send(buf, i + 1, options); + conn1->Send(std::span(buf).first(i + 1), options); + conn2->Send(std::span(buf).first(i + 1), options); time_controller_.AdvanceTime(kSimulatedRtt); } @@ -878,8 +879,9 @@ IsRtcOk()); // Send some data from Udp to TurnPort. - unsigned char buf[256] = {0}; - conn2->Send(buf, sizeof(buf), options); + std::array<uint8_t, 256> buf; + buf.fill(0); + conn2->Send(buf, options); // Now release the TurnPort allocation. // This will send a REFRESH with lifetime 0 to server. @@ -917,7 +919,7 @@ std::unique_ptr<TurnPortTestVirtualSocketServer> ss_; GlobalSimulatedTimeController time_controller_; const Environment env_; - webrtc::Thread* main_; + Thread* main_; std::unique_ptr<AsyncPacketSocket> socket_; TestTurnServer turn_server_; std::unique_ptr<TurnPort> turn_port_; @@ -1693,8 +1695,8 @@ // Tell the TURN server to reject all bind requests from now on. turn_server_.server()->set_reject_bind_requests(true); - std::string data = "ABC"; - conn1->Send(data.data(), data.length(), options); + auto data = std::to_array<uint8_t>({'A', 'B', 'C'}); + conn1->Send(data, options); EXPECT_THAT( WaitUntil([&] { return CheckConnectionFailedAndPruned(conn1); }, IsTrue(), @@ -1708,7 +1710,7 @@ // received unchanneled, not channeled. udp_packets_.emplace_back(packet); }); - conn1->Send(data.data(), data.length(), options); + conn1->Send(data, options); EXPECT_THAT(WaitUntil([&] { return !udp_packets_.empty(); }, IsTrue(), {.timeout = kSimulatedRtt, .clock = &time_controller_}), IsRtcOk()); @@ -2247,12 +2249,11 @@ metrics::Reset(); SetDnsResolverExpectations( - [](webrtc::MockAsyncDnsResolver* resolver, - webrtc::MockAsyncDnsResolverResult* resolver_result) { + [](MockAsyncDnsResolver* resolver, + MockAsyncDnsResolverResult* resolver_result) { EXPECT_CALL(*resolver, Start(SocketAddress("localhost", 5000), /*family=*/AF_INET, _)) - .WillOnce([](const webrtc::SocketAddress& /* addr */, - int /* family */, + .WillOnce([](const SocketAddress& /* addr */, int /* family */, absl::AnyInvocable<void()> callback) { callback(); }); EXPECT_CALL(*resolver, result) .WillRepeatedly(ReturnPointee(resolver_result));
diff --git a/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc b/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc index 7ff86af..cadc0ad 100644 --- a/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc +++ b/p2p/dtls/dtls_stun_piggyback_controller_unittest.cc
@@ -89,8 +89,7 @@ std::unique_ptr<StunByteStringAttribute> WrapInStun( IceAttributeType type, const std::vector<uint8_t>& data) { - return std::make_unique<StunByteStringAttribute>(type, data.data(), - data.size()); + return std::make_unique<StunByteStringAttribute>(type, data); } std::unique_ptr<StunByteStringAttribute> WrapInStun(
diff --git a/p2p/test/fake_ice_transport.h b/p2p/test/fake_ice_transport.h index 864bd40..4163af5 100644 --- a/p2p/test/fake_ice_transport.h +++ b/p2p/test/fake_ice_transport.h
@@ -521,8 +521,7 @@ int flags) RTC_EXCLUSIVE_LOCKS_REQUIRED(network_thread_) { last_sent_packet_ = packet; - bool is_stun = - StunMessage::ValidateFingerprint(packet.data<char>(), packet.size()); + bool is_stun = StunMessage::ValidateFingerprint(packet); if (packet_send_filter_func_ && packet_send_filter_func_(packet.data<char>(), packet.size(), options, flags)) { @@ -609,7 +608,7 @@ } std::unique_ptr<IceMessage> GetStunMessage(const CopyOnWriteBuffer& packet) { - if (!StunMessage::ValidateFingerprint(packet.data<char>(), packet.size())) { + if (!StunMessage::ValidateFingerprint(packet)) { return nullptr; }
diff --git a/p2p/test/test_port.cc b/p2p/test/test_port.cc index eb4161c..83cc7ec 100644 --- a/p2p/test/test_port.cc +++ b/p2p/test/test_port.cc
@@ -97,15 +97,13 @@ conn->set_use_candidate_attr(true); return conn; } -int TestPort::SendTo(const void* data, - size_t size, +int TestPort::SendTo(std::span<const uint8_t> data, const SocketAddress& /* addr */, const AsyncSocketPacketOptions& /* options */, bool payload) { if (!payload) { auto msg = std::make_unique<IceMessage>(); - auto buf = std::make_unique<BufferT<uint8_t>>( - static_cast<const char*>(data), size); + auto buf = std::make_unique<BufferT<uint8_t>>(data); ByteBufferReader read_buf(*buf); if (!msg->Read(&read_buf)) { return -1; @@ -113,8 +111,9 @@ last_stun_buf_ = std::move(buf); last_stun_msg_ = std::move(msg); } - return static_cast<int>(size); + return static_cast<int>(data.size()); } + int TestPort::SetOption(Socket::Option /* opt */, int /* value */) { return 0; }
diff --git a/p2p/test/test_port.h b/p2p/test/test_port.h index 8d10797..1334027 100644 --- a/p2p/test/test_port.h +++ b/p2p/test/test_port.h
@@ -61,12 +61,13 @@ Connection* CreateConnection(const Candidate& remote_candidate, CandidateOrigin origin) override; - int SendTo(const void* data, - size_t size, + int SendTo(std::span<const uint8_t> data, const SocketAddress& addr, const AsyncSocketPacketOptions& options, bool payload) override; + int SetOption(Socket::Option opt, int value) override; + int GetOption(Socket::Option opt, int* value) override; int GetError() override; void Reset();
diff --git a/p2p/test/turn_server.cc b/p2p/test/turn_server.cc index 40060b1..9a1236f 100644 --- a/p2p/test/turn_server.cc +++ b/p2p/test/turn_server.cc
@@ -813,7 +813,7 @@ msg.AddAttribute(std::make_unique<StunXorAddressAttribute>( STUN_ATTR_XOR_PEER_ADDRESS, packet.source_address())); msg.AddAttribute(std::make_unique<StunByteStringAttribute>( - STUN_ATTR_DATA, packet.payload().data(), packet.payload().size())); + STUN_ATTR_DATA, packet.payload())); server_->SendStun(&conn_, &msg, packet.ecn()); } else { RTC_LOG(LS_WARNING)
diff --git a/rtc_base/async_packet_socket.h b/rtc_base/async_packet_socket.h index d209a68..30723fc 100644 --- a/rtc_base/async_packet_socket.h +++ b/rtc_base/async_packet_socket.h
@@ -14,6 +14,7 @@ #include <cstddef> #include <cstdint> #include <functional> +#include <span> #include <utility> #include <vector> @@ -103,10 +104,19 @@ virtual int Send(const void* pv, size_t cb, const AsyncSocketPacketOptions& options) = 0; + int Send(std::span<const uint8_t> data, + const AsyncSocketPacketOptions& options) { + return Send(data.data(), data.size(), options); + } virtual int SendTo(const void* pv, size_t cb, const SocketAddress& addr, const AsyncSocketPacketOptions& options) = 0; + int SendTo(std::span<const uint8_t> data, + const SocketAddress& addr, + const AsyncSocketPacketOptions& options) { + return SendTo(data.data(), data.size(), addr, options); + } // Close the socket. virtual int Close() = 0;
diff --git a/rtc_base/async_tcp_socket.h b/rtc_base/async_tcp_socket.h index c937fdd..0b0b09c 100644 --- a/rtc_base/async_tcp_socket.h +++ b/rtc_base/async_tcp_socket.h
@@ -37,6 +37,9 @@ AsyncTCPSocketBase(const AsyncTCPSocketBase&) = delete; AsyncTCPSocketBase& operator=(const AsyncTCPSocketBase&) = delete; + using AsyncPacketSocket::Send; + using AsyncPacketSocket::SendTo; + // Pure virtual methods to send and recv data. int Send(const void* pv, size_t cb, @@ -89,6 +92,9 @@ AsyncTCPSocket(const AsyncTCPSocket&) = delete; AsyncTCPSocket& operator=(const AsyncTCPSocket&) = delete; + using AsyncTCPSocketBase::Send; + using AsyncTCPSocketBase::SendTo; + int Send(const void* pv, size_t cb, const AsyncSocketPacketOptions& options) override;
diff --git a/rtc_base/crc32.h b/rtc_base/crc32.h index e175271..c94adb3 100644 --- a/rtc_base/crc32.h +++ b/rtc_base/crc32.h
@@ -13,6 +13,7 @@ #include <cstddef> #include <cstdint> +#include <span> #include "absl/strings/string_view.h" @@ -29,6 +30,9 @@ inline uint32_t ComputeCrc32(absl::string_view str) { return ComputeCrc32(str.data(), str.size()); } +inline uint32_t ComputeCrc32(std::span<const uint8_t> data) { + return ComputeCrc32(data.data(), data.size()); +} } // namespace webrtc
diff --git a/rtc_base/message_digest.h b/rtc_base/message_digest.h index ef3edb1..d8c78d0 100644 --- a/rtc_base/message_digest.h +++ b/rtc_base/message_digest.h
@@ -13,6 +13,8 @@ #include <stddef.h> +#include <cstdint> +#include <span> #include <string> #include "absl/strings/string_view.h" @@ -70,6 +72,12 @@ size_t in_len, void* output, size_t out_len); +inline size_t ComputeDigest(absl::string_view alg, + std::span<const uint8_t> input, + std::span<uint8_t> output) { + return ComputeDigest(alg, input.data(), input.size(), output.data(), + output.size()); +} // Computes the hash of `input` using the `digest` hash implementation, and // returns it as a hex-encoded string. std::string ComputeDigest(MessageDigest* digest, absl::string_view input); @@ -111,6 +119,20 @@ size_t in_len, void* output, size_t out_len); +inline size_t ComputeHmac(absl::string_view alg, + std::span<const uint8_t> key, + std::span<const uint8_t> input, + std::span<uint8_t> output) { + return ComputeHmac(alg, key.data(), key.size(), input.data(), input.size(), + output.data(), output.size()); +} +inline size_t ComputeHmac(absl::string_view alg, + absl::string_view key, + std::span<const uint8_t> input, + std::span<uint8_t> output) { + return ComputeHmac(alg, key.data(), key.size(), input.data(), input.size(), + output.data(), output.size()); +} // Computes the HMAC of `input` using the `digest` hash implementation and `key` // to key the HMAC, and returns it as a hex-encoded string. std::string ComputeHmac(MessageDigest* digest,
diff --git a/rtc_base/span_helpers.h b/rtc_base/span_helpers.h index 6de382c..89a9913 100644 --- a/rtc_base/span_helpers.h +++ b/rtc_base/span_helpers.h
@@ -13,6 +13,9 @@ #include <cstdint> #include <span> +#include <string> + +#include "absl/strings/string_view.h" namespace webrtc { @@ -43,6 +46,15 @@ span.size()); } +inline std::span<const uint8_t> AsUint8Span(absl::string_view s) { + return std::span<const uint8_t>(reinterpret_cast<const uint8_t*>(s.data()), + s.size()); +} + +inline std::span<const uint8_t> AsUint8Span(const std::string& s) { + return AsUint8Span(absl::string_view(s)); +} + inline absl::string_view AsStringView(std::span<const uint8_t> span) { return absl::string_view(reinterpret_cast<const char*>(span.data()), span.size());
diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 34244b1..f68cf15 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn
@@ -517,7 +517,9 @@ webrtc_fuzzer_test("stun_validator_fuzzer") { sources = [ "stun_validator_fuzzer.cc" ] deps = [ + ":fuzz_data_helper", "../../api/transport:stun_types", + "../../rtc_base:span_helpers", "//third_party/abseil-cpp/absl/strings:string_view", ] seed_corpus = "corpora/stun-corpus"
diff --git a/test/fuzzers/stun_validator_fuzzer.cc b/test/fuzzers/stun_validator_fuzzer.cc index ec1892a..d1bd1df 100644 --- a/test/fuzzers/stun_validator_fuzzer.cc +++ b/test/fuzzers/stun_validator_fuzzer.cc
@@ -13,14 +13,15 @@ #include "absl/strings/string_view.h" #include "api/transport/stun.h" +#include "rtc_base/span_helpers.h" #include "test/fuzzers/fuzz_data_helper.h" namespace webrtc { void FuzzOneInput(FuzzDataHelper fuzz_data) { absl::string_view message = fuzz_data.ReadString(); - webrtc::StunMessage::ValidateFingerprint(message.data(), message.size()); - webrtc::StunMessage::ValidateMessageIntegrityForTesting(message.data(), - message.size(), ""); + std::span<const uint8_t> data = AsUint8Span(message); + StunMessage::ValidateFingerprint(data); + StunMessage::ValidateMessageIntegrityForTesting("", data); } } // namespace webrtc
diff --git a/test/peer_scenario/bwe_integration_tests/l4s_test.cc b/test/peer_scenario/bwe_integration_tests/l4s_test.cc index 8efff17..f1b0ce6 100644 --- a/test/peer_scenario/bwe_integration_tests/l4s_test.cc +++ b/test/peer_scenario/bwe_integration_tests/l4s_test.cc
@@ -587,9 +587,7 @@ // be RTCP. Negotiation is still done using not ECT. callee_to_caller_node->router()->SetWatcher( [&](const EmulatedIpPacket& packet) { - if (StunMessage::ValidateFingerprint( - reinterpret_cast<const char*>(packet.data.data()), - packet.data.size())) { + if (StunMessage::ValidateFingerprint(packet.data)) { return; } if (packet.ecn == EcnMarking::kEct1 || packet.ecn == EcnMarking::kCe) {