Skip to content

Commit

Permalink
Move the routing-info inside the jingle element.
Browse files Browse the repository at this point in the history 
Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.

Review-Url: https://codereview.chromium.org/2042513002
Cr-Commit-Position: refs/heads/master@{#398383}
(cherry picked from commit 046534d)

BUG=618143

Review URL: https://codereview.chromium.org/2061503002 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#326}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
[email protected] committed Jun 11, 2016
1 parent a94f6a6 commit cbc9d49
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 30 deletions.
65 changes: 49 additions & 16 deletions remoting/protocol/jingle_messages.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,27 @@ buzz::QName GetQNameByField(Field attr, bool from) {
}

SignalingAddress ParseAddress(
const buzz::XmlElement* stanza, bool from, std::string* error) {
const buzz::XmlElement* iq, bool from, std::string* error) {
SignalingAddress empty_instance;

std::string jid(stanza->Attr(GetQNameByField(Field::JID, from)));
std::string jid(iq->Attr(GetQNameByField(Field::JID, from)));

const XmlElement* jingle = iq->FirstNamed(QName(kJingleNamespace, "jingle"));

if (!jingle) {
return SignalingAddress(jid);
}

std::string type(iq->Attr(QName(std::string(), "type")));
// For error IQs, flips the |from| flag as the jingle node represents the
// original request.
if (type == "error") {
from = !from;
}

std::string endpoint_id(
stanza->Attr(GetQNameByField(Field::ENDPOINT_ID, from)));
std::string channel_str(stanza->Attr(GetQNameByField(Field::CHANNEL, from)));
jingle->Attr(GetQNameByField(Field::ENDPOINT_ID, from)));
std::string channel_str(jingle->Attr(GetQNameByField(Field::CHANNEL, from)));
SignalingAddress::Channel channel;

if (channel_str.empty()) {
Expand All @@ -198,20 +212,35 @@ SignalingAddress ParseAddress(
return SignalingAddress(jid, endpoint_id, channel);
}

void SetAddress(buzz::XmlElement* iqElement,
void SetAddress(buzz::XmlElement* iq,
buzz::XmlElement* jingle,
const SignalingAddress& address,
bool from) {
if (address.empty()) {
return;
}

iqElement->AddAttr(GetQNameByField(Field::JID, from), address.jid);
// Always set the JID.
iq->SetAttr(GetQNameByField(Field::JID, from), address.jid);

// Do not tamper the routing-info in the jingle tag for error IQ's, as
// it corresponds to the original message.
std::string type(iq->Attr(QName(std::string(), "type")));
if (type == "error") {
return;
}

// Start from a fresh slate regardless of the previous address format.
jingle->ClearAttr(GetQNameByField(Field::CHANNEL, from));
jingle->ClearAttr(GetQNameByField(Field::ENDPOINT_ID, from));

// Only set the channel and endpoint_id in the LCS channel.
if (address.channel == SignalingAddress::Channel::LCS) {
iqElement->AddAttr(GetQNameByField(Field::ENDPOINT_ID, from),
address.endpoint_id);
const char* channel_str = ValueToName(kChannelTypes, address.channel);
iqElement->AddAttr(GetQNameByField(Field::CHANNEL, from), channel_str);
jingle->AddAttr(
GetQNameByField(Field::ENDPOINT_ID, from), address.endpoint_id);
jingle->AddAttr(
GetQNameByField(Field::CHANNEL, from),
ValueToName(kChannelTypes, address.channel));
}
}

Expand Down Expand Up @@ -390,14 +419,14 @@ std::unique_ptr<buzz::XmlElement> JingleMessage::ToXml() const {
new XmlElement(QName("jabber:client", "iq"), true));

DCHECK(!to.empty());
SetAddress(root.get(), to, false);
SetAddress(root.get(), from, true);
root->SetAttr(QName(kEmptyNamespace, "type"), "set");

XmlElement* jingle_tag =
new XmlElement(QName(kJingleNamespace, "jingle"), true);
root->AddElement(jingle_tag);
jingle_tag->AddAttr(QName(kEmptyNamespace, "sid"), sid);
SetAddress(root.get(), jingle_tag, to, false);
SetAddress(root.get(), jingle_tag, from, true);

const char* action_attr = ValueToName(kActionTypes, action);
if (!action_attr)
Expand Down Expand Up @@ -471,23 +500,27 @@ std::unique_ptr<buzz::XmlElement> JingleMessageReply::ToXml(
std::unique_ptr<XmlElement> iq(
new XmlElement(QName(kJabberNamespace, "iq"), true));

iq->SetAttr(QName(kEmptyNamespace, "id"),
request_stanza->Attr(QName(kEmptyNamespace, "id")));

SignalingAddress original_from;
std::string error_message;
original_from = ParseAddress(request_stanza, true, &error_message);
DCHECK(error_message.empty());
SetAddress(iq.get(), original_from, false);

iq->SetAttr(QName(kEmptyNamespace, "id"),
request_stanza->Attr(QName(kEmptyNamespace, "id")));

if (type == REPLY_RESULT) {
iq->SetAttr(QName(kEmptyNamespace, "type"), "result");
XmlElement* jingle =
new XmlElement(QName(kJingleNamespace, "jingle"), true);
iq->AddElement(jingle);
SetAddress(iq.get(), jingle, original_from, false);
return iq;
}

DCHECK_EQ(type, REPLY_ERROR);

iq->SetAttr(QName(kEmptyNamespace, "type"), "error");
SetAddress(iq.get(), nullptr, original_from, false);

for (const buzz::XmlElement* child = request_stanza->FirstElement();
child != nullptr; child = child->NextElement()) {
Expand Down
32 changes: 18 additions & 14 deletions remoting/protocol/jingle_messages_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,15 +403,18 @@ TEST(JingleMessageTest, SessionInfo) {
buzz::QName("urn:xmpp:jingle:1", "test-info"));
}

TEST(JingleMessageTest, Address) {
TEST(JingleMessageTest, ParseAddress) {
const char* kTestSessionInfoMessage =
"<cli:iq from='[email protected]' "
"from-channel='lcs' "
"from-endpoint-id='[email protected]/xBrnereror=' "
"to='[email protected]/chromiumsy5C6A652D' type='set' "
"xmlns:cli='jabber:client'><jingle action='session-info' "
"sid='2227053353' xmlns='urn:xmpp:jingle:1'><test-info>TestMessage"
"</test-info></jingle></cli:iq>";
"to='[email protected]/chromiumsy5C6A652D' type='set' "
"xmlns:cli='jabber:client'>"
"<jingle action='session-info' "
"sid='2227053353' xmlns='urn:xmpp:jingle:1' "
"from-channel='lcs' "
"from-endpoint-id='[email protected]/xBrnereror='>"
"<test-info>TestMessage</test-info>"
"</jingle>"
"</cli:iq>";

JingleMessage message;
ParseFormatAndCompare(kTestSessionInfoMessage, &message);
Expand All @@ -437,9 +440,9 @@ TEST(JingleMessageReplyTest, ToXml) {
"</reason></jingle></cli:iq>";
const char* kTestIncomingMessage2 =
"<cli:iq from='[email protected]' id='4' "
"from-channel='lcs' from-endpoint-id='[email protected]/AbCdEf1234=' "
"to='[email protected]/chromiumsy5C6A652D' type='set' "
"xmlns:cli='jabber:client'><jingle action='session-terminate' "
"from-channel='lcs' from-endpoint-id='[email protected]/AbCdEf1234=' "
"sid='2227053353' xmlns='urn:xmpp:jingle:1'><reason><success/>"
"</reason></jingle></cli:iq>";

Expand Down Expand Up @@ -493,17 +496,17 @@ TEST(JingleMessageReplyTest, ToXml) {
kTestIncomingMessage1},
{JingleMessageReply::INVALID_SID, "ErrorText",
"<iq xmlns='jabber:client' "
"to='[email protected]' to-channel='lcs' "
"to-endpoint-id='[email protected]/AbCdEf1234=' id='4' "
"to='[email protected]' id='4' "
"type='error'><jingle "
"action='session-terminate' sid='2227053353' xmlns='urn:xmpp:jingle:1'>"
"action='session-terminate' sid='2227053353' xmlns='urn:xmpp:jingle:1' "
"from-channel='lcs' from-endpoint-id='[email protected]/AbCdEf1234='>"
"<reason><success/></reason></jingle><error type='modify'>"
"<item-not-found/><text xml:lang='en'>ErrorText</text></error></iq>",
kTestIncomingMessage2},
{JingleMessageReply::NONE, "",
"<iq xmlns='jabber:client' "
"to='[email protected]' to-channel='lcs' "
"to-endpoint-id='[email protected]/AbCdEf1234=' id='4' type='result'></iq>",
"<iq xmlns='jabber:client' to='[email protected]' id='4' "
"type='result'><jingle xmlns='urn:xmpp:jingle:1' to-channel='lcs' "
"to-endpoint-id='[email protected]/AbCdEf1234='/></iq>",
kTestIncomingMessage2},
};

Expand All @@ -512,6 +515,7 @@ TEST(JingleMessageReplyTest, ToXml) {
XmlElement::ForStr(tests[i].incoming_message));
ASSERT_TRUE(incoming_message.get());

SCOPED_TRACE(testing::Message() << "Running test case: " << i);
JingleMessageReply reply_msg;
if (tests[i].error_text.empty()) {
reply_msg = JingleMessageReply(tests[i].error);
Expand Down

0 comments on commit cbc9d49

Please sign in to comment.