From c592b4f0464694332acf236d7bbfa1169253e04c Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 17 Mar 2014 21:06:45 +0100 Subject: [PATCH] Improve Exceptions (SMACK-426) --- .../smack/SASLAuthentication.java | 17 +++--- .../jivesoftware/smack/SmackException.java | 41 +++++++++++++- .../jivesoftware/smack/sasl/SASLError.java | 14 +++-- .../smack/sasl/SASLErrorException.java | 2 + .../address/MultipleRecipientManager.java | 2 +- .../ibb/InBandBytestreamSession.java | 16 +++--- .../socks5/Socks5BytestreamManager.java | 3 +- .../bytestreams/socks5/Socks5Client.java | 53 +++++++++---------- .../socks5/Socks5ByteStreamManagerTest.java | 12 ++--- 9 files changed, 103 insertions(+), 57 deletions(-) diff --git a/core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java b/core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java index 7c7e0e9a3..e7107e84f 100644 --- a/core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java +++ b/core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java @@ -18,6 +18,7 @@ package org.jivesoftware.smack; import org.jivesoftware.smack.SmackException.NoResponseException; +import org.jivesoftware.smack.SmackException.ResourceBindingNotOfferedException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.Bind; import org.jivesoftware.smack.packet.Packet; @@ -210,9 +211,10 @@ public class SASLAuthentication { * @throws XMPPErrorException * @throws NoResponseException * @throws SASLErrorException + * @throws ResourceBindingNotOfferedException */ public String authenticate(String resource, CallbackHandler cbh) throws IOException, - NoResponseException, XMPPErrorException, SASLErrorException { + NoResponseException, XMPPErrorException, SASLErrorException, ResourceBindingNotOfferedException { // Locate the SASLMechanism to use String selectedMechanism = null; for (String mechanism : mechanismsPreferences) { @@ -265,8 +267,7 @@ public class SASLAuthentication { return bindResourceAndEstablishSession(resource); } else { - // SASL authentication failed - throw new SaslException(); + throw new NoResponseException(); } } @@ -352,8 +353,7 @@ public class SASLAuthentication { return bindResourceAndEstablishSession(resource); } else { - // SASL authentication failed - throw new SaslException(); + throw new NoResponseException(); } } else { @@ -404,11 +404,12 @@ public class SASLAuthentication { return bindResourceAndEstablishSession(null); } else { - throw new SaslException(); + throw new NoResponseException(); } } - private String bindResourceAndEstablishSession(String resource) throws NoResponseException, XMPPErrorException { + private String bindResourceAndEstablishSession(String resource) throws XMPPErrorException, + ResourceBindingNotOfferedException, NoResponseException { // Wait until server sends response containing the element synchronized (this) { if (!resourceBinded) { @@ -424,7 +425,7 @@ public class SASLAuthentication { if (!resourceBinded) { // Server never offered resource binding, which is REQURIED in XMPP client and server // implementations as per RFC6120 7.2 - throw new IllegalStateException("Resource binding not offered by server"); + throw new ResourceBindingNotOfferedException(); } Bind bindResource = new Bind(); diff --git a/core/src/main/java/org/jivesoftware/smack/SmackException.java b/core/src/main/java/org/jivesoftware/smack/SmackException.java index 7e8131be0..33f89e927 100644 --- a/core/src/main/java/org/jivesoftware/smack/SmackException.java +++ b/core/src/main/java/org/jivesoftware/smack/SmackException.java @@ -152,8 +152,45 @@ public class SmackException extends Exception { */ private static final long serialVersionUID = 4713404802621452016L; - public FeatureNotSupportedException(String message) { - super(message); + private final String feature; + private final String jid; + + public FeatureNotSupportedException(String feature) { + this(feature, null); + } + + public FeatureNotSupportedException(String feature, String jid) { + super(feature + " not supported" + (jid == null ? "" : " by '" + jid + "'")); + this.jid = jid; + this.feature = feature; + } + + /** + * Get the feature which is not supported. + * + * @return the feature which is not supported + */ + public String getFeature() { + return feature; + } + + /** + * Get JID which does not support the feature. The JID can be null in cases when there are + * multiple JIDs queried for this feature. + * + * @return the JID which does not support the feature, or null + */ + public String getJid() { + return jid; } } + + public static class ResourceBindingNotOfferedException extends SmackException { + + /** + * + */ + private static final long serialVersionUID = 2346934138253437571L; + + } } diff --git a/core/src/main/java/org/jivesoftware/smack/sasl/SASLError.java b/core/src/main/java/org/jivesoftware/smack/sasl/SASLError.java index 94cfa7f4e..b066ec541 100644 --- a/core/src/main/java/org/jivesoftware/smack/sasl/SASLError.java +++ b/core/src/main/java/org/jivesoftware/smack/sasl/SASLError.java @@ -16,6 +16,9 @@ */ package org.jivesoftware.smack.sasl; +import java.util.logging.Level; +import java.util.logging.Logger; + public enum SASLError { aborted, @@ -29,6 +32,7 @@ public enum SASLError { not_authorized, temporary_auth_failure; + private static final Logger LOGGER = Logger.getLogger(SASLError.class.getName()); @Override public String toString() { return this.name().replace('_', '-'); @@ -36,10 +40,12 @@ public enum SASLError { public static SASLError fromString(String string) { string = string.replace('-', '_'); - for (SASLError error : SASLError.values()) { - if (error.name().equals(string)) - return error; + SASLError saslError = null; + try { + saslError = SASLError.valueOf(string); + } catch (Exception e) { + LOGGER.log(Level.WARNING, "Could not transform string '" + string + "' to SASLError", e); } - return null; + return saslError; } } diff --git a/core/src/main/java/org/jivesoftware/smack/sasl/SASLErrorException.java b/core/src/main/java/org/jivesoftware/smack/sasl/SASLErrorException.java index 87aa3dd77..ebae180b5 100644 --- a/core/src/main/java/org/jivesoftware/smack/sasl/SASLErrorException.java +++ b/core/src/main/java/org/jivesoftware/smack/sasl/SASLErrorException.java @@ -34,12 +34,14 @@ public class SASLErrorException extends XMPPException { private final Map texts; public SASLErrorException(String mechanism, SASLFailure saslFailure) { + super("SASLError using " + mechanism + ": " + saslFailure); this.mechanism = mechanism; this.saslFailure = saslFailure; this.texts = new HashMap(); } public SASLErrorException(String mechanism, SASLFailure saslFailure, Map texts) { + super("SASLError using " + mechanism + ": " + saslFailure); this.mechanism = mechanism; this.saslFailure = saslFailure; this.texts = texts; diff --git a/extensions/src/main/java/org/jivesoftware/smackx/address/MultipleRecipientManager.java b/extensions/src/main/java/org/jivesoftware/smackx/address/MultipleRecipientManager.java index 06dc2d9b0..21a213146 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/address/MultipleRecipientManager.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/address/MultipleRecipientManager.java @@ -113,7 +113,7 @@ public class MultipleRecipientManager { (replyRoom != null && replyRoom.trim().length() > 0)) { // Some specified JEP-33 features were requested so throw an exception alerting // the user that this features are not available - throw new FeatureNotSupportedException("Extended Stanza Addressing not supported by server"); + throw new FeatureNotSupportedException("Extended Stanza Addressing"); } // Send the packet to each individual recipient sendToIndividualRecipients(connection, packet, to, cc, bcc); diff --git a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamSession.java b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamSession.java index 4c033d7b7..ff992183c 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamSession.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamSession.java @@ -211,9 +211,11 @@ public class InBandBytestreamSession implements BytestreamSession { connection.createPacketCollectorAndSend(close).nextResultOrThrow(); } catch (Exception e) { - // Sadly we are unable to wrap the exception within the IOException, because the - // IOException(String,Throwable) constructor is only support from Android API 9 on. - throw new IOException(); + // Sadly we are unable to use the IOException(Throwable) constructor because this + // constructor is only supported from Android API 9 on. + IOException ioException = new IOException(); + ioException.initCause(e); + throw ioException; } this.inputStream.cleanup(); @@ -769,9 +771,11 @@ public class InBandBytestreamSession implements BytestreamSession { // close session unless it is already closed if (!this.isClosed) { InBandBytestreamSession.this.close(); - // Sadly we are unable to wrap the exception within the IOException, because the - // IOException(String,Throwable) constructor is only support from Android API 9 on. - throw new IOException(); + // Sadly we are unable to use the IOException(Throwable) constructor because this + // constructor is only supported from Android API 9 on. + IOException ioException = new IOException(); + ioException.initCause(e); + throw ioException; } } diff --git a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java index 85b04c1b1..b37f24bab 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java @@ -32,6 +32,7 @@ import java.util.concurrent.TimeoutException; import org.jivesoftware.smack.AbstractConnectionListener; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.SmackException.NoResponseException; +import org.jivesoftware.smack.SmackException.FeatureNotSupportedException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.ConnectionCreationListener; import org.jivesoftware.smack.XMPPException; @@ -436,7 +437,7 @@ public final class Socks5BytestreamManager implements BytestreamManager { XMPPErrorException discoveryException = null; // check if target supports SOCKS5 Bytestream if (!supportsSocks5(targetJID)) { - throw new SmackException(targetJID + " doesn't support SOCKS5 Bytestream"); + throw new FeatureNotSupportedException("SOCKS5 Bytestream", targetJID); } List proxies = new ArrayList(); diff --git a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Client.java b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Client.java index 13e74c4f2..cb181349b 100644 --- a/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Client.java +++ b/extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Client.java @@ -144,8 +144,9 @@ class Socks5Client { * @return true if if a stream could be established, otherwise false. * If false is returned the given Socket should be closed. * @throws SmackException + * @throws IOException */ - protected boolean establish(Socket socket) throws SmackException { + protected boolean establish(Socket socket) throws SmackException, IOException { byte[] connectionRequest; byte[] connectionResponse; @@ -153,39 +154,35 @@ class Socks5Client { * use DataInputStream/DataOutpuStream to assure read and write is completed in a single * statement */ - try { - DataInputStream in = new DataInputStream(socket.getInputStream()); - DataOutputStream out = new DataOutputStream(socket.getOutputStream()); + DataInputStream in = new DataInputStream(socket.getInputStream()); + DataOutputStream out = new DataOutputStream(socket.getOutputStream()); - // authentication negotiation - byte[] cmd = new byte[3]; + // authentication negotiation + byte[] cmd = new byte[3]; - cmd[0] = (byte) 0x05; // protocol version 5 - cmd[1] = (byte) 0x01; // number of authentication methods supported - cmd[2] = (byte) 0x00; // authentication method: no-authentication required + cmd[0] = (byte) 0x05; // protocol version 5 + cmd[1] = (byte) 0x01; // number of authentication methods supported + cmd[2] = (byte) 0x00; // authentication method: no-authentication required - out.write(cmd); - out.flush(); + out.write(cmd); + out.flush(); - byte[] response = new byte[2]; - in.readFully(response); + byte[] response = new byte[2]; + in.readFully(response); - // check if server responded with correct version and no-authentication method - if (response[0] != (byte) 0x05 || response[1] != (byte) 0x00) { - return false; - } - - // request SOCKS5 connection with given address/digest - connectionRequest = createSocks5ConnectRequest(); - out.write(connectionRequest); - out.flush(); - - // receive response - connectionResponse = Socks5Utils.receiveSocks5Message(in); - } - catch (IOException e) { - throw new SmackException(e); + // check if server responded with correct version and no-authentication method + if (response[0] != (byte) 0x05 || response[1] != (byte) 0x00) { + return false; } + + // request SOCKS5 connection with given address/digest + connectionRequest = createSocks5ConnectRequest(); + out.write(connectionRequest); + out.flush(); + + // receive response + connectionResponse = Socks5Utils.receiveSocks5Message(in); + // verify response connectionRequest[1] = (byte) 0x00; // set expected return status to 0 return Arrays.equals(connectionRequest, connectionResponse); diff --git a/extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java b/extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java index 35fe39901..d6923bb09 100644 --- a/extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java +++ b/extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java @@ -25,6 +25,7 @@ import java.io.OutputStream; import java.net.ConnectException; import org.jivesoftware.smack.SmackException; +import org.jivesoftware.smack.SmackException.FeatureNotSupportedException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; @@ -152,13 +153,10 @@ public class Socks5ByteStreamManagerTest { fail("exception should be thrown"); } - catch (SmackException e) { - assertTrue(e.getMessage().contains("doesn't support SOCKS5 Bytestream")); - } - catch (IOException e) { - fail(e.getMessage()); - } - catch (InterruptedException e) { + catch (FeatureNotSupportedException e) { + assertTrue(e.getFeature().equals("SOCKS5 Bytestream")); + assertTrue(e.getJid().equals(targetJID)); + } catch(Exception e) { fail(e.getMessage()); }