admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change StunMessage::AddAttribute return type from bool to void.

Browse Source 
Proper error handling was missing, using VERIFY to crash in debug
builds, while release builds would ignore the error and leak the
attribute memory. The check of attribute type consistency was changed
to a RTC_DCHECK.

Also removes a large number of uses of the deprecated VERIFY macro.

BUG=webrtc:6424

Review-Url: https://codereview.webrtc.org/2665343002
Cr-Commit-Position: refs/heads/master@{#16413}
This commit is contained in:
nisse 2017-02-02 01:31:30 -08:00 committed by Commit bot
parent f7826d668a
commit cc99bc25d8
6 changed files with 66 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
webrtc/p2p/base/relayport.cc
View File

@ -580,32 +580,32 @@ int RelayEntry::SendTo(const void* data, size_t size,
StunAttribute::CreateByteString(STUN_ATTR_MAGIC_COOKIE);
magic_cookie_attr->CopyBytes(TURN_MAGIC_COOKIE_VALUE,
sizeof(TURN_MAGIC_COOKIE_VALUE));
VERIFY(request.AddAttribute(magic_cookie_attr));
request.AddAttribute(magic_cookie_attr);
StunByteStringAttribute* username_attr =
StunAttribute::CreateByteString(STUN_ATTR_USERNAME);
username_attr->CopyBytes(port_->username_fragment().c_str(),
port_->username_fragment().size());
VERIFY(request.AddAttribute(username_attr));
request.AddAttribute(username_attr);
StunAddressAttribute* addr_attr =
StunAttribute::CreateAddress(STUN_ATTR_DESTINATION_ADDRESS);
addr_attr->SetIP(addr.ipaddr());
addr_attr->SetPort(addr.port());
VERIFY(request.AddAttribute(addr_attr));
request.AddAttribute(addr_attr);
// Attempt to lock
if (ext_addr_ == addr) {
StunUInt32Attribute* options_attr =
StunAttribute::CreateUInt32(STUN_ATTR_OPTIONS);
options_attr->SetValue(0x1);
VERIFY(request.AddAttribute(options_attr));
request.AddAttribute(options_attr);
}
StunByteStringAttribute* data_attr =
StunAttribute::CreateByteString(STUN_ATTR_DATA);
data_attr->CopyBytes(data, size);
VERIFY(request.AddAttribute(data_attr));
request.AddAttribute(data_attr);
// TODO: compute the HMAC.
@ -792,7 +792,7 @@ void AllocateRequest::Prepare(StunMessage* request) {
username_attr->CopyBytes(
entry_->port()->username_fragment().c_str(),
entry_->port()->username_fragment().size());
VERIFY(request->AddAttribute(username_attr));
request->AddAttribute(username_attr);
}
void AllocateRequest::OnSent() {

13
webrtc/p2p/base/stun.cc
View File

@ -16,7 +16,6 @@
#include "webrtc/base/byteorder.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/common.h"
#include "webrtc/base/crc32.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/messagedigest.h"
@ -74,11 +73,10 @@ bool StunMessage::SetTransactionID(const std::string& str) {
return true;
}
bool StunMessage::AddAttribute(StunAttribute* attr) {
void StunMessage::AddAttribute(StunAttribute* attr) {
// Fail any attributes that aren't valid for this type of message.
if (attr->value_type() != GetAttributeValueType(attr->type())) {
return false;
}
RTC_DCHECK_EQ(attr->value_type(), GetAttributeValueType(attr->type()));
attrs_->push_back(attr);
attr->SetOwner(this);
size_t attr_length = attr->length();
@ -86,7 +84,6 @@ bool StunMessage::AddAttribute(StunAttribute* attr) {
attr_length += (4 - (attr_length % 4));
}
length_ += static_cast<uint16_t>(attr_length + 4);
return true;
}
const StunAddressAttribute* StunMessage::GetAddress(int type) const {
@ -220,7 +217,7 @@ bool StunMessage::AddMessageIntegrity(const char* key,
StunByteStringAttribute* msg_integrity_attr =
new StunByteStringAttribute(STUN_ATTR_MESSAGE_INTEGRITY,
std::string(kStunMessageIntegritySize, '0'));
VERIFY(AddAttribute(msg_integrity_attr));
AddAttribute(msg_integrity_attr);
// Calculate the HMAC for the message.
ByteBufferWriter buf;
@ -281,7 +278,7 @@ bool StunMessage::AddFingerprint() {
// it can't fail.
StunUInt32Attribute* fingerprint_attr =
new StunUInt32Attribute(STUN_ATTR_FINGERPRINT, 0);
VERIFY(AddAttribute(fingerprint_attr));
AddAttribute(fingerprint_attr);
// Calculate the CRC-32 for the message and insert it.
ByteBufferWriter buf;

6
webrtc/p2p/base/stun.h
View File

@ -158,10 +158,8 @@ class StunMessage {
const StunErrorCodeAttribute* GetErrorCode() const;
const StunUInt16ListAttribute* GetUnknownAttributes() const;
// Takes ownership of the specified attribute, verifies it is of the correct
// type, and adds it to the message. The return value indicates whether this
// was successful.
bool AddAttribute(StunAttribute* attr);
// Takes ownership of the specified attribute and adds it to the message.
void AddAttribute(StunAttribute* attr);
// Validates that a raw STUN message has a correct MESSAGE-INTEGRITY value.
// This can't currently be done on a StunMessage, since it is affected by

30
webrtc/p2p/base/stun_unittest.cc
View File

@ -885,7 +885,7 @@ TEST_F(StunTest, WriteMessageWithIPv6AddressAttribute) {
StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS);
rtc::SocketAddress test_addr(test_ip, kTestMessagePort2);
addr->SetAddress(test_addr);
EXPECT_TRUE(msg.AddAttribute(addr));
msg.AddAttribute(addr);
CheckStunHeader(msg, STUN_BINDING_REQUEST, (size - 20));
@ -915,7 +915,7 @@ TEST_F(StunTest, WriteMessageWithIPv4AddressAttribute) {
StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS);
rtc::SocketAddress test_addr(test_ip, kTestMessagePort4);
addr->SetAddress(test_addr);
EXPECT_TRUE(msg.AddAttribute(addr));
msg.AddAttribute(addr);
CheckStunHeader(msg, STUN_BINDING_RESPONSE, (size - 20));
@ -945,7 +945,7 @@ TEST_F(StunTest, WriteMessageWithIPv6XorAddressAttribute) {
StunAttribute::CreateXorAddress(STUN_ATTR_XOR_MAPPED_ADDRESS);
rtc::SocketAddress test_addr(test_ip, kTestMessagePort1);
addr->SetAddress(test_addr);
EXPECT_TRUE(msg.AddAttribute(addr));
msg.AddAttribute(addr);
CheckStunHeader(msg, STUN_BINDING_RESPONSE, (size - 20));
@ -976,7 +976,7 @@ TEST_F(StunTest, WriteMessageWithIPv4XoreAddressAttribute) {
StunAttribute::CreateXorAddress(STUN_ATTR_XOR_MAPPED_ADDRESS);
rtc::SocketAddress test_addr(test_ip, kTestMessagePort3);
addr->SetAddress(test_addr);
EXPECT_TRUE(msg.AddAttribute(addr));
msg.AddAttribute(addr);
CheckStunHeader(msg, STUN_BINDING_RESPONSE, (size - 20));
@ -1076,7 +1076,7 @@ TEST_F(StunTest, WriteMessageWithAnErrorCodeAttribute) {
StunErrorCodeAttribute* errorcode = StunAttribute::CreateErrorCode();
errorcode->SetCode(kTestErrorCode);
errorcode->SetReason(kTestErrorReason);
EXPECT_TRUE(msg.AddAttribute(errorcode));
msg.AddAttribute(errorcode);
CheckStunHeader(msg, STUN_BINDING_ERROR_RESPONSE, (size - 20));
rtc::ByteBufferWriter out;
@ -1099,7 +1099,7 @@ TEST_F(StunTest, WriteMessageWithAUInt16ListAttribute) {
list->AddType(0x1U);
list->AddType(0x1000U);
list->AddType(0xAB0CU);
EXPECT_TRUE(msg.AddAttribute(list));
msg.AddAttribute(list);
CheckStunHeader(msg, STUN_BINDING_REQUEST, (size - 20));
rtc::ByteBufferWriter out;
@ -1120,7 +1120,7 @@ TEST_F(StunTest, WriteMessageWithOriginAttribute) {
kStunTransactionIdLength));
StunByteStringAttribute* origin =
new StunByteStringAttribute(STUN_ATTR_ORIGIN, kTestOrigin);
EXPECT_TRUE(msg.AddAttribute(origin));
msg.AddAttribute(origin);
rtc::ByteBufferWriter out;
EXPECT_TRUE(msg.Write(&out));
@ -1393,7 +1393,7 @@ TEST_F(StunTest, ReadRelayMessage) {
StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS);
addr2->SetPort(13);
addr2->SetIP(legacy_ip);
EXPECT_TRUE(msg2.AddAttribute(addr2));
msg2.AddAttribute(addr2);
const StunByteStringAttribute* bytes = msg.GetByteString(STUN_ATTR_USERNAME);
ASSERT_TRUE(bytes != NULL);
@ -1403,7 +1403,7 @@ TEST_F(StunTest, ReadRelayMessage) {
StunByteStringAttribute* bytes2 =
StunAttribute::CreateByteString(STUN_ATTR_USERNAME);
bytes2->CopyBytes("abcdefghijkl");
EXPECT_TRUE(msg2.AddAttribute(bytes2));
msg2.AddAttribute(bytes2);
const StunUInt32Attribute* uval = msg.GetUInt32(STUN_ATTR_LIFETIME);
ASSERT_TRUE(uval != NULL);
@ -1411,7 +1411,7 @@ TEST_F(StunTest, ReadRelayMessage) {
StunUInt32Attribute* uval2 = StunAttribute::CreateUInt32(STUN_ATTR_LIFETIME);
uval2->SetValue(11);
EXPECT_TRUE(msg2.AddAttribute(uval2));
msg2.AddAttribute(uval2);
bytes = msg.GetByteString(STUN_ATTR_MAGIC_COOKIE);
ASSERT_TRUE(bytes != NULL);
@ -1424,7 +1424,7 @@ TEST_F(StunTest, ReadRelayMessage) {
bytes2 = StunAttribute::CreateByteString(STUN_ATTR_MAGIC_COOKIE);
bytes2->CopyBytes(reinterpret_cast<const char*>(TURN_MAGIC_COOKIE_VALUE),
sizeof(TURN_MAGIC_COOKIE_VALUE));
EXPECT_TRUE(msg2.AddAttribute(bytes2));
msg2.AddAttribute(bytes2);
uval = msg.GetUInt32(STUN_ATTR_BANDWIDTH);
ASSERT_TRUE(uval != NULL);
@ -1432,7 +1432,7 @@ TEST_F(StunTest, ReadRelayMessage) {
uval2 = StunAttribute::CreateUInt32(STUN_ATTR_BANDWIDTH);
uval2->SetValue(6);
EXPECT_TRUE(msg2.AddAttribute(uval2));
msg2.AddAttribute(uval2);
addr = msg.GetAddress(STUN_ATTR_DESTINATION_ADDRESS);
ASSERT_TRUE(addr != NULL);
@ -1443,7 +1443,7 @@ TEST_F(StunTest, ReadRelayMessage) {
addr2 = StunAttribute::CreateAddress(STUN_ATTR_DESTINATION_ADDRESS);
addr2->SetPort(13);
addr2->SetIP(legacy_ip);
EXPECT_TRUE(msg2.AddAttribute(addr2));
msg2.AddAttribute(addr2);
addr = msg.GetAddress(STUN_ATTR_SOURCE_ADDRESS2);
ASSERT_TRUE(addr != NULL);
@ -1454,7 +1454,7 @@ TEST_F(StunTest, ReadRelayMessage) {
addr2 = StunAttribute::CreateAddress(STUN_ATTR_SOURCE_ADDRESS2);
addr2->SetPort(13);
addr2->SetIP(legacy_ip);
EXPECT_TRUE(msg2.AddAttribute(addr2));
msg2.AddAttribute(addr2);
bytes = msg.GetByteString(STUN_ATTR_DATA);
ASSERT_TRUE(bytes != NULL);
@ -1463,7 +1463,7 @@ TEST_F(StunTest, ReadRelayMessage) {
bytes2 = StunAttribute::CreateByteString(STUN_ATTR_DATA);
bytes2->CopyBytes("abcdefg");
EXPECT_TRUE(msg2.AddAttribute(bytes2));
msg2.AddAttribute(bytes2);
rtc::ByteBufferWriter out;
EXPECT_TRUE(msg.Write(&out));

38
webrtc/p2p/base/turnport.cc
View File

@ -928,12 +928,12 @@ void TurnPort::SendRequest(StunRequest* req, int delay) {
void TurnPort::AddRequestAuthInfo(StunMessage* msg) {
// If we've gotten the necessary data from the server, add it to our request.
VERIFY(!hash_.empty());
VERIFY(msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_USERNAME, credentials_.username)));
VERIFY(msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_REALM, realm_)));
VERIFY(msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_NONCE, nonce_)));
msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_USERNAME, credentials_.username));
msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_REALM, realm_));
msg->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_NONCE, nonce_));
VERIFY(msg->AddMessageIntegrity(hash()));
}
@ -1082,7 +1082,7 @@ void TurnAllocateRequest::Prepare(StunMessage* request) {
StunUInt32Attribute* transport_attr = StunAttribute::CreateUInt32(
STUN_ATTR_REQUESTED_TRANSPORT);
transport_attr->SetValue(IPPROTO_UDP << 24);
VERIFY(request->AddAttribute(transport_attr));
request->AddAttribute(transport_attr);
if (!port_->hash().empty()) {
port_->AddRequestAuthInfo(request);
}
@ -1256,8 +1256,8 @@ void TurnRefreshRequest::Prepare(StunMessage* request) {
// No attributes need to be included.
request->SetType(TURN_REFRESH_REQUEST);
if (lifetime_ > -1) {
VERIFY(request->AddAttribute(new StunUInt32Attribute(
STUN_ATTR_LIFETIME, lifetime_)));
request->AddAttribute(new StunUInt32Attribute(
STUN_ATTR_LIFETIME, lifetime_));
}
port_->AddRequestAuthInfo(request);
@ -1326,8 +1326,8 @@ TurnCreatePermissionRequest::TurnCreatePermissionRequest(
void TurnCreatePermissionRequest::Prepare(StunMessage* request) {
// Create the request as indicated in RFC5766, Section 9.1.
request->SetType(TURN_CREATE_PERMISSION_REQUEST);
VERIFY(request->AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_)));
request->AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_));
port_->AddRequestAuthInfo(request);
}
@ -1387,10 +1387,10 @@ TurnChannelBindRequest::TurnChannelBindRequest(
void TurnChannelBindRequest::Prepare(StunMessage* request) {
// Create the request as indicated in RFC5766, Section 11.1.
request->SetType(TURN_CHANNEL_BIND_REQUEST);
VERIFY(request->AddAttribute(new StunUInt32Attribute(
STUN_ATTR_CHANNEL_NUMBER, channel_id_ << 16)));
VERIFY(request->AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_)));
request->AddAttribute(new StunUInt32Attribute(
STUN_ATTR_CHANNEL_NUMBER, channel_id_ << 16));
request->AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_));
port_->AddRequestAuthInfo(request);
}
@ -1471,10 +1471,10 @@ int TurnEntry::Send(const void* data, size_t size, bool payload,
msg.SetType(TURN_SEND_INDICATION);
msg.SetTransactionID(
rtc::CreateRandomString(kStunTransactionIdLength));
VERIFY(msg.AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_)));
VERIFY(msg.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_DATA, data, size)));
msg.AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_));
msg.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_DATA, data, size));
VERIFY(msg.Write(&buf));
// If we're sending real data, request a channel bind that we can use later.

38
webrtc/p2p/base/turnserver.cc
View File

@ -113,8 +113,8 @@ static bool InitErrorResponse(const StunMessage* req, int code,
return false;
resp->SetType(resp_type);
resp->SetTransactionID(req->transaction_id());
VERIFY(resp->AddAttribute(new cricket::StunErrorCodeAttribute(
STUN_ATTR_ERROR_CODE, code, reason)));
resp->AddAttribute(new cricket::StunErrorCodeAttribute(
STUN_ATTR_ERROR_CODE, code, reason));
return true;
}
@ -356,7 +356,7 @@ void TurnServer::HandleBindingRequest(TurnServerConnection* conn,
StunAddressAttribute* mapped_addr_attr;
mapped_addr_attr = new StunXorAddressAttribute(
STUN_ATTR_XOR_MAPPED_ADDRESS, conn->src());
VERIFY(response.AddAttribute(mapped_addr_attr));
response.AddAttribute(mapped_addr_attr);
SendStun(conn, &response);
}
@ -470,10 +470,10 @@ void TurnServer::SendErrorResponseWithRealmAndNonce(
timestamp = ts_for_next_nonce_;
ts_for_next_nonce_ = 0;
}
VERIFY(resp.AddAttribute(
new StunByteStringAttribute(STUN_ATTR_NONCE, GenerateNonce(timestamp))));
VERIFY(resp.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_REALM, realm_)));
resp.AddAttribute(
new StunByteStringAttribute(STUN_ATTR_NONCE, GenerateNonce(timestamp)));
resp.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_REALM, realm_));
SendStun(conn, &resp);
}
@ -483,8 +483,8 @@ void TurnServer::SendErrorResponseWithAlternateServer(
TurnMessage resp;
InitErrorResponse(msg, STUN_ERROR_TRY_ALTERNATE,
STUN_ERROR_REASON_TRY_ALTERNATE_SERVER, &resp);
VERIFY(resp.AddAttribute(new StunAddressAttribute(
STUN_ATTR_ALTERNATE_SERVER, addr)));
resp.AddAttribute(new StunAddressAttribute(
STUN_ATTR_ALTERNATE_SERVER, addr));
SendStun(conn, &resp);
}
@ -492,8 +492,8 @@ void TurnServer::SendStun(TurnServerConnection* conn, StunMessage* msg) {
rtc::ByteBufferWriter buf;
// Add a SOFTWARE attribute if one is set.
if (!software_.empty()) {
VERIFY(msg->AddAttribute(
new StunByteStringAttribute(STUN_ATTR_SOFTWARE, software_)));
msg->AddAttribute(
new StunByteStringAttribute(STUN_ATTR_SOFTWARE, software_));
}
msg->Write(&buf);
Send(conn, buf);
@ -658,9 +658,9 @@ void TurnServerAllocation::HandleAllocateRequest(const TurnMessage* msg) {
external_socket_->GetLocalAddress());
StunUInt32Attribute* lifetime_attr =
new StunUInt32Attribute(STUN_ATTR_LIFETIME, lifetime_secs);
VERIFY(response.AddAttribute(mapped_addr_attr));
VERIFY(response.AddAttribute(relayed_addr_attr));
VERIFY(response.AddAttribute(lifetime_attr));
response.AddAttribute(mapped_addr_attr);
response.AddAttribute(relayed_addr_attr);
response.AddAttribute(lifetime_attr);
SendResponse(&response);
}
@ -682,7 +682,7 @@ void TurnServerAllocation::HandleRefreshRequest(const TurnMessage* msg) {
StunUInt32Attribute* lifetime_attr =
new StunUInt32Attribute(STUN_ATTR_LIFETIME, lifetime_secs);
VERIFY(response.AddAttribute(lifetime_attr));
response.AddAttribute(lifetime_attr);
SendResponse(&response);
}
@ -819,10 +819,10 @@ void TurnServerAllocation::OnExternalPacket(
msg.SetType(TURN_DATA_INDICATION);
msg.SetTransactionID(
rtc::CreateRandomString(kStunTransactionIdLength));
VERIFY(msg.AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, addr)));
VERIFY(msg.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_DATA, data, size)));
msg.AddAttribute(new StunXorAddressAttribute(
STUN_ATTR_XOR_PEER_ADDRESS, addr));
msg.AddAttribute(new StunByteStringAttribute(
STUN_ATTR_DATA, data, size));
server_->SendStun(&conn_, &msg);
} else {
LOG_J(LS_WARNING, this) << "Received external packet without permission, "