Commit e96e77dd authored by Guus der Kinderen's avatar Guus der Kinderen Committed by akrherz

OF-1471: Terminate upon invalid Stream Management data.

When a client tells us that it received an amount of stanzas that's greater than the number of stanzas
that we've sent it, the stream should be terminated. Similarly, we should not allow a client to resume
a stream when it tells us it received more data on the old stream than what we've sent it.
parent c3727084
...@@ -253,11 +253,18 @@ public class StreamManager { ...@@ -253,11 +253,18 @@ public class StreamManager {
} }
Log.debug("Found existing session, checking status"); Log.debug("Found existing session, checking status");
// Previd identifies proper session. Now check SM status // Previd identifies proper session. Now check SM status
if (!otherSession.getStreamManager().resume) {
Log.debug("Not allowing a client to resume a session, the session to be resumed does not have the stream management resumption feature enabled." );
sendError(new PacketError(PacketError.Condition.unexpected_request));
return;
}
if (!otherSession.getStreamManager().namespace.equals(namespace)) { if (!otherSession.getStreamManager().namespace.equals(namespace)) {
Log.debug("Not allowing a client to resume a session, the session to be resumed used a different version ({}) of the session management resumption feature as compared to the version that's requested now: {}.", otherSession.getStreamManager().namespace, namespace);
sendError(new PacketError(PacketError.Condition.unexpected_request)); sendError(new PacketError(PacketError.Condition.unexpected_request));
return; return;
} }
if (!otherSession.getStreamManager().resume) { if (!validateClientAcknowledgement(h)) {
Log.debug("Not allowing a client to resume a session, as it reports it received more stanzas from us than that we've send it." );
sendError(new PacketError(PacketError.Condition.unexpected_request)); sendError(new PacketError(PacketError.Condition.unexpected_request));
return; return;
} }
...@@ -335,6 +342,17 @@ public class StreamManager { ...@@ -335,6 +342,17 @@ public class StreamManager {
this.namespace = null; // isEnabled() is testing this. this.namespace = null; // isEnabled() is testing this.
} }
/**
* Checks if the amount of stanzas that the client acknowledges is equal to or less than the amount of stanzas that
* we've sent to the client.
*
* @param h Then number of stanzas that the client acknowledges it has received from us.
* @return false if we sent less stanzas to the client than the number it is acknowledging.
*/
private synchronized boolean validateClientAcknowledgement(long h) {
return h <= ( unacknowledgedServerStanzas.isEmpty() ? clientProcessedStanzas : unacknowledgedServerStanzas.getLast().x );
}
/** /**
* Process client acknowledgements for a given value of h. * Process client acknowledgements for a given value of h.
* *
...@@ -343,8 +361,9 @@ public class StreamManager { ...@@ -343,8 +361,9 @@ public class StreamManager {
private void processClientAcknowledgement(long h) { private void processClientAcknowledgement(long h) {
synchronized (this) { synchronized (this) {
if ( !unacknowledgedServerStanzas.isEmpty() && h > unacknowledgedServerStanzas.getLast().x ) { if ( !validateClientAcknowledgement(h) ) {
Log.warn( "Client acknowledges stanzas that we didn't send! Client Ack h: {}, our last stanza: {}", h, unacknowledgedServerStanzas.getLast().x ); // All paths leading up to here should have checked for this. Race condition?
throw new IllegalStateException( "Client acknowledges stanzas that we didn't send! Client Ack h: "+h+", our last stanza: " + unacknowledgedServerStanzas.getLast().x );
} }
clientProcessedStanzas = h; clientProcessedStanzas = h;
...@@ -384,6 +403,15 @@ public class StreamManager { ...@@ -384,6 +403,15 @@ public class StreamManager {
final long h = Long.valueOf(ack.attributeValue("h")); final long h = Long.valueOf(ack.attributeValue("h"));
Log.debug( "Received acknowledgement from client: h={}", h ); Log.debug( "Received acknowledgement from client: h={}", h );
if (!validateClientAcknowledgement(h)) {
Log.warn( "Closing client session. Client acknowledges stanzas that we didn't send! Client Ack h: {}, our last stanza: {}", h, unacknowledgedServerStanzas.getLast().x );
final StreamError error = new StreamError( StreamError.Condition.undefined_condition, "You acknowledged stanzas that we didn't send. Your Ack h: " + h + ", our last stanza: " + unacknowledgedServerStanzas.getLast().x );
session.deliverRawText( error.toXML() );
session.close();
return;
}
processClientAcknowledgement(h); processClientAcknowledgement(h);
} }
} }
......
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