Skip to content

Commit fbdeba0

Browse files
jpwsuttonjpwsutton
committed
Fixing issue where attempting to send a packet with no properties caused a nullpointer exception
Signed-off-by: James Sutton <james.sutton@uk.ibm.com>
1 parent 7416251 commit fbdeba0

18 files changed

Lines changed: 184 additions & 23 deletions

File tree

‎org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/internal/ConnectActionListener.java‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@ public ConnectActionListener(MqttAsyncClient client, MqttClientPersistence persi
108108
* the {@link IMqttToken} from the successful connection
109109
*/
110110
public void onSuccess(IMqttToken token) {
111-
userToken.internalTok.markComplete(token.getResponse(), null);
112-
userToken.internalTok.notifyComplete();
113-
userToken.internalTok.setClient(this.client); // fix bug 469527 - maybe should be set elsewhere?
114-
115111
// Set properties imposed on us by the Server
116112
MqttToken myToken = (MqttToken) token;
117113
mqttSession.setReceiveMaximum(myToken.getMessageProperties().getReceiveMaximum());
@@ -124,10 +120,34 @@ public void onSuccess(IMqttToken token) {
124120
mqttSession.setSubscriptionIdentifiersAvailable(
125121
myToken.getMessageProperties().isSubscriptionIdentifiersAvailable());
126122
mqttSession.setSharedSubscriptionsAvailable(myToken.getMessageProperties().isSharedSubscriptionAvailable());
123+
124+
// If we are assigning the client ID post connect, then we need to re-initialise
125+
// our persistence layer.
127126
if (myToken.getMessageProperties().getAssignedClientIdentifier() != null) {
128127
mqttSession.setClientId(myToken.getMessageProperties().getAssignedClientIdentifier());
128+
try {
129+
persistence.open(myToken.getMessageProperties().getAssignedClientIdentifier());
130+
131+
if (options.isCleanSession()) {
132+
persistence.clear();
133+
}
134+
} catch (MqttPersistenceException exception) {
135+
136+
// If we fail to open persistence at this point, our best bet is to immediately
137+
// close the connection.
138+
try {
139+
client.disconnect();
140+
} catch (MqttException ex) {
141+
}
142+
onFailure(token, exception);
143+
return;
144+
}
129145
}
130146

147+
userToken.internalTok.markComplete(token.getResponse(), null);
148+
userToken.internalTok.notifyComplete();
149+
userToken.internalTok.setClient(this.client); // fix bug 469527 - maybe should be set elsewhere?
150+
131151
if (reconnect) {
132152
comms.notifyReconnect();
133153
}
@@ -197,10 +217,12 @@ public void connect() throws MqttPersistenceException {
197217
token.setActionCallback(this);
198218
token.setUserContext(this);
199219

200-
persistence.open(client.getClientId());
220+
if (!client.getClientId().equals("")) {
221+
persistence.open(client.getClientId());
201222

202-
if (options.isCleanSession()) {
203-
persistence.clear();
223+
if (options.isCleanSession()) {
224+
persistence.clear();
225+
}
204226
}
205227

206228
try {

‎org.eclipse.paho.mqttv5.client/src/main/java/org/eclipse/paho/mqttv5/client/internal/MqttSession.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ public String getClientId() {
122122
return clientId;
123123
}
124124

125-
126125
public void setClientId(String clientId) {
127126
this.clientId = clientId;
128127
}

‎org.eclipse.paho.mqttv5.client/src/test/java/org/eclipse/paho/mqttv5/client/test/ServerAssignedClientIdentifierTest.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package org.eclipse.paho.mqttv5.client.test;
2+
3+
import java.net.URI;
4+
import java.net.URISyntaxException;
5+
import java.util.logging.Logger;
6+
7+
import org.eclipse.paho.mqttv5.client.IMqttToken;
8+
import org.eclipse.paho.mqttv5.client.MqttAsyncClient;
9+
import org.eclipse.paho.mqttv5.client.MqttConnectionOptions;
10+
import org.eclipse.paho.mqttv5.client.persist.MemoryPersistence;
11+
import org.eclipse.paho.mqttv5.client.test.properties.TestProperties;
12+
import org.eclipse.paho.mqttv5.client.test.utilities.TestMemoryPersistence;
13+
import org.eclipse.paho.mqttv5.common.MqttException;
14+
import org.eclipse.paho.mqttv5.common.packet.MqttProperties;
15+
import org.junit.Assert;
16+
import org.junit.Test;
17+
18+
public class ServerAssignedClientIdentifierTest {
19+
private static URI serverURI;
20+
private static final String className = ServerAssignedClientIdentifierTest.class.getName();
21+
private static final Logger log = Logger.getLogger(className);
22+
23+
@Test
24+
/**
25+
* Tests that an MqttV5 client can connect to a broker with a null client
26+
* identifier. The broker should then return a new client identifier in the
27+
* CONNACK packet.
28+
*/
29+
public void connectWithNullClientIdentifier() throws URISyntaxException, MqttException, InterruptedException {
30+
serverURI = TestProperties.getServerURI();
31+
32+
TestMemoryPersistence memoryPersistence = new TestMemoryPersistence();
33+
34+
// Create an MqttAsyncClient with a null Client ID.
35+
MqttAsyncClient client = new MqttAsyncClient(serverURI.toString(), null, memoryPersistence, null, null);
36+
37+
IMqttToken connectToken = client.connect();
38+
connectToken.waitForCompletion(1000);
39+
Assert.assertTrue("The client should be connected.", client.isConnected());
40+
41+
MqttProperties connectProperties = connectToken.getResponse().getProperties();
42+
String assignedClientIdentifier = connectProperties.getAssignedClientIdentifier();
43+
log.info("Server assigned client identifer: " + assignedClientIdentifier);
44+
45+
// First, validate that we actually received an Assigned Client Identifier from
46+
// the Server
47+
Assert.assertNotNull("AssignedClilent Identifier should not be null.", assignedClientIdentifier);
48+
49+
// Validate that the Client now knows about it's own Client ID;
50+
Assert.assertEquals("The client should have the correct ID set.", assignedClientIdentifier,
51+
client.getClientId());
52+
53+
// Cleanup
54+
IMqttToken disconnectToken = client.disconnect();
55+
disconnectToken.waitForCompletion(1000);
56+
Assert.assertFalse("The client should now be disconnected.", client.isConnected());
57+
client.close();
58+
}
59+
60+
61+
}

‎org.eclipse.paho.mqttv5.client/src/test/java/org/eclipse/paho/mqttv5/client/test/utilities/TestMemoryPersistence.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
public class TestMemoryPersistence implements MqttClientPersistence {
3535

3636
private Hashtable data;
37+
private String clientId;
38+
private String serverURI;
3739

3840
/* (non-Javadoc)
3941
* @see org.eclipse.paho.client.mqttv3.MqttClientPersistence#close()
@@ -60,6 +62,8 @@ public MqttPersistable get(String key) throws MqttPersistenceException {
6062
* @see org.eclipse.paho.client.mqttv3.MqttClientPersistence#open(java.lang.String, java.lang.String)
6163
*/
6264
public void open(String clientId, String serverURI) throws MqttPersistenceException {
65+
this.clientId = clientId;
66+
this.serverURI = serverURI;
6367
if(this.data == null){
6468
this.data = new Hashtable();
6569
}
@@ -98,6 +102,30 @@ public void open(String clientId) throws MqttPersistenceException {
98102
data = new Hashtable();
99103

100104
}
105+
106+
public Hashtable getData() {
107+
return data;
108+
}
109+
110+
public void setData(Hashtable data) {
111+
this.data = data;
112+
}
113+
114+
public String getClientId() {
115+
return clientId;
116+
}
117+
118+
public void setClientId(String clientId) {
119+
this.clientId = clientId;
120+
}
121+
122+
public String getServerURI() {
123+
return serverURI;
124+
}
125+
126+
public void setServerURI(String serverURI) {
127+
this.serverURI = serverURI;
128+
}
101129

102130

103131
}

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttAuth.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ public MqttAuth(byte[] data) throws IOException, MqttException {
7474
*/
7575
public MqttAuth(int returnCode, MqttProperties properties) throws MqttException {
7676
super(MqttWireMessage.MESSAGE_TYPE_AUTH);
77-
this.properties = properties;
77+
if (properties != null) {
78+
this.properties = properties;
79+
} else {
80+
this.properties = new MqttProperties();
81+
}
7882
this.properties.setValidProperties(validProperties);
7983
validateReturnCode(returnCode, validReturnCodes);
8084
this.returnCode = returnCode;

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttConnAck.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,11 @@ public MqttConnAck(byte[] variableHeader) throws IOException, MqttException {
7070

7171
public MqttConnAck(boolean sessionPresent, int returnCode, MqttProperties properties) throws MqttException {
7272
super(MqttWireMessage.MESSAGE_TYPE_CONNACK);
73-
this.properties = properties;
73+
if (properties != null) {
74+
this.properties = properties;
75+
} else {
76+
this.properties = new MqttProperties();
77+
}
7478
this.properties.setValidProperties(validProperties);
7579
this.sessionPresent = sessionPresent;
7680
validateReturnCode(returnCode, validReturnCodes);

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttConnect.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ public MqttConnect(String clientId, int mqttVersion, boolean cleanSession, int k
154154
this.mqttVersion = mqttVersion;
155155
this.cleanSession = cleanSession;
156156
this.keepAliveInterval = keepAliveInterval;
157-
this.properties = properties;
157+
if (properties != null) {
158+
this.properties = properties;
159+
} else {
160+
this.properties = new MqttProperties();
161+
}
158162
this.properties.setValidProperties(validProperties);
159163
this.willProperties = willProperties;
160164
this.willProperties.setValidProperties(validWillProperties);

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttDisconnect.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ public MqttDisconnect(int returnCode, MqttProperties properties) throws MqttExce
7878
super(MqttWireMessage.MESSAGE_TYPE_DISCONNECT);
7979
validateReturnCode(returnCode, validReturnCodes);
8080
this.returnCode = returnCode;
81-
this.properties = properties;
81+
if (properties != null) {
82+
this.properties = properties;
83+
} else {
84+
this.properties = new MqttProperties();
85+
}
8286
this.properties.setValidProperties(validProperties);
8387
}
8488

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttPubAck.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ public MqttPubAck(int returnCode, int msgId, MqttProperties properties) throws M
6161
super(MqttWireMessage.MESSAGE_TYPE_PUBACK);
6262
this.returnCode = returnCode;
6363
this.msgId = msgId;
64-
this.properties = properties;
64+
if (properties != null) {
65+
this.properties = properties;
66+
} else {
67+
this.properties = new MqttProperties();
68+
}
6569
this.properties.setValidProperties(validProperties);
6670
validateReturnCode(returnCode, validReturnCodes);
6771
}

‎org.eclipse.paho.mqttv5.common/src/main/java/org/eclipse/paho/mqttv5/common/packet/MqttPubComp.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ public MqttPubComp(int returnCode, int msgId, MqttProperties properties) throws
6161
validateReturnCode(returnCode, validReturnCodes);
6262
this.returnCode = returnCode;
6363
this.msgId = msgId;
64-
this.properties = properties;
64+
if (properties != null) {
65+
this.properties = properties;
66+
} else {
67+
this.properties = new MqttProperties();
68+
}
6569
this.properties.setValidProperties(validProperties);
6670
}
6771

0 commit comments

Comments
 (0)