[filetransfer] Remove FaultTolerantNegotiator

The FaultTolerantNegotiator is the reason why Smack replies in a
non-standard way to file transfer requests: Smack puts two values in
the stream-method field, while the field is a list-single field,
i.e. a field which only allows one value.

Even if what Smack does is probably better, as it allows for a
fallback in case the bytestream transport fails, it is not standard
compliant. And, Jingle provide a proper fallback specification for
file transfers.

Fixes SMACK-561.
This commit is contained in:
Florian Schmaus 2020-05-11 22:12:22 +02:00
parent 2c39dff653
commit 3270c113c5
5 changed files with 11 additions and 133 deletions

View File

@ -1,111 +0,0 @@
/**
*
* Copyright 2003-2006 Jive Software.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.filetransfer;
import java.io.InputStream;
import java.io.OutputStream;
import org.jivesoftware.smack.SmackException;
import org.jivesoftware.smack.XMPPConnection;
import org.jivesoftware.smack.XMPPException;
import org.jivesoftware.smack.XMPPException.XMPPErrorException;
import org.jivesoftware.smack.packet.IQ;
import org.jivesoftware.smack.packet.Stanza;
import org.jivesoftware.smackx.bytestreams.ibb.packet.Open;
import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream;
import org.jivesoftware.smackx.si.packet.StreamInitiation;
import org.jxmpp.jid.Jid;
/**
* The fault tolerant negotiator takes two stream negotiators, the primary and the secondary
* negotiator. If the primary negotiator fails during the stream negotiation process, the second
* negotiator is used.
*/
public class FaultTolerantNegotiator extends StreamNegotiator {
private final StreamNegotiator primaryNegotiator;
private final StreamNegotiator secondaryNegotiator;
public FaultTolerantNegotiator(XMPPConnection connection, StreamNegotiator primary,
StreamNegotiator secondary) {
super(connection);
this.primaryNegotiator = primary;
this.secondaryNegotiator = secondary;
}
@Override
public void newStreamInitiation(Jid from, String streamID) {
primaryNegotiator.newStreamInitiation(from, streamID);
secondaryNegotiator.newStreamInitiation(from, streamID);
}
@Override
InputStream negotiateIncomingStream(Stanza streamInitiation) {
throw new UnsupportedOperationException("Negotiation only handled by create incoming " +
"stream method.");
}
@Override
public InputStream createIncomingStream(final StreamInitiation initiation) throws SmackException, XMPPErrorException, InterruptedException {
// This could be either an xep47 ibb 'open' iq or an xep65 streamhost iq
IQ initiationSet = initiateIncomingStream(connection(), initiation);
StreamNegotiator streamNegotiator = determineNegotiator(initiationSet);
return streamNegotiator.negotiateIncomingStream(initiationSet);
}
private StreamNegotiator determineNegotiator(Stanza streamInitiation) {
if (streamInitiation instanceof Bytestream) {
return primaryNegotiator;
} else if (streamInitiation instanceof Open) {
return secondaryNegotiator;
} else {
throw new IllegalStateException("Unknown stream initiation type");
}
}
@Override
public OutputStream createOutgoingStream(String streamID, Jid initiator, Jid target)
throws SmackException, XMPPException, InterruptedException {
OutputStream stream;
try {
stream = primaryNegotiator.createOutgoingStream(streamID, initiator, target);
}
catch (Exception ex) {
stream = secondaryNegotiator.createOutgoingStream(streamID, initiator, target);
}
return stream;
}
@Override
public String[] getNamespaces() {
String[] primary = primaryNegotiator.getNamespaces();
String[] secondary = secondaryNegotiator.getNamespaces();
String[] namespaces = new String[primary.length + secondary.length];
System.arraycopy(primary, 0, namespaces, 0, primary.length);
System.arraycopy(secondary, 0, namespaces, primary.length, secondary.length);
return namespaces;
}
}

View File

@ -239,12 +239,7 @@ public final class FileTransferNegotiator extends Manager {
throw new FileTransferException.NoAcceptableTransferMechanisms();
}
if (isByteStream && isIBB) {
return new FaultTolerantNegotiator(connection(),
byteStreamTransferManager,
inbandTransferManager);
}
else if (isByteStream) {
if (isByteStream) {
return byteStreamTransferManager;
}
else {
@ -355,11 +350,7 @@ public final class FileTransferNegotiator extends Manager {
throw new FileTransferException.NoAcceptableTransferMechanisms();
}
if (isByteStream && isIBB) {
return new FaultTolerantNegotiator(connection(),
byteStreamTransferManager, inbandTransferManager);
}
else if (isByteStream) {
if (isByteStream) {
return byteStreamTransferManager;
}
else {

View File

@ -90,8 +90,8 @@ public class IBBTransferNegotiator extends StreamNegotiator {
}
@Override
public String[] getNamespaces() {
return new String[] { DataPacketExtension.NAMESPACE };
public String getNamespace() {
return DataPacketExtension.NAMESPACE;
}
@Override

View File

@ -88,8 +88,8 @@ public class Socks5TransferNegotiator extends StreamNegotiator {
}
@Override
public String[] getNamespaces() {
return new String[] { Bytestream.NAMESPACE };
public String getNamespace() {
return Bytestream.NAMESPACE;
}
@Override

View File

@ -69,11 +69,11 @@ public abstract class StreamNegotiator extends Manager {
* initiator.
*
* @param streamInitiationOffer The offer from the stream initiator to connect for a stream.
* @param namespaces The namespace that relates to the accepted means of transfer.
* @param namespace The namespace that relates to the accepted means of transfer.
* @return The response to be forwarded to the initiator.
*/
protected static StreamInitiation createInitiationAccept(
StreamInitiation streamInitiationOffer, String[] namespaces) {
StreamInitiation streamInitiationOffer, String namespace) {
StreamInitiation response = new StreamInitiation();
response.setTo(streamInitiationOffer.getFrom());
response.setFrom(streamInitiationOffer.getTo());
@ -83,9 +83,7 @@ public abstract class StreamNegotiator extends Manager {
DataForm form = new DataForm(DataForm.Type.submit);
FormField.Builder field = FormField.builder(
FileTransferNegotiator.STREAM_DATA_FIELD_NAME);
for (String namespace : namespaces) {
field.addValue(namespace);
}
field.addValue(namespace);
form.addField(field.build());
response.setFeatureNegotiationForm(form);
@ -95,7 +93,7 @@ public abstract class StreamNegotiator extends Manager {
protected final IQ initiateIncomingStream(final XMPPConnection connection, StreamInitiation initiation)
throws NoResponseException, XMPPErrorException, NotConnectedException {
final StreamInitiation response = createInitiationAccept(initiation,
getNamespaces());
getNamespace());
newStreamInitiation(initiation.getFrom(), initiation.getSessionID());
@ -182,7 +180,7 @@ public abstract class StreamNegotiator extends Manager {
* @return Returns the XMPP namespace reserved for this particular type of
* file transfer.
*/
public abstract String[] getNamespaces();
public abstract String getNamespace();
public static void signal(String eventKey, IQ eventValue) {
initationSetEvents.signalEvent(eventKey, eventValue);