Move the routing-info inside the jingle element.

remoting/protocol/jingle_messages.cc

 // 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.
 
 #include "remoting/protocol/jingle_messages.h"
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "remoting/base/constants.h"
 #include "remoting/protocol/content_description.h"
@@ skipping 131 matching lines @@
   result->SetAttr(QName(kEmptyNamespace, "type"), candidate.candidate.type());
   result->SetAttr(QName(kEmptyNamespace, "protocol"),
                   candidate.candidate.protocol());
   result->SetAttr(QName(kEmptyNamespace, "priority"),
                   base::UintToString(candidate.candidate.priority()));
   result->SetAttr(QName(kEmptyNamespace, "generation"),
                   base::UintToString(candidate.candidate.generation()));
   return result;
 }
 
+XmlElement* EnsureJingleTag(XmlElement* iq) {
+  XmlElement* jingle = iq->FirstNamed(QName(kJingleNamespace, "jingle"));
+  if (!jingle) {
+    jingle = new XmlElement(QName(kJingleNamespace, "jingle"), true);
+    iq->AddElement(jingle);
+  }
+  return jingle;
+}
+
 // Represents the XML attrbute names for the various address fields in the
 // iq stanza.
 enum class Field { JID, CHANNEL, ENDPOINT_ID };
 
 buzz::QName GetQNameByField(Field attr, bool from) {
   std::string attribute_name;
   switch (attr) {
     case Field::JID:
       attribute_name = (from) ? "from" : "to";
       break;
     case Field::ENDPOINT_ID:
       attribute_name = (from) ? "from-endpoint-id" : "to-endpoint-id";
       break;
     case Field::CHANNEL:
       attribute_name = (from) ? "from-channel" : "to-channel";
       break;
     default:
       NOTREACHED();
   }
   return QName(kEmptyNamespace, attribute_name);
 }
 
 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()) {
     channel = SignalingAddress::Channel::XMPP;
   } else if (!NameToValue(kChannelTypes, channel_str, &channel)) {
     *error = "Unknown channel: " + channel_str;
     return empty_instance;
   }
 
   bool isLcs = (channel == SignalingAddress::Channel::LCS);
 
   if (isLcs == endpoint_id.empty()) {
     *error = (isLcs ? "Missing |endpoint-id| for LCS channel"
                     : "|endpoint_id| should be empty for XMPP channel");
     return empty_instance;
   }
 
   return SignalingAddress(jid, endpoint_id, channel);
 }
 
-void SetAddress(buzz::XmlElement* iqElement,
+void SetAddress(buzz::XmlElement* iq,
                 const SignalingAddress& address,
                 bool from) {
-  // Start from a fresh slate regardless of the previous address format.
-  iqElement->ClearAttr(GetQNameByField(Field::JID, from));
-  iqElement->ClearAttr(GetQNameByField(Field::CHANNEL, from));
-  iqElement->ClearAttr(GetQNameByField(Field::ENDPOINT_ID, from));
-
   if (address.empty()) {
     return;
   }
 
   // Always set the JID.
-  iqElement->AddAttr(GetQNameByField(Field::JID, from), address.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.
+  XmlElement* jingle = EnsureJingleTag(iq);

[Sergey Ulanov, 2016/06/04 17:10:31]

I suggest passing a pointer to <jingle> explicitly as an argument for
SetAddress(). And then change JingleMessageReply to create <jingle> explicitly.

[kelvinp, 2016/06/06 20:43:41]

Done.

+  jingle->ClearAttr(GetQNameByField(Field::CHANNEL, from));

[Sergey Ulanov, 2016/06/04 17:10:31]

I this necessary? SetAddress() is always called with a newly created message, so
these attributes are never set as far as I can tell

[kelvinp, 2016/06/06 20:43:41]

This is true.  For the sake of future maintenance, I think SetAddress should
always clear previously set address and return a message with the correct
address representation regardless of what happens a priori.

+  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));
   }
 }
 
 }  // namespace
 
 IceTransportInfo::NamedCandidate::NamedCandidate(
     const std::string& name,
     const cricket::Candidate& candidate)
     : name(name),
       candidate(candidate) {
@@ skipping 162 matching lines @@
 
 std::unique_ptr<buzz::XmlElement> JingleMessage::ToXml() const {
   std::unique_ptr<XmlElement> root(
       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);
+  XmlElement* jingle_tag = EnsureJingleTag(root.get());
   jingle_tag->AddAttr(QName(kEmptyNamespace, "sid"), sid);
 
   const char* action_attr = ValueToName(kActionTypes, action);
   if (!action_attr)
     LOG(FATAL) << "Invalid action value " << action;
   jingle_tag->AddAttr(QName(kEmptyNamespace, "action"), action_attr);
 
   if (action == SESSION_INFO) {
     if (info.get())
       jingle_tag->AddElement(new XmlElement(*info.get()));
@@ skipping 54 matching lines @@
       text(text_value) {
 }
 
 JingleMessageReply::~JingleMessageReply() { }
 
 std::unique_ptr<buzz::XmlElement> JingleMessageReply::ToXml(
     const buzz::XmlElement* request_stanza) const {
   std::unique_ptr<XmlElement> iq(
       new XmlElement(QName(kJabberNamespace, "iq"), true));
 
+  iq->SetAttr(QName(kEmptyNamespace, "id"),
+              request_stanza->Attr(QName(kEmptyNamespace, "id")));
+
+  std::string type_string(type == REPLY_RESULT ? "result" : "error");
+  iq->SetAttr(QName(kEmptyNamespace, "type"), type_string);
+
   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");
     return iq;
   }
 
   DCHECK_EQ(type, REPLY_ERROR);
 
   iq->SetAttr(QName(kEmptyNamespace, "type"), "error");
 
   for (const buzz::XmlElement* child = request_stanza->FirstElement();
        child != nullptr; child = child->NextElement()) {
     iq->AddElement(new buzz::XmlElement(*child));
@@ skipping 98 matching lines @@
     result->AddElement(FormatIceCredentials(credentials));
   }
   for (const NamedCandidate& candidate : candidates) {
     result->AddElement(FormatIceCandidate(candidate));
   }
   return result;
 }
 
 }  // namespace protocol
 }  // namespace remoting