Ran into this issue recently. mqtt.patch is my take on the fix: new message parser, zero copy in most cases and using 128 bytes less RAM.

The topic is null-terminated, see mqtt.c line 755. So I think we should keep the last null byte in the test for the topic. I also tried without without checking the last nul byte and it also fails so the problem isn't there.

For the payload, I indeed didn't include the '\0' in the payload (rx_data) so you're right, this is corrected now in the new patch file.

(file #48422)

I could reproduce the problem, it comes when the topic ends-up being spread over 2 TCP segments. So I added 3 unit tests in the patch attached, a standard single segment incoming publish, and 2 segmented either in the topic or message.

The tests do pass with commit #ed561a5 :

Running suite(s): IPv4

  < Receive publish
    < Topic: "topic" (message len: 62)
    < Data: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" (len: 62, rem: 0)
> Receive segmented topic
    < Topic: "topic" (message len: 62)
    < Data: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" (len: 62, rem: 0)
> Receive segmented message
    < Topic: "topic" (message len: 62)
    < Data: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" (len: 62, rem: 0)
 SOCKETS
100%: Checks: 112, Failures: 0, Errors: 0

- They fail with master:

  < Receive publish
    < Topic: "topic" (message len: 62)
    < Data: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" (len: 62, rem: 0)
> Receive segmented topic
 SOCKETS
99%: Checks: 112, Failures: 0, Errors: 1
../../../../src/../test/unit/mqtt/test_mqtt.c:317:E:MQTT:segmented_sub_pub:0: (after this point) Received signal 11 (Segmentation fault)

(file #48421)

comment #1:

> I can't find the code you mention in git head. Could you send a proposed patch of how to fix this?

That has been changed in commit #2cc420e, at the bottom of this page: #2cc420e

-      pbuf_copy_partial(p, client->rx_buffer + cpy_start, cpy_len, in_offset);
+      pbuf_copy_partial(p, client->rx_buffer + fixed_hdr_len, cpy_len, in_offset);

I have not been able to grasp all the inner workings to patch it and there are multiple changes after this commit for the zero-copy part. So it might be better if I can reproduce the problem in master with a simple example.

What could I use as an example to reproduce this use case? We're using STM32 but the platform is irrelevant, so I better try with what are you using for testing.

I have a "Socket error on client..." since commit #2cc420e linked to bug #55607

I found that when I subscribe to a "topic/#" that has a few retained topics/values, the TCP packets are split into multiple segments that each hold multiple MQTT topic/payload.

When the end of the segment contains the beginning of a topic (say 10 chars), then the next segment get's the remaining of the topic and should write it on rx_buffer[10], which was the role of cpy_start and that offset was applied in

pbuf_copy_partial(p, client->rx_buffer + cpy_start, cpy_len, in_offset);

The changes seems to remove this:

-      pbuf_copy_partial(p, client->rx_buffer + cpy_start, cpy_len, in_offset);
+      pbuf_copy_partial(p, client->rx_buffer + fixed_hdr_len, cpy_len, in_offset);

I compared cpy_start with fixed_hdr_len and when the problem appears, cpy_start = 10 and fixed_hdr_len = 2 instead of 10.

I guess the problem can be reproduced by publishing various topics/subtopic with long payload, then subscribing to topic/#.