Issue #545 - Validating the Topic provided when a response topic is set.

Signed-off-by: James Sutton <james.sutton@uk.ibm.com>

Author: jpwsutton

org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/MqttAsyncClient.java

@@ -65,6 +65,7 @@
 import org.eclipse.paho.mqttv5.common.packet.MqttReturnCode;
 import org.eclipse.paho.mqttv5.common.packet.MqttSubscribe;
 import org.eclipse.paho.mqttv5.common.packet.MqttUnsubscribe;
+import org.eclipse.paho.mqttv5.common.util.MqttTopicValidator;
 
 /**
  * Lightweight client for talking to an MQTT server using non-blocking methods
@@ -1134,7 +1135,7 @@ public String getCurrentServerURI() {
 	 *             if the topic contains a '+' or '#' wildcard character.
 	 */
 	protected MqttTopic getTopic(String topic) {
-		MqttTopic.validate(topic, false/* wildcards NOT allowed */);
+		MqttTopicValidator.validate(topic, false/* wildcards NOT allowed */);
 
 		MqttTopic result = (MqttTopic) topics.get(topic);
 		if (result == null) {
@@ -1255,7 +1256,7 @@ public IMqttToken subscribe(MqttSubscription[] subscriptions, Object userContext
 				subs.append(subscriptions[i].toString());
 
 				// Check if the topic filter is valid before subscribing
-				MqttTopic.validate(subscriptions[i].getTopic(), true/* allow wildcards */);
+				MqttTopicValidator.validate(subscriptions[i].getTopic(), true/* allow wildcards */);
 			}
 			// @TRACE 106=Subscribe topicFilter={0} userContext={1} callback={2}
 			log.fine(CLASS_NAME, methodName, "106", new Object[] { subs.toString(), userContext, callback });
@@ -1459,7 +1460,7 @@ public IMqttToken unsubscribe(String[] topicFilters, Object userContext, MqttAct
 			// Although we already checked when subscribing, but invalid
 			// topic filter is meanless for unsubscribing, just prohibit it
 			// to reduce unnecessary control packet send to broker.
-			MqttTopic.validate(topicFilters[i], true/* allow wildcards */);
+			MqttTopicValidator.validate(topicFilters[i], true/* allow wildcards */);
 		}
 
 		// remove message handlers from the list for this client
@@ -1588,7 +1589,7 @@ public IMqttDeliveryToken publish(String topic, MqttMessage message, Object user
 		log.fine(CLASS_NAME, methodName, "111", new Object[] { topic, userContext, callback });
 
 		// Checks if a topic is valid when publishing a message.
-		MqttTopic.validate(topic, false/* wildcards NOT allowed */);
+		MqttTopicValidator.validate(topic, false/* wildcards NOT allowed */);
 
 		MqttDeliveryToken token = new MqttDeliveryToken(getClientId());
 		token.setActionCallback(callback);

org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/MqttConnectionOptions.java

@@ -30,6 +30,7 @@
 import org.eclipse.paho.mqttv5.common.MqttMessage;
 import org.eclipse.paho.mqttv5.common.packet.MqttProperties;
 import org.eclipse.paho.mqttv5.common.packet.UserProperty;
+import org.eclipse.paho.mqttv5.common.util.MqttTopicValidator;
 
 /**
  * Holds the set of options that control how the client connects to a server.
@@ -201,7 +202,7 @@ public void setWill(String topic, MqttMessage message) {
 		if (topic == null || message == null || message.getPayload() == null) {
 			throw new IllegalArgumentException();
 		}
-		MqttTopic.validate(topic, false); // Wildcards are not allowed
+		MqttTopicValidator.validate(topic, false); // Wildcards are not allowed
 		this.willDestination = topic;
 		this.willMessage = message;
 		// Prevent any more changes to the will message

org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/MqttTopic.java

@@ -15,10 +15,7 @@
  */
 package org.eclipse.paho.mqttv5.client;
 
-import java.io.UnsupportedEncodingException;
-
 import org.eclipse.paho.mqttv5.client.internal.ClientComms;
-import org.eclipse.paho.mqttv5.client.util.Strings;
 import org.eclipse.paho.mqttv5.common.MqttException;
 import org.eclipse.paho.mqttv5.common.MqttMessage;
 import org.eclipse.paho.mqttv5.common.MqttPersistenceException;
@@ -30,40 +27,6 @@
  */
 public class MqttTopic {
 
-	/**
-	 * The forward slash (/) is used to separate each level within a topic tree and
-	 * provide a hierarchical structure to the topic space. The use of the topic
-	 * level separator is significant when the two wildcard characters are
-	 * encountered in topics specified by subscribers.
-	 */
-	public static final String TOPIC_LEVEL_SEPARATOR = "/";
-
-	/**
-	 * Multi-level wildcard The number sign (#) is a wildcard character that matches
-	 * any number of levels within a topic.
-	 */
-	public static final String MULTI_LEVEL_WILDCARD = "#";
-
-	/**
-	 * Single-level wildcard The plus sign (+) is a wildcard character that matches
-	 * only one topic level.
-	 */
-	public static final String SINGLE_LEVEL_WILDCARD = "+";
-
-	/**
-	 * Multi-level wildcard pattern(/#)
-	 */
-	public static final String MULTI_LEVEL_WILDCARD_PATTERN = TOPIC_LEVEL_SEPARATOR + MULTI_LEVEL_WILDCARD;
-
-	/**
-	 * Topic wildcards (#+)
-	 */
-	public static final String TOPIC_WILDCARDS = MULTI_LEVEL_WILDCARD + SINGLE_LEVEL_WILDCARD;
-
-	// topic name and topic filter length range defined in the spec
-	private static final int MIN_TOPIC_LEN = 1;
-	private static final int MAX_TOPIC_LEN = 65535;
-	private static final char NUL = '\u0000';
 
 	private ClientComms comms;
 	private String name;
@@ -159,159 +122,4 @@ public String toString() {
 		return getName();
 	}
 
-	/**
-	 * Validate the topic name or topic filter
-	 *
-	 * @param topicString
-	 *            topic name or filter
-	 * @param wildcardAllowed
-	 *            true if validate topic filter, false otherwise
-	 * @throws IllegalArgumentException
-	 *             if the topic is invalid
-	 */
-	public static void validate(String topicString, boolean wildcardAllowed) throws IllegalArgumentException {
-		int topicLen = 0;
-		try {
-			topicLen = topicString.getBytes("UTF-8").length;
-		} catch (UnsupportedEncodingException e) {
-			throw new IllegalStateException(e.getMessage());
-		}
-
-		// Spec: length check
-		// - All Topic Names and Topic Filters MUST be at least one character
-		// long
-		// - Topic Names and Topic Filters are UTF-8 encoded strings, they MUST
-		// NOT encode to more than 65535 bytes
-		if (topicLen < MIN_TOPIC_LEN || topicLen > MAX_TOPIC_LEN) {
-			throw new IllegalArgumentException(String.format("Invalid topic length, should be in range[%d, %d]!",
-					new Object[] { new Integer(MIN_TOPIC_LEN), new Integer(MAX_TOPIC_LEN) }));
-		}
-
-		// *******************************************************************************
-		// 1) This is a topic filter string that can contain wildcard characters
-		// *******************************************************************************
-		if (wildcardAllowed) {
-			// Only # or +
-			if (Strings.equalsAny(topicString, new String[] { MULTI_LEVEL_WILDCARD, SINGLE_LEVEL_WILDCARD })) {
-				return;
-			}
-
-			// 1) Check multi-level wildcard
-			// Rule:
-			// The multi-level wildcard can be specified only on its own or next
-			// to the topic level separator character.
-
-			// - Can only contains one multi-level wildcard character
-			// - The multi-level wildcard must be the last character used within
-			// the topic tree
-			if (Strings.countMatches(topicString, MULTI_LEVEL_WILDCARD) > 1
-					|| (topicString.contains(MULTI_LEVEL_WILDCARD)
-							&& !topicString.endsWith(MULTI_LEVEL_WILDCARD_PATTERN))) {
-				throw new IllegalArgumentException(
-						"Invalid usage of multi-level wildcard in topic string: " + topicString);
-			}
-
-			// 2) Check single-level wildcard
-			// Rule:
-			// The single-level wildcard can be used at any level in the topic
-			// tree, and in conjunction with the
-			// multilevel wildcard. It must be used next to the topic level
-			// separator, except when it is specified on
-			// its own.
-			validateSingleLevelWildcard(topicString);
-
-			return;
-		}
-
-		// *******************************************************************************
-		// 2) This is a topic name string that MUST NOT contains any wildcard characters
-		// *******************************************************************************
-		if (Strings.containsAny(topicString, TOPIC_WILDCARDS)) {
-			throw new IllegalArgumentException("The topic name MUST NOT contain any wildcard characters (#+)");
-		}
-	}
-
-	private static void validateSingleLevelWildcard(String topicString) {
-		char singleLevelWildcardChar = SINGLE_LEVEL_WILDCARD.charAt(0);
-		char topicLevelSeparatorChar = TOPIC_LEVEL_SEPARATOR.charAt(0);
-
-		char[] chars = topicString.toCharArray();
-		int length = chars.length;
-		char prev = NUL, next = NUL;
-		for (int i = 0; i < length; i++) {
-			prev = (i - 1 >= 0) ? chars[i - 1] : NUL;
-			next = (i + 1 < length) ? chars[i + 1] : NUL;
-
-			if (chars[i] == singleLevelWildcardChar) {
-				// prev and next can be only '/' or none
-				if (prev != topicLevelSeparatorChar && prev != NUL || next != topicLevelSeparatorChar && next != NUL) {
-					throw new IllegalArgumentException(
-							String.format("Invalid usage of single-level wildcard in topic string '%s'!",
-									new Object[] { topicString }));
-
-				}
-			}
-		}
-	}
-
-	/**
-	 * Check the supplied topic name and filter match
-	 *
-	 * @param topicFilter
-	 *            topic filter: wildcards allowed
-	 * @param topicName
-	 *            topic name: wildcards not allowed
-	 * @return true if the topic matches the filter
-	 * @throws IllegalArgumentException
-	 *             if the topic name or filter is invalid
-	 */
-	public static boolean isMatched(String topicFilter, String topicName) throws IllegalArgumentException {
-		int topicPos = 0;
-		int filterPos = 0;
-		int topicLen = topicName.length();
-		int filterLen = topicFilter.length();
-
-		MqttTopic.validate(topicFilter, true);
-		MqttTopic.validate(topicName, false);
-
-		if (topicFilter.equals(topicName)) {
-			return true;
-		}
-
-		while (filterPos < filterLen && topicPos < topicLen) {
-			if (topicName.charAt(topicPos) == '/' && topicFilter.charAt(filterPos) != '/')
-
-				break;
-			if (topicFilter.charAt(filterPos) != '+' && topicFilter.charAt(filterPos) != '#'
-					&& topicFilter.charAt(filterPos) != topicName.charAt(topicPos))
-				break;
-			if (topicFilter.charAt(filterPos) == '+') { // skip until we meet the next separator, or end of string
-				int nextpos = topicPos + 1;
-				while (nextpos < topicLen && topicName.charAt(nextpos) != '/')
-					nextpos = ++topicPos + 1;
-			} else if (topicFilter.charAt(filterPos) == '#')
-				topicPos = topicLen - 1; // skip until end of string
-			filterPos++;
-			topicPos++;
-		}
-
-		if ((topicPos == topicLen) && (filterPos == filterLen)) {
-			return true;
-		} else {
-			/*
-			 * https://github.com/eclipse/paho.mqtt.java/issues/418
-			 * Covers edge case to match sport/# to sport
-			 */
-			if ((topicFilter.length() - topicName.length()) == 2 &&
-					topicFilter.substring(topicFilter.length() -2, topicFilter.length()).equals("/#")) {
-				String filterSub = topicFilter.substring(0, topicFilter.length() - 2);
-				if (filterSub.equals(topicName)) {
-					System.err.println("filterSub equals topicName: " + filterSub + " == " + topicName);
-					return true;
-				}
-			}
-		}
-		return false;
-	}
-
 }

org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/internal/ClientState.java

@@ -1115,6 +1115,7 @@ protected void notifyReceivedAck(MqttAck ack) throws MqttException {
 	 * @param ack
 	 *            - The Orphaned Ack
 	 * @throws MqttException
+	 *             if an exception occurs whilst handling orphaned Acks
 	 */
 	protected void handleOrphanedAcks(MqttAck ack) throws MqttException {
 		final String methodName = "handleOrphanedAcks";
@@ -1161,7 +1162,7 @@ protected void handleInboundPubRel(MqttPubRel pubRel) throws MqttException {
 			MqttPubComp pubComp = new MqttPubComp(MqttReturnCode.RETURN_CODE_SUCCESS, pubRel.getMessageId(),
 					new MqttProperties());
 			// @TRACE 668=Creating MqttPubComp: {0}
-			log.info(CLASS_NAME, methodName, "668", new Object[] { pubComp.toString()});
+			log.info(CLASS_NAME, methodName, "668", new Object[] { pubComp.toString() });
 			this.send(pubComp, null);
 		}
 	}

org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/internal/CommsCallback.java

@@ -32,7 +32,6 @@
 import org.eclipse.paho.mqttv5.client.MqttDeliveryToken;
 import org.eclipse.paho.mqttv5.client.MqttDisconnectResponse;
 import org.eclipse.paho.mqttv5.client.MqttToken;
-import org.eclipse.paho.mqttv5.client.MqttTopic;
 import org.eclipse.paho.mqttv5.client.logging.Logger;
 import org.eclipse.paho.mqttv5.client.logging.LoggerFactory;
 import org.eclipse.paho.mqttv5.common.MqttException;
@@ -45,6 +44,7 @@
 import org.eclipse.paho.mqttv5.common.packet.MqttPublish;
 import org.eclipse.paho.mqttv5.common.packet.MqttReturnCode;
 import org.eclipse.paho.mqttv5.common.packet.UserProperty;
+import org.eclipse.paho.mqttv5.common.util.MqttTopicValidator;
 
 /**
  * Bridge between Receiver and the external API. This class gets called by
@@ -596,7 +596,7 @@ protected boolean deliverMessage(String topicName, int messageId, MqttMessage aM
 		if (aMessage.getProperties().getSubscriptionIdentifiers().isEmpty()) {
 			// No Subscription IDs, use topic filter matching
 			for (Map.Entry<String, Integer> entry : this.callbackTopicMap.entrySet()) {
-				if (MqttTopic.isMatched(entry.getKey(), topicName)) {
+				if (MqttTopicValidator.isMatched(entry.getKey(), topicName)) {
 					aMessage.setId(messageId);
 					this.callbackMap.get(entry.getValue()).messageArrived(topicName, aMessage);
 					delivered = true;

org.eclipse.paho.mqttv5.client/src/test/java/org/eclipse/paho/mqttv5/client/test/StringValidationTest.java

@@ -10,6 +10,7 @@
 import java.util.List;
 
 import org.eclipse.paho.mqttv5.common.MqttException;
+import org.eclipse.paho.mqttv5.common.util.MqttTopicValidator;
 
 /**
  * MQTT v5 Properties Class. This Class contains all of the available MQTTv5
@@ -1225,6 +1226,7 @@ public String getResponseTopic() {
 	 *            The Response Topic.
 	 */
 	public void setResponseTopic(String responseTopic) {
+		MqttTopicValidator.validate(responseTopic, false);
 		this.responseTopic = responseTopic;
 	}