diff options
| -rw-r--r-- | remoting/host/heartbeat_sender.cc | 3 | ||||
| -rw-r--r-- | remoting/host/heartbeat_sender.h | 2 | ||||
| -rw-r--r-- | remoting/host/heartbeat_sender_unittest.cc | 4 | ||||
| -rw-r--r-- | remoting/host/register_support_host_request.cc | 3 | ||||
| -rw-r--r-- | remoting/host/register_support_host_request.h | 2 | ||||
| -rw-r--r-- | remoting/host/register_support_host_request_unittest.cc | 2 | ||||
| -rw-r--r-- | remoting/jingle_glue/iq_sender.cc | 60 | ||||
| -rw-r--r-- | remoting/jingle_glue/iq_sender.h | 19 | ||||
| -rw-r--r-- | remoting/jingle_glue/iq_sender_unittest.cc | 105 | ||||
| -rw-r--r-- | remoting/jingle_glue/jingle_info_request.cc | 3 | ||||
| -rw-r--r-- | remoting/jingle_glue/jingle_info_request.h | 4 | ||||
| -rw-r--r-- | remoting/protocol/jingle_messages.cc | 7 | ||||
| -rw-r--r-- | remoting/protocol/jingle_messages.h | 3 | ||||
| -rw-r--r-- | remoting/protocol/jingle_session.cc | 153 | ||||
| -rw-r--r-- | remoting/protocol/jingle_session.h | 24 |
15 files changed, 278 insertions, 116 deletions
diff --git a/remoting/host/heartbeat_sender.cc b/remoting/host/heartbeat_sender.cc index e8dacd2..16ec93e 100644 --- a/remoting/host/heartbeat_sender.cc +++ b/remoting/host/heartbeat_sender.cc @@ -106,7 +106,8 @@ void HeartbeatSender::DoSendStanza() { ++sequence_id_; } -void HeartbeatSender::ProcessResponse(const XmlElement* response) { +void HeartbeatSender::ProcessResponse(IqRequest* request, + const XmlElement* response) { std::string type = response->Attr(buzz::QN_TYPE); if (type == buzz::STR_ERROR) { const XmlElement* error_element = diff --git a/remoting/host/heartbeat_sender.h b/remoting/host/heartbeat_sender.h index daa6b3a..9f48f78 100644 --- a/remoting/host/heartbeat_sender.h +++ b/remoting/host/heartbeat_sender.h @@ -103,7 +103,7 @@ class HeartbeatSender : public SignalStrategy::Listener { void SendStanza(); void ResendStanza(); void DoSendStanza(); - void ProcessResponse(const buzz::XmlElement* response); + void ProcessResponse(IqRequest* request, const buzz::XmlElement* response); void SetInterval(int interval); void SetSequenceId(int sequence_id); diff --git a/remoting/host/heartbeat_sender_unittest.cc b/remoting/host/heartbeat_sender_unittest.cc index 08f3267..f94726d 100644 --- a/remoting/host/heartbeat_sender_unittest.cc +++ b/remoting/host/heartbeat_sender_unittest.cc @@ -176,7 +176,7 @@ TEST_F(HeartbeatSenderTest, DoSendStanzaWithExpectedSequenceId) { result->AddElement(expected_sequence_id); const int kExpectedSequenceId = 456; expected_sequence_id->AddText(base::IntToString(kExpectedSequenceId)); - heartbeat_sender_->ProcessResponse(response.get()); + heartbeat_sender_->ProcessResponse(NULL, response.get()); message_loop_.RunAllPending(); scoped_ptr<XmlElement> stanza2(sent_iq2); @@ -204,7 +204,7 @@ TEST_F(HeartbeatSenderTest, ProcessResponseSetInterval) { const int kTestInterval = 123; set_interval->AddText(base::IntToString(kTestInterval)); - heartbeat_sender_->ProcessResponse(response.get()); + heartbeat_sender_->ProcessResponse(NULL, response.get()); EXPECT_EQ(kTestInterval * 1000, heartbeat_sender_->interval_ms_); } diff --git a/remoting/host/register_support_host_request.cc b/remoting/host/register_support_host_request.cc index cf84d2c..f46b67f 100644 --- a/remoting/host/register_support_host_request.cc +++ b/remoting/host/register_support_host_request.cc @@ -161,7 +161,8 @@ bool RegisterSupportHostRequest::ParseResponse(const XmlElement* response, return true; } -void RegisterSupportHostRequest::ProcessResponse(const XmlElement* response) { +void RegisterSupportHostRequest::ProcessResponse(IqRequest* request, + const XmlElement* response) { std::string support_id; base::TimeDelta lifetime; bool success = ParseResponse(response, &support_id, &lifetime); diff --git a/remoting/host/register_support_host_request.h b/remoting/host/register_support_host_request.h index b05e880..86c4e07 100644 --- a/remoting/host/register_support_host_request.h +++ b/remoting/host/register_support_host_request.h @@ -67,7 +67,7 @@ class RegisterSupportHostRequest : public SignalStrategy::Listener { const std::string& jid); scoped_ptr<buzz::XmlElement> CreateSignature(const std::string& jid); - void ProcessResponse(const buzz::XmlElement* response); + void ProcessResponse(IqRequest* request, const buzz::XmlElement* response); bool ParseResponse(const buzz::XmlElement* response, std::string* support_id, base::TimeDelta* lifetime); diff --git a/remoting/host/register_support_host_request_unittest.cc b/remoting/host/register_support_host_request_unittest.cc index 5d1a53a..4f091b8 100644 --- a/remoting/host/register_support_host_request_unittest.cc +++ b/remoting/host/register_support_host_request_unittest.cc @@ -74,7 +74,6 @@ class RegisterSupportHostRequestTest : public testing::Test { MockCallback callback_; }; - TEST_F(RegisterSupportHostRequestTest, Send) { // |iq_request| is freed by RegisterSupportHostRequest. int64 start_time = static_cast<int64>(base::Time::Now().ToDoubleT()); @@ -128,6 +127,7 @@ TEST_F(RegisterSupportHostRequestTest, Send) { base::TimeDelta::FromSeconds(300))); scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ)); + response->AddAttr(QName("", "from"), kChromotingBotJid); response->AddAttr(QName("", "type"), "result"); response->AddAttr(QName("", "id"), kStanzaId); diff --git a/remoting/jingle_glue/iq_sender.cc b/remoting/jingle_glue/iq_sender.cc index eb8f17a..ca0b3f6 100644 --- a/remoting/jingle_glue/iq_sender.cc +++ b/remoting/jingle_glue/iq_sender.cc @@ -4,7 +4,10 @@ #include "remoting/jingle_glue/iq_sender.h" +#include "base/bind.h" +#include "base/location.h" #include "base/logging.h" +#include "base/message_loop_proxy.h" #include "base/string_number_conversions.h" #include "remoting/jingle_glue/signal_strategy.h" #include "third_party/libjingle/source/talk/xmllite/xmlelement.h" @@ -36,13 +39,14 @@ IqSender::~IqSender() { scoped_ptr<IqRequest> IqSender::SendIq(scoped_ptr<buzz::XmlElement> stanza, const ReplyCallback& callback) { + std::string addressee = stanza->Attr(buzz::QN_TO); std::string id = signal_strategy_->GetNextId(); stanza->AddAttr(buzz::QN_ID, id); if (!signal_strategy_->SendStanza(stanza.Pass())) { return scoped_ptr<IqRequest>(NULL); } DCHECK(requests_.find(id) == requests_.end()); - scoped_ptr<IqRequest> request(new IqRequest(this, callback)); + scoped_ptr<IqRequest> request(new IqRequest(this, callback, addressee)); if (!callback.is_null()) requests_[id] = request.get(); return request.Pass(); @@ -92,32 +96,72 @@ bool IqSender::OnSignalStrategyIncomingStanza(const buzz::XmlElement* stanza) { return false; } + std::string from = stanza->Attr(buzz::QN_FROM); + IqRequestMap::iterator it = requests_.find(id); + + // This is a hack to workaround the issue with the WCS changing IDs + // of IQ responses sent from client to host. Whenever we receive a + // stanza with an unknown ID we try to match it with an outstanding + // request sent to the same peer. + if (it == requests_.end()) { + for (it = requests_.begin(); it != requests_.end(); ++it) { + if (it->second->addressee_ == from) { + break; + } + } + } + if (it == requests_.end()) { return false; } IqRequest* request = it->second; - requests_.erase(it); + if (request->addressee_ != from) { + LOG(ERROR) << "Received IQ response from from a invalid JID. Ignoring it." + << " Message received from: " << from + << " Original JID: " << request->addressee_; + return false; + } + + requests_.erase(it); request->OnResponse(stanza); + return true; } -IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback) +IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback, + const std::string& addressee) : sender_(sender), - callback_(callback) { + callback_(callback), + addressee_(addressee) { } IqRequest::~IqRequest() { sender_->RemoveRequest(this); } +void IqRequest::SetTimeout(base::TimeDelta timeout) { + base::MessageLoopProxy::current()->PostDelayedTask( + FROM_HERE, base::Bind(&IqRequest::OnTimeout, AsWeakPtr()), + timeout.InMilliseconds()); +} + +void IqRequest::CallCallback(const buzz::XmlElement* stanza) { + if (!callback_.is_null()) { + IqSender::ReplyCallback callback(callback_); + callback_.Reset(); + callback.Run(this, stanza); + } +} + +void IqRequest::OnTimeout() { + CallCallback(NULL); +} + void IqRequest::OnResponse(const buzz::XmlElement* stanza) { - DCHECK(!callback_.is_null()); - IqSender::ReplyCallback callback(callback_); - callback_.Reset(); - callback.Run(stanza); + CallCallback(stanza); } } // namespace remoting diff --git a/remoting/jingle_glue/iq_sender.h b/remoting/jingle_glue/iq_sender.h index 5244584..677bd472 100644 --- a/remoting/jingle_glue/iq_sender.h +++ b/remoting/jingle_glue/iq_sender.h @@ -12,6 +12,7 @@ #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "remoting/jingle_glue/signal_strategy.h" namespace buzz { @@ -27,7 +28,10 @@ class SignalStrategy; // those requests. class IqSender : public SignalStrategy::Listener { public: - typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback; + // Callback that is called when an Iq response is received. Called + // with the |response| set to NULL in case of a timeout. + typedef base::Callback<void(IqRequest* request, + const buzz::XmlElement* response)> ReplyCallback; explicit IqSender(SignalStrategy* signal_strategy); virtual ~IqSender(); @@ -72,19 +76,28 @@ class IqSender : public SignalStrategy::Listener { }; // This call must only be used on the thread it was created on. -class IqRequest { +class IqRequest : public base::SupportsWeakPtr<IqRequest> { public: - IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback); + IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback, + const std::string& addressee); ~IqRequest(); + // Sets timeout for the request. When the timeout expires the + // callback is called with the |response| set to NULL. + void SetTimeout(base::TimeDelta timeout); + private: friend class IqSender; + void CallCallback(const buzz::XmlElement* stanza); + void OnTimeout(); + // Called by IqSender when a response is received. void OnResponse(const buzz::XmlElement* stanza); IqSender* sender_; IqSender::ReplyCallback callback_; + std::string addressee_; DISALLOW_COPY_AND_ASSIGN(IqRequest); }; diff --git a/remoting/jingle_glue/iq_sender_unittest.cc b/remoting/jingle_glue/iq_sender_unittest.cc index e14cf2a..06730f5 100644 --- a/remoting/jingle_glue/iq_sender_unittest.cc +++ b/remoting/jingle_glue/iq_sender_unittest.cc @@ -4,6 +4,7 @@ #include "base/bind.h" #include "base/memory/ref_counted.h" +#include "base/message_loop.h" #include "base/stringprintf.h" #include "remoting/jingle_glue/iq_sender.h" #include "remoting/jingle_glue/mock_objects.h" @@ -14,6 +15,7 @@ using ::testing::_; using ::testing::DeleteArg; +using ::testing::InvokeWithoutArgs; using ::testing::NotNull; using ::testing::Return; using ::testing::SaveArg; @@ -34,7 +36,7 @@ const char kTo[] = "user@domain.com"; class MockCallback { public: - MOCK_METHOD1(OnReply, void(const XmlElement* reply)); + MOCK_METHOD2(OnReply, void(IqRequest* request, const XmlElement* reply)); }; } // namespace @@ -49,43 +51,100 @@ class IqSenderTest : public testing::Test { } protected: + void SendTestMessage() { + scoped_ptr<XmlElement> iq_body( + new XmlElement(QName(kNamespace, kBodyTag))); + XmlElement* sent_stanza; + EXPECT_CALL(signal_strategy_, GetNextId()) + .WillOnce(Return(kStanzaId)); + EXPECT_CALL(signal_strategy_, SendStanzaPtr(_)) + .WillOnce(DoAll(SaveArg<0>(&sent_stanza), Return(true))); + request_ = sender_->SendIq(kType, kTo, iq_body.Pass(), base::Bind( + &MockCallback::OnReply, base::Unretained(&callback_))); + + std::string expected_xml_string = + base::StringPrintf( + "<cli:iq type=\"%s\" to=\"%s\" id=\"%s\" " + "xmlns:cli=\"jabber:client\">" + "<%s:%s xmlns:%s=\"%s\"/>" + "</cli:iq>", + kType, kTo, kStanzaId, kNamespacePrefix, kBodyTag, + kNamespacePrefix, kNamespace); + EXPECT_EQ(expected_xml_string, sent_stanza->Str()); + delete sent_stanza; + } + + MessageLoop message_loop_; MockSignalStrategy signal_strategy_; scoped_ptr<IqSender> sender_; MockCallback callback_; + scoped_ptr<IqRequest> request_; }; TEST_F(IqSenderTest, SendIq) { - scoped_ptr<XmlElement> iq_body( - new XmlElement(QName(kNamespace, kBodyTag))); - XmlElement* sent_stanza; - EXPECT_CALL(signal_strategy_, GetNextId()) - .WillOnce(Return(kStanzaId)); - EXPECT_CALL(signal_strategy_, SendStanzaPtr(_)) - .WillOnce(DoAll(SaveArg<0>(&sent_stanza), Return(true))); - scoped_ptr<IqRequest> request = - sender_->SendIq(kType, kTo, iq_body.Pass(), base::Bind( - &MockCallback::OnReply, base::Unretained(&callback_))); - - std::string expected_xml_string = - base::StringPrintf( - "<cli:iq type=\"%s\" to=\"%s\" id=\"%s\" " - "xmlns:cli=\"jabber:client\">" - "<%s:%s xmlns:%s=\"%s\"/>" - "</cli:iq>", - kType, kTo, kStanzaId, kNamespacePrefix, kBodyTag, - kNamespacePrefix, kNamespace); - EXPECT_EQ(expected_xml_string, sent_stanza->Str()); - delete sent_stanza; + ASSERT_NO_FATAL_FAILURE({ + SendTestMessage(); + }); + + scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ)); + response->AddAttr(QName("", "type"), "result"); + response->AddAttr(QName("", "id"), kStanzaId); + response->AddAttr(QName("", "from"), kTo); + + XmlElement* result = new XmlElement( + QName("test:namespace", "response-body")); + response->AddElement(result); + + EXPECT_CALL(callback_, OnReply(request_.get(), response.get())); + EXPECT_TRUE(sender_->OnSignalStrategyIncomingStanza(response.get())); +} + +TEST_F(IqSenderTest, Timeout) { + ASSERT_NO_FATAL_FAILURE({ + SendTestMessage(); + }); + + request_->SetTimeout(base::TimeDelta::FromMilliseconds(2)); + + EXPECT_CALL(callback_, OnReply(request_.get(), NULL)) + .WillOnce(InvokeWithoutArgs(&message_loop_, &MessageLoop::Quit)); + message_loop_.Run(); +} + +TEST_F(IqSenderTest, InvalidFrom) { + ASSERT_NO_FATAL_FAILURE({ + SendTestMessage(); + }); scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ)); response->AddAttr(QName("", "type"), "result"); response->AddAttr(QName("", "id"), kStanzaId); + response->AddAttr(QName("", "from"), "different_user@domain.com"); + + XmlElement* result = new XmlElement( + QName("test:namespace", "response-body")); + response->AddElement(result); + + EXPECT_CALL(callback_, OnReply(request_.get(), response.get())) + .Times(0); + EXPECT_FALSE(sender_->OnSignalStrategyIncomingStanza(response.get())); +} + +TEST_F(IqSenderTest, IdMatchingHack) { + ASSERT_NO_FATAL_FAILURE({ + SendTestMessage(); + }); + + scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ)); + response->AddAttr(QName("", "type"), "result"); + response->AddAttr(QName("", "id"), "DIFFERENT_ID"); + response->AddAttr(QName("", "from"), kTo); XmlElement* result = new XmlElement( QName("test:namespace", "response-body")); response->AddElement(result); - EXPECT_CALL(callback_, OnReply(response.get())); + EXPECT_CALL(callback_, OnReply(request_.get(), response.get())); EXPECT_TRUE(sender_->OnSignalStrategyIncomingStanza(response.get())); } diff --git a/remoting/jingle_glue/jingle_info_request.cc b/remoting/jingle_glue/jingle_info_request.cc index 56d93ca..eb5bd7b 100644 --- a/remoting/jingle_glue/jingle_info_request.cc +++ b/remoting/jingle_glue/jingle_info_request.cc @@ -32,7 +32,8 @@ void JingleInfoRequest::Send(const OnJingleInfoCallback& callback) { base::Bind(&JingleInfoRequest::OnResponse, base::Unretained(this))); } -void JingleInfoRequest::OnResponse(const buzz::XmlElement* stanza) { +void JingleInfoRequest::OnResponse(IqRequest* request, + const buzz::XmlElement* stanza) { const buzz::XmlElement* query = stanza->FirstNamed(buzz::QN_JINGLE_INFO_QUERY); if (query == NULL) { diff --git a/remoting/jingle_glue/jingle_info_request.h b/remoting/jingle_glue/jingle_info_request.h index 9843586..a1b10de 100644 --- a/remoting/jingle_glue/jingle_info_request.h +++ b/remoting/jingle_glue/jingle_info_request.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -54,7 +54,7 @@ class JingleInfoRequest : public sigslot::has_slots<> { private: struct PendingDnsRequest; - void OnResponse(const buzz::XmlElement* stanza); + void OnResponse(IqRequest* request, const buzz::XmlElement* stanza); IqSender iq_sender_; scoped_ptr<IqRequest> request_; diff --git a/remoting/protocol/jingle_messages.cc b/remoting/protocol/jingle_messages.cc index 8fe749d..a1b87ff 100644 --- a/remoting/protocol/jingle_messages.cc +++ b/remoting/protocol/jingle_messages.cc @@ -141,6 +141,11 @@ bool JingleMessage::IsJingleMessage(const buzz::XmlElement* stanza) { stanza->FirstNamed(QName(kJingleNamespace, "jingle")) != NULL; } +// static +std::string JingleMessage::GetActionName(ActionType action) { + return ValueToName(kActionTypes, arraysize(kActionTypes), action); +} + JingleMessage::JingleMessage() : action(UNKNOWN_ACTION), reason(UNKNOWN_REASON) { @@ -268,7 +273,7 @@ bool JingleMessage::ParseXml(const buzz::XmlElement* stanza, return true; } -scoped_ptr<buzz::XmlElement> JingleMessage::ToXml() { +scoped_ptr<buzz::XmlElement> JingleMessage::ToXml() const { scoped_ptr<XmlElement> root( new XmlElement(QName("jabber:client", "iq"), true)); diff --git a/remoting/protocol/jingle_messages.h b/remoting/protocol/jingle_messages.h index 03e5063..3dcdd41 100644 --- a/remoting/protocol/jingle_messages.h +++ b/remoting/protocol/jingle_messages.h @@ -52,12 +52,13 @@ struct JingleMessage { // Caller keeps ownership of |stanza|. static bool IsJingleMessage(const buzz::XmlElement* stanza); + static std::string GetActionName(ActionType action); // Caller keeps ownership of |stanza|. |error| is set to debug error // message when parsing fails. bool ParseXml(const buzz::XmlElement* stanza, std::string* error); - scoped_ptr<buzz::XmlElement> ToXml(); + scoped_ptr<buzz::XmlElement> ToXml() const; std::string from; std::string to; diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc index 61d4bc9..979499a 100644 --- a/remoting/protocol/jingle_session.cc +++ b/remoting/protocol/jingle_session.cc @@ -32,6 +32,11 @@ namespace { // process. const int kTransportInfoSendDelayMs = 2; +// How long we should wait for a response from the other end. This +// value is used for all requests include |session-initiate| and +// |transport-info|. +const int kMessageResponseTimeoutSeconds = 10; + Session::Error AuthRejectionReasonToError( Authenticator::RejectionReason reason) { switch (reason) { @@ -54,6 +59,8 @@ JingleSession::JingleSession(JingleSessionManager* session_manager) } JingleSession::~JingleSession() { + STLDeleteContainerPointers(pending_requests_.begin(), + pending_requests_.end()); STLDeleteContainerPairSecondPointers(channels_.begin(), channels_.end()); session_manager_->SessionDestroyed(this); } @@ -102,10 +109,7 @@ void JingleSession::StartConnection( message.description.reset( new ContentDescription(candidate_config_->Clone(), authenticator_->GetNextMessage())); - initiate_request_ = session_manager_->iq_sender()->SendIq( - message.ToXml(), - base::Bind(&JingleSession::OnSessionInitiateResponse, - base::Unretained(this))); + SendMessage(message); SetState(CONNECTING); } @@ -158,10 +162,7 @@ void JingleSession::AcceptIncomingConnection( message.description.reset( new ContentDescription(CandidateSessionConfig::CreateFrom(config_), auth_message.Pass())); - initiate_request_ = session_manager_->iq_sender()->SendIq( - message.ToXml(), - base::Bind(&JingleSession::OnSessionInitiateResponse, - base::Unretained(this))); + SendMessage(message); // Update state. SetState(CONNECTED); @@ -175,20 +176,6 @@ void JingleSession::AcceptIncomingConnection( return; } -void JingleSession::OnSessionInitiateResponse( - const buzz::XmlElement* response) { - const std::string& type = response->Attr(buzz::QName("", "type")); - if (type != "result") { - LOG(ERROR) << "Received error in response to session-initiate message: \"" - << response->Str() - << "\". Terminating the session."; - - // TODO(sergeyu): There may be different reasons for error - // here. Parse the response stanza to find failure reason. - CloseInternal(PEER_IS_OFFLINE); - } -} - void JingleSession::CreateStreamChannel( const std::string& name, const StreamChannelCallback& callback) { @@ -282,6 +269,87 @@ void JingleSession::OnTransportDeleted(Transport* transport) { channels_.erase(it); } +void JingleSession::SendMessage(const JingleMessage& message) { + scoped_ptr<IqRequest> request = session_manager_->iq_sender()->SendIq( + message.ToXml(), + base::Bind(&JingleSession::OnMessageResponse, + base::Unretained(this), message.action)); + if (request.get()) { + request->SetTimeout( + base::TimeDelta::FromSeconds(kMessageResponseTimeoutSeconds)); + pending_requests_.push_back(request.release()); + } else { + LOG(ERROR) << "Failed to send a " + << JingleMessage::GetActionName(message.action) << " message"; + } +} + +void JingleSession::OnMessageResponse( + JingleMessage::ActionType request_type, + IqRequest* request, + const buzz::XmlElement* response) { + Error error = OK; + + std::string type_str = JingleMessage::GetActionName(request_type); + + if (!response) { + LOG(ERROR) << type_str << " request timed out."; + // Most likely the session-initiate timeout indicates a problem + // with the signaling. + error = UNKNOWN_ERROR; + } else { + const std::string& type = response->Attr(buzz::QName("", "type")); + if (type != "result") { + LOG(ERROR) << "Received error in response to " << type_str + << " message: \"" << response->Str() + << "\". Terminating the session."; + + switch (request_type) { + case JingleMessage::SESSION_INFO: + // session-info is used for the new authentication protocol, + // and wasn't previously supported. + error = INCOMPATIBLE_PROTOCOL; + + default: + // TODO(sergeyu): There may be different reasons for error + // here. Parse the response stanza to find failure reason. + error = PEER_IS_OFFLINE; + } + } + } + + CleanupPendingRequests(request); + + if (error != OK) { + CloseInternal(error); + } +} + +void JingleSession::CleanupPendingRequests(IqRequest* request) { + DCHECK(!pending_requests_.empty()); + DCHECK(request); + + // This method is called whenever a response to |request| is + // received. Here we delete that request and all requests that were + // sent before it. The idea here is that if we send messages A, B + // and C and then suddenly receive response to C then it means that + // either A and B messages or the corresponding response messages + // were somehow lost. E.g. that may happen when the client switches + // from one network to another. The best way to handle that case is + // to ignore errors and timeouts for A and B by deleting the + // corresponding IqRequest objects. + while (!pending_requests_.empty() && pending_requests_.front() != request) { + delete pending_requests_.front(); + pending_requests_.pop_front(); + } + + // Delete the |request| itself. + DCHECK_EQ(request, pending_requests_.front()); + delete request; + if (!pending_requests_.empty()) + pending_requests_.pop_front(); +} + void JingleSession::OnIncomingMessage(const JingleMessage& message, const ReplyCallback& reply_callback) { DCHECK(CalledOnValidThread()); @@ -451,11 +519,7 @@ void JingleSession::ProcessAuthenticationStep() { JingleMessage message(peer_jid_, JingleMessage::SESSION_INFO, session_id_); message.info = authenticator_->GetNextMessage(); DCHECK(message.info.get()); - - session_info_request_ = session_manager_->iq_sender()->SendIq( - message.ToXml(), base::Bind( - &JingleSession::OnSessionInfoResponse, - base::Unretained(this))); + SendMessage(message); } DCHECK_NE(authenticator_->state(), Authenticator::MESSAGE_READY); @@ -467,42 +531,12 @@ void JingleSession::ProcessAuthenticationStep() { } } -void JingleSession::OnSessionInfoResponse(const buzz::XmlElement* response) { - const std::string& type = response->Attr(buzz::QName("", "type")); - if (type != "result") { - LOG(ERROR) << "Received error in response to session-info message: \"" - << response->Str() - << "\". Terminating the session."; - CloseInternal(INCOMPATIBLE_PROTOCOL); - } -} - -void JingleSession::OnTransportInfoResponse(const buzz::XmlElement* response) { - const std::string& type = response->Attr(buzz::QName("", "type")); - if (type != "result") { - LOG(ERROR) << "Received error in response to session-initiate message: \"" - << response->Str() - << "\". Terminating the session."; - - if (state_ == CONNECTING) { - CloseInternal(PEER_IS_OFFLINE); - } else { - // Host has disconnected without sending session-terminate message. - CloseInternal(OK); - } - } -} - void JingleSession::SendTransportInfo() { JingleMessage message(peer_jid_, JingleMessage::TRANSPORT_INFO, session_id_); message.candidates.swap(pending_candidates_); - transport_info_request_ = session_manager_->iq_sender()->SendIq( - message.ToXml(), base::Bind( - &JingleSession::OnTransportInfoResponse, - base::Unretained(this))); + SendMessage(message); } - void JingleSession::CloseInternal(Error error) { DCHECK(CalledOnValidThread()); @@ -527,8 +561,7 @@ void JingleSession::CloseInternal(Error error) { JingleMessage message(peer_jid_, JingleMessage::SESSION_TERMINATE, session_id_); message.reason = reason; - session_manager_->iq_sender()->SendIq( - message.ToXml(), IqSender::ReplyCallback()); + SendMessage(message); } error_ = error; diff --git a/remoting/protocol/jingle_session.h b/remoting/protocol/jingle_session.h index f092402..c747250 100644 --- a/remoting/protocol/jingle_session.h +++ b/remoting/protocol/jingle_session.h @@ -13,6 +13,7 @@ #include "base/timer.h" #include "crypto/rsa_private_key.h" #include "net/base/completion_callback.h" +#include "remoting/jingle_glue/iq_sender.h" #include "remoting/protocol/authenticator.h" #include "remoting/protocol/jingle_messages.h" #include "remoting/protocol/session.h" @@ -25,9 +26,6 @@ class StreamSocket; } // namespace net namespace remoting { - -class IqRequest; - namespace protocol { class JingleSessionManager; @@ -86,8 +84,15 @@ class JingleSession : public Session, scoped_ptr<Authenticator> authenticator); void AcceptIncomingConnection(const JingleMessage& initiate_message); - // Handler for session-initiate response. - void OnSessionInitiateResponse(const buzz::XmlElement* response); + // Helper to send IqRequests to the peer. It sets up the response + // callback to OnMessageResponse() which simply terminates the + // session whenever a request fails or times out. This method should + // not be used for messages that need to be handled differently. + void SendMessage(const JingleMessage& message); + void OnMessageResponse(JingleMessage::ActionType request_type, + IqRequest* request, + const buzz::XmlElement* response); + void CleanupPendingRequests(IqRequest* request); // Called by JingleSessionManager on incoming |message|. Must call // |reply_callback| to send reply message before sending any other @@ -108,10 +113,8 @@ class JingleSession : public Session, bool InitializeConfigFromDescription(const ContentDescription* description); void ProcessAuthenticationStep(); - void OnSessionInfoResponse(const buzz::XmlElement* response); void SendTransportInfo(); - void OnTransportInfoResponse(const buzz::XmlElement* response); // Terminates the session and sends session-terminate if it is // necessary. |error| specifies the error code in case when the @@ -136,9 +139,10 @@ class JingleSession : public Session, scoped_ptr<Authenticator> authenticator_; - scoped_ptr<IqRequest> initiate_request_; - scoped_ptr<IqRequest> session_info_request_; - scoped_ptr<IqRequest> transport_info_request_; + // Container for pending Iq requests. Requests are removed in + // CleanupPendingRequests() which is called when a response is + // received or one of the requests times out. + std::list<IqRequest*> pending_requests_; ChannelsMap channels_; |