diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 72f92b20ab..93fda17e7c 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1411,12 +1411,7 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request, void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request, StunMessage* response) { - const StunErrorCodeAttribute* error_attr = response->GetErrorCode(); - int error_code = STUN_ERROR_GLOBAL_FAILURE; - if (error_attr) { - error_code = error_attr->code(); - } - + int error_code = response->GetErrorCodeValue(); LOG_J(LS_INFO, this) << "Received STUN error response" << " id=" << rtc::hex_encode(request->id()) << " code=" << error_code diff --git a/webrtc/p2p/base/relayport.cc b/webrtc/p2p/base/relayport.cc index 5eacf6d708..e353511f80 100644 --- a/webrtc/p2p/base/relayport.cc +++ b/webrtc/p2p/base/relayport.cc @@ -824,7 +824,7 @@ void AllocateRequest::OnResponse(StunMessage* response) { void AllocateRequest::OnErrorResponse(StunMessage* response) { const StunErrorCodeAttribute* attr = response->GetErrorCode(); if (!attr) { - LOG(INFO) << "Bad allocate response error code"; + LOG(LS_ERROR) << "Missing allocate response error code."; } else { LOG(INFO) << "Allocate error response:" << " code=" << attr->code() diff --git a/webrtc/p2p/base/stun.cc b/webrtc/p2p/base/stun.cc index 4d6b21ba54..8217c029c3 100644 --- a/webrtc/p2p/base/stun.cc +++ b/webrtc/p2p/base/stun.cc @@ -115,6 +115,11 @@ const StunErrorCodeAttribute* StunMessage::GetErrorCode() const { GetAttribute(STUN_ATTR_ERROR_CODE)); } +int StunMessage::GetErrorCodeValue() const { + const StunErrorCodeAttribute* error_attribute = GetErrorCode(); + return error_attribute ? error_attribute->code() : STUN_ERROR_GLOBAL_FAILURE; +} + const StunUInt16ListAttribute* StunMessage::GetUnknownAttributes() const { return static_cast( GetAttribute(STUN_ATTR_UNKNOWN_ATTRIBUTES)); diff --git a/webrtc/p2p/base/stun.h b/webrtc/p2p/base/stun.h index ab200687f5..4ea45050ca 100644 --- a/webrtc/p2p/base/stun.h +++ b/webrtc/p2p/base/stun.h @@ -156,6 +156,9 @@ class StunMessage { // Gets these specific attribute values. const StunErrorCodeAttribute* GetErrorCode() const; + // Returns the code inside the error code attribute, if present, and + // STUN_ERROR_GLOBAL_FAILURE otherwise. + int GetErrorCodeValue() const; const StunUInt16ListAttribute* GetUnknownAttributes() const; // Takes ownership of the specified attribute and adds it to the message. diff --git a/webrtc/p2p/base/stun_unittest.cc b/webrtc/p2p/base/stun_unittest.cc index efc11b2408..15d4648f27 100644 --- a/webrtc/p2p/base/stun_unittest.cc +++ b/webrtc/p2p/base/stun_unittest.cc @@ -1016,6 +1016,15 @@ TEST_F(StunTest, ReadErrorCodeAttribute) { EXPECT_EQ(kTestErrorNumber, errorcode->number()); EXPECT_EQ(kTestErrorReason, errorcode->reason()); EXPECT_EQ(kTestErrorCode, errorcode->code()); + EXPECT_EQ(kTestErrorCode, msg.GetErrorCodeValue()); +} + +// Test that GetErrorCodeValue returns STUN_ERROR_GLOBAL_FAILURE if the message +// in question doesn't have an error code attribute, rather than crashing. +TEST_F(StunTest, GetErrorCodeValueWithNoErrorAttribute) { + StunMessage msg; + ReadStunMessage(&msg, kStunMessageWithIPv6MappedAddress); + EXPECT_EQ(STUN_ERROR_GLOBAL_FAILURE, msg.GetErrorCodeValue()); } TEST_F(StunTest, ReadMessageWithAUInt16ListAttribute) { diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc index 43e8d9fb5c..5801ba7dd6 100644 --- a/webrtc/p2p/base/stunport.cc +++ b/webrtc/p2p/base/stunport.cc @@ -66,7 +66,7 @@ class StunBindingRequest : public StunRequest { virtual void OnErrorResponse(StunMessage* response) override { const StunErrorCodeAttribute* attr = response->GetErrorCode(); if (!attr) { - LOG(LS_ERROR) << "Bad allocate response error code"; + LOG(LS_ERROR) << "Missing binding response error code."; } else { LOG(LS_ERROR) << "Binding error response:" << " class=" << attr->eclass() diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 9015169221..56dadde9bb 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -1165,19 +1165,18 @@ void TurnAllocateRequest::OnResponse(StunMessage* response) { void TurnAllocateRequest::OnErrorResponse(StunMessage* response) { // Process error response according to RFC5766, Section 6.4. - const StunErrorCodeAttribute* error_code = response->GetErrorCode(); + int error_code = response->GetErrorCodeValue(); LOG_J(LS_INFO, port_) << "Received TURN allocate error response" << ", id=" << rtc::hex_encode(id()) - << ", code=" << error_code->code() - << ", rtt=" << Elapsed(); + << ", code=" << error_code << ", rtt=" << Elapsed(); - switch (error_code->code()) { + switch (error_code) { case STUN_ERROR_UNAUTHORIZED: // Unauthrorized. - OnAuthChallenge(response, error_code->code()); + OnAuthChallenge(response, error_code); break; case STUN_ERROR_TRY_ALTERNATE: - OnTryAlternate(response, error_code->code()); + OnTryAlternate(response, error_code); break; case STUN_ERROR_ALLOCATION_MISMATCH: // We must handle this error async because trying to delete the socket in @@ -1186,10 +1185,10 @@ void TurnAllocateRequest::OnErrorResponse(StunMessage* response) { TurnPort::MSG_ALLOCATE_MISMATCH); break; default: - LOG_J(LS_WARNING, port_) << "Received TURN allocate error response" - << ", id=" << rtc::hex_encode(id()) - << ", code=" << error_code->code() - << ", rtt=" << Elapsed(); + LOG_J(LS_WARNING, port_) + << "Received TURN allocate error response" + << ", id=" << rtc::hex_encode(id()) << ", code=" << error_code + << ", rtt=" << Elapsed(); port_->OnAllocateError(); } } @@ -1321,20 +1320,20 @@ void TurnRefreshRequest::OnResponse(StunMessage* response) { } void TurnRefreshRequest::OnErrorResponse(StunMessage* response) { - const StunErrorCodeAttribute* error_code = response->GetErrorCode(); + int error_code = response->GetErrorCodeValue(); - if (error_code->code() == STUN_ERROR_STALE_NONCE) { + if (error_code == STUN_ERROR_STALE_NONCE) { if (port_->UpdateNonce(response)) { // Send RefreshRequest immediately. port_->SendRequest(new TurnRefreshRequest(port_), 0); } } else { - LOG_J(LS_WARNING, port_) << "Received TURN refresh error response" - << ", id=" << rtc::hex_encode(id()) - << ", code=" << error_code->code() - << ", rtt=" << Elapsed(); + LOG_J(LS_WARNING, port_) + << "Received TURN refresh error response" + << ", id=" << rtc::hex_encode(id()) << ", code=" << error_code + << ", rtt=" << Elapsed(); port_->OnRefreshError(); - port_->SignalTurnRefreshResult(port_, error_code->code()); + port_->SignalTurnRefreshResult(port_, error_code); } } @@ -1380,13 +1379,12 @@ void TurnCreatePermissionRequest::OnResponse(StunMessage* response) { } void TurnCreatePermissionRequest::OnErrorResponse(StunMessage* response) { - const StunErrorCodeAttribute* error_code = response->GetErrorCode(); + int error_code = response->GetErrorCodeValue(); LOG_J(LS_WARNING, port_) << "Received TURN create permission error response" << ", id=" << rtc::hex_encode(id()) - << ", code=" << error_code->code() - << ", rtt=" << Elapsed(); + << ", code=" << error_code << ", rtt=" << Elapsed(); if (entry_) { - entry_->OnCreatePermissionError(response, error_code->code()); + entry_->OnCreatePermissionError(response, error_code); } } @@ -1450,13 +1448,12 @@ void TurnChannelBindRequest::OnResponse(StunMessage* response) { } void TurnChannelBindRequest::OnErrorResponse(StunMessage* response) { - const StunErrorCodeAttribute* error_code = response->GetErrorCode(); + int error_code = response->GetErrorCodeValue(); LOG_J(LS_WARNING, port_) << "Received TURN channel bind error response" << ", id=" << rtc::hex_encode(id()) - << ", code=" << error_code->code() - << ", rtt=" << Elapsed(); + << ", code=" << error_code << ", rtt=" << Elapsed(); if (entry_) { - entry_->OnChannelBindError(response, error_code->code()); + entry_->OnChannelBindError(response, error_code); } }