Skip to content

Implement PINGREQ / PINGRESP support #125

Description

@crazyrokr

Summary

The broker has no registered handler for MqttMessageType.PING_REQUEST (type ID 12). When a client sends a PINGREQ, the packet parses correctly but the handler lookup returns null, causing a NullPointerException that is caught and logged as a warning. The connection stays open but no PINGRESP is sent back to the client. This violates MQTT spec [MQTT-3.12.4-1]: "The Server MUST send a PINGRESP packet in response to a PINGREQ packet."

Implementation Plan

Step 1: Create PingRequestMqttInMessageHandler

New file: core-service/src/main/java/javasabr/mqtt/service/handler/impl/PingRequestMqttInMessageHandler.java

@Component
public class PingRequestMqttInMessageHandler implements MqttInMessageHandler {

    private final MqttMessageOutFactoryService messageOutFactoryService;

    public PingRequestMqttInMessageHandler(MqttMessageOutFactoryService messageOutFactoryService) {
        this.messageOutFactoryService = messageOutFactoryService;
    }

    @Override
    public MqttMessageType messageType() {
        return MqttMessageType.PING_REQUEST;
    }

    @Override
    public void processValidMessage(MqttConnection connection, MqttInMessage message) {
        var user = connection.user();
        var factory = messageOutFactoryService.getFactory(connection);
        var pingResponse = factory.createPingResponse();
        user.sendAsync(pingResponse);
    }
}

Step 2: Verify createPingResponse() exists in factory hierarchy

Check if Mqtt311MessageOutFactory and Mqtt5MessageOutFactory have a createPingResponse() method. If not, add one that creates a PingResponseMqttOutMessage (type 13, zero-length body).

Step 3: Register handler

Spring @Component annotation on PingRequestMqttInMessageHandler will auto-register it via component scan. Verify it is picked up by the externalMqttConnectionService bean which accepts Collection<? extends MqttInMessageHandler>.

Step 4: Add null guard in DefaultConnectionService

Even with the PINGREQ fix, add a defensive null check:

MqttInMessageHandler messageHandler = inMessageHandlers[mqttInMessage.messageTypeId()];
if (messageHandler == null) {
    log.warning(mqttInMessage, "Received not supported MQTT message:[%s]"::formatted);
    return;
}
messageHandler.processValidMessage(connection, mqttInMessage);

This removes the reliance on NPE as a control flow mechanism and the //noinspection DataFlowIssue suppression.

Unit Tests

// In a new PingRequestMqttInMessageHandlerTest.groovy

def "should send PINGRESP in response to PINGREQ"() {
    given:
    def connection = mockConnection(MQTT_3_1_1)
    def pingreq = new PingRequestMqttInMessage((byte) 0)
    def handler = new PingRequestMqttInMessageHandler(factoryService)

    when:
    handler.processValidMessage(connection, pingreq)

    then:
    1 * connection.user().sendAsync(_) >> { args ->
        assert args[0] instanceof PingResponseMqttOutMessage
    }
}

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions