Fixes from review

Author: embhorn

AGENTS.md

@@ -166,13 +166,30 @@ Every shell command's exit code must be checked.
 Never proceed after a silent failure.
 A command that failed and was ignored is not a completed step.
 
-### Test vector discipline
-Never derive test vectors from the code under test. Acceptable oracles:
-- OpenSSL (`openssl kdf`, `openssl enc`, `openssl pkcs8 ... | sha256sum`)
-- pyca/cryptography (`pkcs12.load_key_and_certificates`, `private_bytes(DER, PKCS8, NoEncryption)`)
-- Bouncy Castle test vectors
-Compute once, hardcode as `hex!(...)` literals or committed binary fixtures. Tests must be fully offline.
-For PKCS12KDF: use `hexpass:` (BMP/UTF-16BE + null terminator), NOT `pass:` (raw ASCII).
+## MQTT Specification Discipline
+Wire format and protocol behavior are governed by the published MQTT specifications. Treat the spec as the source of truth, not the code.
+
+Relevant specifications:
+- MQTT v3.1.1 — OASIS Standard (`mqtt-v3.1.1-os`)
+- MQTT v5.0 — OASIS Standard (`mqtt-v5.0`)
+- MQTT-SN v1.2 — OASIS
+
+When implementing or testing a normative requirement, cite it in a comment so reviewers can verify against the spec:
+- MQTT v3.1.1 / v5.0: bracketed conformance identifiers, e.g. `[MQTT-3.8.1-1]`, `[MQTT-2.3.1-1]`.
+- When a rule has no bracketed identifier, reference the section number, e.g. `MQTT 5.0 section 3.15.2`.
+- MQTT-SN v1.2: `MQTT-SN 1.2 section X.Y`.
+
+Match the version scope of the change. MQTT v5.0 adds packets and fields (AUTH, Reason Codes, Properties) that do not exist in v3.1.1; do not apply a v5-only rule to v3.1.1 code paths or vice versa. Guard v5-only logic with `WOLFMQTT_V5` and v3.1.1-only logic accordingly.
+
+## Test Oracle Discipline
+Do not use the code under test as its own oracle for wire-format behavior. For encoder or decoder tests, use one of:
+- A hand-constructed byte sequence that matches the spec wire format, built by reading the relevant section (not captured from encoder output). Comment the byte layout so the fixture is auditable.
+- Values from the spec's worked examples or conformance annex.
+- A cross-check against an independent implementation (e.g. mosquitto) captured once and committed as a fixed byte array.
+
+Roundtrip tests (encode then decode, or vice versa) are acceptable for regression and structural coverage, but they cannot be the sole oracle for a wire-format rule — a bug present in both encoder and decoder will still roundtrip. Pair roundtrip coverage with at least one independent fixture per rule.
+
+Tests must be fully offline and must not fetch vectors from the network at runtime.
 
 ## Dependencies
 

scripts/broker.test

@@ -327,12 +327,13 @@ if ./$sub_bin -h 2>&1 | grep -q "Max packet size"; then
 fi
 if [ "$v5_active" = "yes" ]; then
     LO_BOUND=10
-    HI_BOUND=13
+    HI_BOUND=14
 else
     LO_BOUND=5
-    HI_BOUND=8
+    HI_BOUND=9
 fi
 rm -f "${TMP_DIR}/t6b_watcher.ready" "${TMP_DIR}/t6b_client.ready"
+t6b_setup_ok=1
 timeout 30 ./$sub_bin -T -h 127.0.0.1 -p $port -n "wolfMQTT/example/lwttopic" \
     -i "t6b_watcher" -R "${TMP_DIR}/t6b_watcher.ready" \
     >"${TMP_DIR}/t6b_sub.log" 2>&1 &
@@ -341,6 +342,7 @@ TEST_PIDS+=($T6B_WATCHER_PID)
 if ! wait_for_file "${TMP_DIR}/t6b_watcher.ready" 5; then
     echo "FAIL: Keep-alive timeout window (watcher did not become ready)"
     FAIL=1
+    t6b_setup_ok=0
 fi
 ./$sub_bin -T -h 127.0.0.1 -p $port -n "test/ka2_trigger" -i "ka2_client" -l -k 4 \
     -R "${TMP_DIR}/t6b_client.ready" >"${TMP_DIR}/t6b_client.log" 2>&1 &
@@ -349,31 +351,36 @@ TEST_PIDS+=($T6B_CLIENT_PID)
 if ! wait_for_file "${TMP_DIR}/t6b_client.ready" 5; then
     echo "FAIL: Keep-alive timeout window (client did not become ready)"
     FAIL=1
+    t6b_setup_ok=0
+fi
+if [ $t6b_setup_ok -eq 1 ]; then
+    kill -STOP $T6B_CLIENT_PID 2>/dev/null
+    T6B_T_START=$(date +%s)
+    T6B_ELAPSED=-1
+    T6B_DEADLINE=$((T6B_T_START + HI_BOUND + 5))
+    while [ $(date +%s) -lt $T6B_DEADLINE ]; do
+        if grep -q "ka2_client" "${TMP_DIR}/t6b_sub.log" 2>/dev/null; then
+            T6B_ELAPSED=$(($(date +%s) - T6B_T_START))
+            break
+        fi
+        sleep 0.1
+    done
 fi
-T6B_T_START=$(date +%s)
-kill -STOP $T6B_CLIENT_PID 2>/dev/null
-T6B_ELAPSED=-1
-T6B_DEADLINE=$((T6B_T_START + HI_BOUND + 5))
-while [ $(date +%s) -lt $T6B_DEADLINE ]; do
-    if grep -q "ka2_client" "${TMP_DIR}/t6b_sub.log" 2>/dev/null; then
-        T6B_ELAPSED=$(($(date +%s) - T6B_T_START))
-        break
-    fi
-    sleep 0.1
-done
 kill -9 $T6B_CLIENT_PID 2>/dev/null
 wait $T6B_CLIENT_PID 2>/dev/null || true
 kill $T6B_WATCHER_PID 2>/dev/null
 wait $T6B_WATCHER_PID 2>/dev/null || true
 TEST_PIDS=()
-if [ $T6B_ELAPSED -lt 0 ]; then
-    echo "FAIL: Keep-alive timeout window (no LWT within $((HI_BOUND + 5))s)"
-    FAIL=1
-elif [ $T6B_ELAPSED -lt $LO_BOUND ] || [ $T6B_ELAPSED -gt $HI_BOUND ]; then
-    echo "FAIL: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
-    FAIL=1
-else
-    echo "PASS: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
+if [ $t6b_setup_ok -eq 1 ]; then
+    if [ $T6B_ELAPSED -lt 0 ]; then
+        echo "FAIL: Keep-alive timeout window (no LWT within $((HI_BOUND + 5))s)"
+        FAIL=1
+    elif [ $T6B_ELAPSED -lt $LO_BOUND ] || [ $T6B_ELAPSED -gt $HI_BOUND ]; then
+        echo "FAIL: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
+        FAIL=1
+    else
+        echo "PASS: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
+    fi
 fi
 fi # has_will
 

src/mqtt_packet.c

@@ -2760,7 +2760,7 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
             }
         }
         else {
-            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
         }
         }
         else {

tests/test_mqtt_packet.c

@@ -804,6 +804,41 @@ TEST(encode_subscribe_topic_filter_oversized_rejected)
     ASSERT_TRUE(rc < 0);
 }
 
+/* when multiple topics are supplied and a later one is oversized,
+ * the encoder must still reject — the length-validation loop covers every
+ * entry, not just the first. */
+TEST(encode_subscribe_topic_filter_oversized_second_rejected)
+{
+    const int str_len = 0x10000;
+    const int buf_len = str_len + 128;
+    byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len);
+    char *filter = (char*)WOLFMQTT_MALLOC(str_len + 1);
+    MqttSubscribe sub;
+    MqttTopic topics[2];
+    int rc;
+
+    if (tx_buf == NULL || filter == NULL) {
+        WOLFMQTT_FREE(tx_buf);
+        WOLFMQTT_FREE(filter);
+        FAIL("allocation failed");
+    }
+    XMEMSET(filter, 'B', str_len);
+    filter[str_len] = '\0';
+
+    XMEMSET(&sub, 0, sizeof(sub));
+    XMEMSET(topics, 0, sizeof(topics));
+    topics[0].topic_filter = "ok/topic";
+    topics[1].topic_filter = filter;
+    sub.topics = topics;
+    sub.topic_count = 2;
+    sub.packet_id = 1;
+    rc = MqttEncode_Subscribe(tx_buf, buf_len, &sub);
+
+    WOLFMQTT_FREE(filter);
+    WOLFMQTT_FREE(tx_buf);
+    ASSERT_TRUE(rc < 0);
+}
+
 /* [MQTT-3.8.1-1] SUBSCRIBE fixed header reserved flags MUST equal 0b0010,
  * which encodes QoS 1. Verifies the QoS 1 bit directly via
  * MQTT_PACKET_FLAGS_GET_QOS so a mutation of MQTT_QOS_1 to MQTT_QOS_0 in
@@ -2027,7 +2062,7 @@ TEST(auth_v5_invalid_reason_code_rejected)
 
     XMEMSET(&dec, 0, sizeof(dec));
     dec_len = MqttDecode_Auth(buf, 4, &dec);
-    ASSERT_TRUE(dec_len < 0);
+    ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, dec_len);
 }
 #endif /* WOLFMQTT_V5 */
 
@@ -2109,6 +2144,7 @@ void run_mqtt_packet_tests(void)
     RUN_TEST(encode_subscribe_options_byte_qos1);
     RUN_TEST(encode_subscribe_options_byte_qos2);
     RUN_TEST(encode_subscribe_topic_filter_oversized_rejected);
+    RUN_TEST(encode_subscribe_topic_filter_oversized_second_rejected);
     RUN_TEST(encode_subscribe_has_qos1_flag);
 
     /* MqttEncode_Unsubscribe */