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"
