Skip to content

MQTT: do not crash when MQTTPublish has no underlayer - #5089

Open
DSeaStar wants to merge 1 commit into
secdev:masterfrom
DSeaStar:fix/mqtt-publish-no-underlayer
Open

MQTT: do not crash when MQTTPublish has no underlayer#5089
DSeaStar wants to merge 1 commit into
secdev:masterfrom
DSeaStar:fix/mqtt-publish-no-underlayer

Conversation

@DSeaStar

Copy link
Copy Markdown

Fixes #5071.

MQTTPublish reads QOS and len from pkt.underlayer in the msgid condition and the value length callback. Building or displaying a publish packet with no underlayer (MQTTPublish().show()) raised AttributeError: 'NoneType' object has no attribute 'QOS'.

Both callbacks now treat a missing underlayer as QoS 0 / empty payload, which matches how a standalone publish is constructed. Existing layered build/dissect tests are unchanged.

Repro that used to crash:

from scapy.contrib.mqtt import MQTTPublish
MQTTPublish().show()

./test/run_tests -P "load_contrib('mqtt')" -t test/contrib/mqtt.uts — 25 passed.

@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.58%. Comparing base (b36473a) to head (79525ee).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5089   +/-   ##
=======================================
  Coverage   80.58%   80.58%           
=======================================
  Files         390      390           
  Lines       96800    96800           
=======================================
+ Hits        78006    78008    +2     
+ Misses      18794    18792    -2     
Files with missing lines Coverage Δ
scapy/contrib/mqtt.py 97.34% <ø> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gpotter2

Copy link
Copy Markdown
Member

Could you disclose what tool you used to generate this PR in the commit tag? Thanks

@DSeaStar

Copy link
Copy Markdown
Author

Sure — the implementation and commit message were drafted with AI assistance (Claude), then reviewed and adjusted by me. I've amended the commit trailer to AI-Assisted: yes (Claude). Happy to use any specific trailer wording the project prefers.

@gpotter2

Copy link
Copy Markdown
Member

Okay thanks for the explanation. Your github profile is very sus, so I hope you don't mind me asking
image

MQTTPublish.msgid and MQTTPublish.value read QOS/len from the underlayer
without a None check, so MQTTPublish().show() raised AttributeError.
Guard both callbacks so a standalone publish packet can be displayed.

Fixes secdev#5071

AI-Assisted: yes (Claude)

Signed-off-by: SeaStar Deng <172368758@qq.com>
@DSeaStar
DSeaStar force-pushed the fix/mqtt-publish-no-underlayer branch from 79525ee to 2da8509 Compare August 15, 2026 10:14
@DSeaStar

Copy link
Copy Markdown
Author

Done — the commit trailer now reads AI-Assisted: yes (Claude) (amended in 2da8509).

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

Successfully merging this pull request may close these issues.

Missing pkt.underlayer causes MQTT to fail

2 participants