Change StunByteStringAttribute to have uint8_t* internal type Also make it more convenient to use uint8_t array view for interfacing to the class. Bug: webrtc:15665 Change-Id: Ib671b5add79a48004133a6ecd99429534f7de1de Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328140 Commit-Queue: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/main@{#41212}
diff --git a/api/transport/stun.cc b/api/transport/stun.cc index efbec13..7ef6852 100644 --- a/api/transport/stun.cc +++ b/api/transport/stun.cc
@@ -1132,13 +1132,13 @@ } void StunByteStringAttribute::CopyBytes(absl::string_view bytes) { - char* new_bytes = new char[bytes.size()]; + 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) { - char* new_bytes = new char[length]; + uint8_t* new_bytes = new uint8_t[length]; memcpy(new_bytes, bytes, length); SetBytes(new_bytes, length); } @@ -1146,7 +1146,7 @@ uint8_t StunByteStringAttribute::GetByte(size_t index) const { RTC_DCHECK(bytes_ != NULL); RTC_DCHECK(index < length()); - return static_cast<uint8_t>(bytes_[index]); + return bytes_[index]; } void StunByteStringAttribute::SetByte(size_t index, uint8_t value) { @@ -1156,10 +1156,8 @@ } bool StunByteStringAttribute::Read(ByteBufferReader* buf) { - bytes_ = new char[length()]; - RTC_CHECK(bytes_); - if (!buf->ReadBytes( - rtc::MakeArrayView(reinterpret_cast<uint8_t*>(bytes_), length()))) { + bytes_ = new uint8_t[length()]; + if (!buf->ReadBytes(rtc::ArrayView<uint8_t>(bytes_, length()))) { return false; } @@ -1172,12 +1170,12 @@ if (!LengthValid(type(), length())) { return false; } - buf->WriteBytes(bytes_, length()); + buf->WriteBytes(reinterpret_cast<const char*>(bytes_), length()); WritePadding(buf); return true; } -void StunByteStringAttribute::SetBytes(char* bytes, size_t length) { +void StunByteStringAttribute::SetBytes(uint8_t* bytes, size_t length) { delete[] bytes_; bytes_ = bytes; SetLength(static_cast<uint16_t>(length));
diff --git a/api/transport/stun.h b/api/transport/stun.h index 4a04db3..62d98f7 100644 --- a/api/transport/stun.h +++ b/api/transport/stun.h
@@ -519,13 +519,22 @@ StunAttributeValueType value_type() const override; - const char* bytes() const { return bytes_; } + [[deprecated("Use array_view")]] const char* bytes() const { + return reinterpret_cast<const char*>(bytes_); + } + // Returns the attribute value as a string. + // Use this for attributes that are text or text-compatible. absl::string_view string_view() const { - return absl::string_view(bytes_, length()); + return absl::string_view(reinterpret_cast<const char*>(bytes_), length()); + } + // Returns the attribute value as an uint8_t view. + // Use this function for values that are not text. + rtc::ArrayView<uint8_t> array_view() const { + return rtc::MakeArrayView(bytes_, length()); } [[deprecated]] std::string GetString() const { - return std::string(bytes_, length()); + return std::string(reinterpret_cast<const char*>(bytes_), length()); } void CopyBytes(const void* bytes, size_t length); @@ -538,9 +547,9 @@ bool Write(rtc::ByteBufferWriter* buf) const override; private: - void SetBytes(char* bytes, size_t length); + void SetBytes(uint8_t* bytes, size_t length); - char* bytes_; + uint8_t* bytes_; }; // Implements STUN attributes that record an error code.
diff --git a/api/transport/stun_unittest.cc b/api/transport/stun_unittest.cc index 65891f3..9ad4da9 100644 --- a/api/transport/stun_unittest.cc +++ b/api/transport/stun_unittest.cc
@@ -1232,8 +1232,8 @@ const StunByteStringAttribute* mi_attr = msg.GetByteString(STUN_ATTR_MESSAGE_INTEGRITY); EXPECT_EQ(20U, mi_attr->length()); - EXPECT_EQ( - 0, memcmp(mi_attr->bytes(), kCalculatedHmac1, sizeof(kCalculatedHmac1))); + EXPECT_EQ(0, memcmp(mi_attr->array_view().data(), kCalculatedHmac1, + sizeof(kCalculatedHmac1))); rtc::ByteBufferWriter buf1; EXPECT_TRUE(msg.Write(&buf1)); @@ -1248,8 +1248,8 @@ const StunByteStringAttribute* mi_attr2 = msg2.GetByteString(STUN_ATTR_MESSAGE_INTEGRITY); EXPECT_EQ(20U, mi_attr2->length()); - EXPECT_EQ( - 0, memcmp(mi_attr2->bytes(), kCalculatedHmac2, sizeof(kCalculatedHmac2))); + EXPECT_EQ(0, memcmp(mi_attr2->array_view().data(), kCalculatedHmac2, + sizeof(kCalculatedHmac2))); rtc::ByteBufferWriter buf3; EXPECT_TRUE(msg2.Write(&buf3)); @@ -1321,7 +1321,7 @@ const StunByteStringAttribute* mi_attr = msg.GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32); EXPECT_EQ(4U, mi_attr->length()); - EXPECT_EQ(0, memcmp(mi_attr->bytes(), kCalculatedHmac1_32, + EXPECT_EQ(0, memcmp(mi_attr->array_view().data(), kCalculatedHmac1_32, sizeof(kCalculatedHmac1_32))); rtc::ByteBufferWriter buf1; @@ -1337,7 +1337,7 @@ const StunByteStringAttribute* mi_attr2 = msg2.GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32); EXPECT_EQ(4U, mi_attr2->length()); - EXPECT_EQ(0, memcmp(mi_attr2->bytes(), kCalculatedHmac2_32, + EXPECT_EQ(0, memcmp(mi_attr2->array_view().data(), kCalculatedHmac2_32, sizeof(kCalculatedHmac2_32))); rtc::ByteBufferWriter buf3; @@ -1502,7 +1502,7 @@ bytes = msg.GetByteString(STUN_ATTR_MAGIC_COOKIE); ASSERT_TRUE(bytes != NULL); EXPECT_EQ(4U, bytes->length()); - EXPECT_EQ(0, memcmp(bytes->bytes(), TURN_MAGIC_COOKIE_VALUE, + EXPECT_EQ(0, memcmp(bytes->array_view().data(), TURN_MAGIC_COOKIE_VALUE, sizeof(TURN_MAGIC_COOKIE_VALUE))); bytes2 = StunAttribute::CreateByteString(STUN_ATTR_MAGIC_COOKIE); @@ -1586,8 +1586,9 @@ 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())->bytes()); + EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(attr.get()) + ->string_view() + .data()); EXPECT_LT(msg.length(), len); } @@ -1609,8 +1610,9 @@ 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())->bytes()); + EXPECT_STREQ("kenta", static_cast<StunByteStringAttribute*>(attr.get()) + ->string_view() + .data()); } // Remove should remove the last added occurrence. @@ -1618,8 +1620,9 @@ 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())->bytes()); + EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(attr.get()) + ->string_view() + .data()); } // Removing something that does exist should return nullptr. @@ -1652,8 +1655,9 @@ auto copy = CopyStunAttribute(*attr.get(), buffer_ptr); ASSERT_EQ(copy->value_type(), STUN_VALUE_BYTE_STRING); - EXPECT_STREQ("kes", - static_cast<StunByteStringAttribute*>(copy.get())->bytes()); + EXPECT_STREQ("kes", static_cast<StunByteStringAttribute*>(copy.get()) + ->string_view() + .data()); } { // Test StunAddressAttribute.
diff --git a/p2p/base/stun_dictionary.cc b/p2p/base/stun_dictionary.cc index 4a54b65..318bed0 100644 --- a/p2p/base/stun_dictionary.cc +++ b/p2p/base/stun_dictionary.cc
@@ -80,8 +80,7 @@ webrtc::RTCErrorOr< std::pair<uint64_t, std::deque<std::unique_ptr<StunAttribute>>>> StunDictionaryView::ParseDelta(const StunByteStringAttribute& delta) { - rtc::ByteBufferReader buf(rtc::MakeArrayView( - reinterpret_cast<const uint8_t*>(delta.bytes()), delta.length())); + rtc::ByteBufferReader buf(delta.array_view()); uint16_t magic; if (!buf.ReadUInt16(&magic)) { return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index a176272..e6f5e77 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc
@@ -1027,9 +1027,10 @@ "peer address, addr: " << ext_addr.ToSensitiveString(); } - - DispatchPacket(data_attr->bytes(), data_attr->length(), ext_addr, PROTO_UDP, - packet_time_us); + // TODO(bugs.webrtc.org/14870): rebuild DispatchPacket to take an + // ArrayView<uint8_t> + DispatchPacket(reinterpret_cast<const char*>(data_attr->array_view().data()), + data_attr->length(), ext_addr, PROTO_UDP, packet_time_us); } void TurnPort::HandleChannelData(int channel_id,
diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 93c4dff..e626947 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc
@@ -1727,8 +1727,7 @@ const StunByteStringAttribute* attr = msg->GetByteString(TestTurnCustomizer::STUN_ATTR_COUNTER); if (attr != nullptr && attr_counter_ != nullptr) { - rtc::ByteBufferReader buf(rtc::MakeArrayView( - reinterpret_cast<const uint8_t*>(attr->bytes()), attr->length())); + rtc::ByteBufferReader buf(attr->array_view()); unsigned int val = ~0u; buf.ReadUInt32(&val); (*attr_counter_)++;
diff --git a/p2p/base/turn_server.cc b/p2p/base/turn_server.cc index e2e1a7d..b0c895e 100644 --- a/p2p/base/turn_server.cc +++ b/p2p/base/turn_server.cc
@@ -670,8 +670,8 @@ // If a permission exists, send the data on to the peer. if (HasPermission(peer_attr->GetAddress().ipaddr())) { - SendExternal(data_attr->bytes(), data_attr->length(), - peer_attr->GetAddress()); + SendExternal(reinterpret_cast<char*>(data_attr->array_view().data()), + data_attr->length(), peer_attr->GetAddress()); } else { RTC_LOG(LS_WARNING) << ToString() << ": Received send indication without permission"