Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

variable header in connect() not clean on multiple connects #185

Closed
vladak opened this issue Nov 17, 2023 · 0 comments
Closed

variable header in connect() not clean on multiple connects #185

vladak opened this issue Nov 17, 2023 · 0 comments

Comments

@vladak
Copy link
Contributor

vladak commented Nov 17, 2023

I have a unit test for a MQTT server implementation (CPython) that goes like this:

import logging
import socket
import ssl
import time
import pytest

import adafruit_minimqtt.adafruit_minimqtt as MQTT

from ..common import mqtt_server


# Run multiple times to catch potentially erroneous reuse of client structures on the server.
@pytest.mark.parametrize("keep_alive_timeout", [8, 16])
def test_keepalive(keep_alive_timeout):
    """
    Connect with a particular keep alive timeout, sleep for double the keep alive timeout, send PINGREQ.
    """
    logging.basicConfig()
    logger = logging.getLogger(__name__)
    logger.setLevel(logging.DEBUG)

    host = "localhost"
    port = 1883
    mqtt_client = MQTT.MQTT(
        broker=host,
        port=port,
        socket_pool=socket,
        ssl_context=ssl.create_default_context(),
        connect_retries=1,
        recv_timeout=5,
        keep_alive=keep_alive_timeout,
    )

    logger.debug(f"connecting with keep alive = {keep_alive_timeout}")
    mqtt_client.connect()
    # The MQTT spec says not hearing from the client for 1.5 times keep alive timeout
    # means it can be considered dead and disconnected.
    sleep_period = 2 * keep_alive_timeout
    logger.debug(f"sleeping for {sleep_period} seconds")
    time.sleep(sleep_period)
    # Will wait for PINGRESP for keep_alive seconds. Should not get anything back.
    with pytest.raises(MQTT.MMQTTException):
        logger.debug("pinging the server")
        mqtt_client.ping()

This produces 2 MQTT TCP sessions. The CONNECT message in the first sessions is fine, however the keepalive value in the CONNECT message in the 2nd session is 24 (8+16 which is incidentally 1.5*16). This addition is caused by this code:

var_header[7] |= self.keep_alive >> 8
var_header[8] |= self.keep_alive & 0x00FF
specifically the bitwise OR operation. This is because the 1st session will modify the MQTT_HDR_CONNECT array and the 2nd session will use the data. Yes, the whole trouble is because the assignment on is not a copy, but a reference.

If I add this line of code to the unit test before the MQTT() initialization then the keep alive value in the 2nd CONNECT message is sent correctly:

    MQTT.MQTT_HDR_CONNECT = bytearray(b"\x04MQTT\x04\x02\0\0")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant