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
    }
}

Metadata

Metadata

Assignees

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions