Commit 5115f9b8 authored by Dave Cridland's avatar Dave Cridland

Change OF-888 fix to be based on error

OF-888 is presumed to be a recursion due to an attempt to bounce a bounce, in
other words it's failing to handle a double-bounce.

It appears to be caused when routing a stanza from a MUC fails, and for some
reason I don't yet understand, the routing of the bounce to the originating
MUC source also fails.

The test for the double bounce is only present for the IQ case, but this fails
in any case because it tests only for the IQ case, and uses a test which checks
the IQ's symbolic type; this is not actually set by Packet.setError(), so does
not trigger.

Tom's fix inserted a sentinel into the original failing stanza, however a new
stanza is created for the bounce, which will not contain the sentinel; therefore
the fix will not protect from a recursion.

Therefore this patch:

1) Removes Tom's fix for OF-888.

2) Tests for the message and presence error types.

3) Tests for the presence of a stanza error (though this is a warning).

4) Explicitly sets the stanza error type.
parent bcd1cc82
...@@ -58,6 +58,7 @@ import org.slf4j.LoggerFactory; ...@@ -58,6 +58,7 @@ import org.slf4j.LoggerFactory;
import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlPullParserException;
import org.xmpp.packet.IQ; import org.xmpp.packet.IQ;
import org.xmpp.packet.IQ.Type;
import org.xmpp.packet.JID; import org.xmpp.packet.JID;
import org.xmpp.packet.Message; import org.xmpp.packet.Message;
import org.xmpp.packet.Packet; import org.xmpp.packet.Packet;
...@@ -637,15 +638,10 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou ...@@ -637,15 +638,10 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou
private void returnErrorToSender(Packet packet) { private void returnErrorToSender(Packet packet) {
RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable(); RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable();
if (packet.getError() != null) {
Log.debug("Possible double bounce: " + packet.toXML());
}
try { try {
// OF-888 (outgoing S2S failure) Prevent recursion for failed delivery of error response packet
String el = "ext";
String ns = getClass().getName();
if (packet.deleteExtension(el, ns)) {
Log.warn("Failed to route error to sender; remote server not found. Original packet: " + packet);
return;
}
packet.addExtension(new PacketExtension(el, ns));
if (packet instanceof IQ) { if (packet instanceof IQ) {
if (((IQ) packet).isResponse()) { if (((IQ) packet).isResponse()) {
Log.debug("XMPP specs forbid us to respond with an IQ error to: " + packet.toXML()); Log.debug("XMPP specs forbid us to respond with an IQ error to: " + packet.toXML());
...@@ -656,23 +652,33 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou ...@@ -656,23 +652,33 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou
reply.setTo(packet.getFrom()); reply.setTo(packet.getFrom());
reply.setFrom(packet.getTo()); reply.setFrom(packet.getTo());
reply.setChildElement(((IQ) packet).getChildElement().createCopy()); reply.setChildElement(((IQ) packet).getChildElement().createCopy());
reply.setType(IQ.Type.error);
reply.setError(PacketError.Condition.remote_server_not_found); reply.setError(PacketError.Condition.remote_server_not_found);
routingTable.routePacket(reply.getTo(), reply, true); routingTable.routePacket(reply.getTo(), reply, true);
} }
else if (packet instanceof Presence) { else if (packet instanceof Presence) {
if (((Presence)packet).getType() == Presence.Type.error) {
Log.debug("Double-bounce of presence: " + packet.toXML());
return;
}
Presence reply = new Presence(); Presence reply = new Presence();
reply.setID(packet.getID()); reply.setID(packet.getID());
reply.setTo(packet.getFrom()); reply.setTo(packet.getFrom());
reply.setFrom(packet.getTo()); reply.setFrom(packet.getTo());
reply.setType(Presence.Type.error);
reply.setError(PacketError.Condition.remote_server_not_found); reply.setError(PacketError.Condition.remote_server_not_found);
routingTable.routePacket(reply.getTo(), reply, true); routingTable.routePacket(reply.getTo(), reply, true);
} }
else if (packet instanceof Message) { else if (packet instanceof Message) {
if (((Message)packet).getType() == Message.Type.error){
Log.debug("Double-bounce of message: " + packet.toXML());
return;
}
Message reply = new Message(); Message reply = new Message();
reply.setID(packet.getID()); reply.setID(packet.getID());
reply.setTo(packet.getFrom()); reply.setTo(packet.getFrom());
reply.setFrom(packet.getTo()); reply.setFrom(packet.getTo());
reply.setType(((Message)packet).getType()); reply.setType(Message.Type.error);
reply.setThread(((Message)packet).getThread()); reply.setThread(((Message)packet).getThread());
reply.setError(PacketError.Condition.remote_server_not_found); reply.setError(PacketError.Condition.remote_server_not_found);
routingTable.routePacket(reply.getTo(), reply, true); routingTable.routePacket(reply.getTo(), reply, true);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment