You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/04/02 08:00:33 UTC
[GitHub] [activemq-artemis] brusdev opened a new pull request #3058:
ARTEMIS-2686 Fix MQTT connect message rejection
brusdev opened a new pull request #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058
Initialize the session state with a default value to fix a NPE, when an incoming
MQTT interceptor rejects a MqttConnectMessage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#3058: ARTEMIS-2686 Fix MQTT connect message rejection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058#discussion_r402124163
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTRejectingInterceptorTest.java
##########
@@ -61,4 +62,36 @@ public boolean intercept(MqttMessage packet, RemotingConnection connection) thro
subscribeProvider.disconnect();
publishProvider.disconnect();
}
+
+ @Test(timeout = 60000)
+ public void testRejectedMqttConnectMessage() throws Exception {
+
+ server.getRemotingService().addIncomingInterceptor((MQTTInterceptor) (packet, connection) -> {
+ if (packet.getClass() == MqttConnectMessage.class) {
+ return false;
+ } else {
+ return true;
+ }
+ });
+
+ Thread publishThread = new Thread(() -> {
+ MQTTClientProvider publishProvider = getMQTTClientProvider();
+
+ try{
+ initializeConnection(publishProvider);
+ publishProvider.disconnect();
+ fail("The connection should be rejected!");
+ } catch (Exception e) {
+ e.printStackTrace();
Review comment:
If this is the expected behaviour you can just ignore the exception or we risk to read the test logs and think that there is some error!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request
#3058: ARTEMIS-2686 Fix MQTT connect message rejection
Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058#discussion_r402124625
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTRejectingInterceptorTest.java
##########
@@ -61,4 +62,36 @@ public boolean intercept(MqttMessage packet, RemotingConnection connection) thro
subscribeProvider.disconnect();
publishProvider.disconnect();
}
+
+ @Test(timeout = 60000)
+ public void testRejectedMqttConnectMessage() throws Exception {
+
+ server.getRemotingService().addIncomingInterceptor((MQTTInterceptor) (packet, connection) -> {
+ if (packet.getClass() == MqttConnectMessage.class) {
+ return false;
+ } else {
+ return true;
+ }
+ });
+
+ Thread publishThread = new Thread(() -> {
Review comment:
We really need to use a separate Thread for this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] brusdev commented on a change in pull request
#3058: ARTEMIS-2686 Fix MQTT connect message rejection
Posted by GitBox <gi...@apache.org>.
brusdev commented on a change in pull request #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058#discussion_r402134843
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTRejectingInterceptorTest.java
##########
@@ -61,4 +62,36 @@ public boolean intercept(MqttMessage packet, RemotingConnection connection) thro
subscribeProvider.disconnect();
publishProvider.disconnect();
}
+
+ @Test(timeout = 60000)
+ public void testRejectedMqttConnectMessage() throws Exception {
+
+ server.getRemotingService().addIncomingInterceptor((MQTTInterceptor) (packet, connection) -> {
+ if (packet.getClass() == MqttConnectMessage.class) {
+ return false;
+ } else {
+ return true;
+ }
+ });
+
+ Thread publishThread = new Thread(() -> {
Review comment:
The MQTTClientProvider hasn't a connection timeout. I could remove the thread and use the test timeout but I would lose the fail message "The connection is stuck!".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #3058: ARTEMIS-2686
Fix MQTT connect message rejection
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] brusdev commented on issue #3058: ARTEMIS-2686
Fix MQTT connect message rejection
Posted by GitBox <gi...@apache.org>.
brusdev commented on issue #3058: ARTEMIS-2686 Fix MQTT connect message rejection
URL: https://github.com/apache/activemq-artemis/pull/3058#issuecomment-607726163
@franz1981 I pushed some changes according to your suggestions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services