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

Crash when some binary data is published #89

Closed
Xploder opened this issue Jan 24, 2022 · 15 comments · Fixed by #91
Closed

Crash when some binary data is published #89

Xploder opened this issue Jan 24, 2022 · 15 comments · Fixed by #91
Labels
bug Something isn't working

Comments

@Xploder
Copy link

Xploder commented Jan 24, 2022

Hey,
on my iPhone 13 Pro, the app straight up crashes when trying to connect to my local broker.
There's no issue with another app (EasyMQTT) or any other application that is using this broker (Z2M, Hass, Valetudo, MQTTExplorer).
The sample brokers work fine.

Running mosquitto:latest docker container with authentication.

https://youtu.be/r-aZThRAo5Y

How can I assist in debugging this?

Thanks in advance!

@philipparndt
Copy link
Owner

Thank you for the detailed explanation 👍
Can you prove more information about your configuration?
I tried to reproduce it with the latest mosquitto version with user/password auth and without the password persisted.
This works as expected. However, your password seems to be empty but the connection works anyway. What settings do you have in your mosquitto.conf?

My test settings for user/password auth are:

persistence true
persistence_location /mosquitto/data/
log_dest file /mosquitto/log/mosquitto.log

password_file /mosquitto/config/passwd
allow_anonymous false

# this will listen for mqtt on tcp
listener 1883

# this will expect websockets connections
#listener 9001
#protocol websockets

# logging settings
#connection_messages false
log_type error

@Xploder
Copy link
Author

Xploder commented Jan 24, 2022

Thanks for responding so quickly!
The password is filled out but the iOS screen recorder removes even the dots.
In reality, the password is there.

Here's my current mosquitto config:

listener 1883
protocol mqtt
password_file /mosquitto/config/pwfile

autosave_interval 1800

persistence true
persistence_file mosquitto.db
persistence_location /mosquitto/config

EDIT: I've also added 'allow_anonymous false' for good measure.
EDIT2: I'm actually running "cmccambridge/mosquitto-unraid:latest" as my docker image. Maybe that's the culprit.
EDIT3: Switched to eclipse-mosquitto:latest docker image with new, sane default configuration pulled from https://github.com/eclipse/mosquitto/blob/master/mosquitto.conf. No change.

Mosquitto 2.0.14 log when trying to connect:

1643040548: mosquitto version 2.0.14 starting

1643040548: Config loaded from /mosquitto/config/mosquitto.conf.
1643040548: Opening ipv4 listen socket on port 1883.
1643040548: Opening ipv6 listen socket on port 1883.
1643040548: mosquitto version 2.0.14 running
...
1643040719: New connection from 192.168.175.107:56978 on port 1883.
1643040719: New client connected from 192.168.175.107:56978 as mqttexplorer (p2, c1, k60, u'esp01').
1643040719: Client mqttexplorer closed its connection.

@philipparndt
Copy link
Owner

Okay, so seems to be nothing special with your configuration.
Maybe the issue does not appear during the connection, but when receiving some initial messages that are on the broker with retain=true (could be something with the JSON parser for example).
Can you see any message that could be suspect? I also see some crashes on devices that have enabled "Developer Feedback" that are crashing in CocoaMQTT while decoding some a message. But I did not find a message to reproduce this and report an issue in the CocoaMQTT repo.

@Xploder
Copy link
Author

Xploder commented Jan 24, 2022

You're exactly right!
It's Valetudos "MapData" topic, which provides the vacuum's map in binary.

image

Once I delete the MapData topic from my broker, MQTTAnalyzer connects.

Anything we can do about it?

@philipparndt
Copy link
Owner

Okay, good to know 👍
I tried to post some ZIP content and some PNG file without success (means no crash ;)).
So it could be the map-data property or there is something special in the PNG of map-data-hass-hack property.

I'll try some more binaries and hope to get a crash for one file.

@philipparndt philipparndt added the bug Something isn't working label Jan 24, 2022
@philipparndt philipparndt changed the title Crash when trying to connect to local broker Crash when some binary data is published Jan 24, 2022
@Xploder
Copy link
Author

Xploder commented Jan 24, 2022

'map-data' is the problem, I can connect to the server with it deleted whilst still having 'map-data-hass-hack' present.

If you're on discord, add me and I will send you the payload so you can debug it better.

@philipparndt
Copy link
Owner

I don't have a discord account, but it would be great when you can send me the payload by mail.
You can find my mail address for example here:
https://github.com/philipparndt/mqtt-analyzer/commit/8a71233495e7f893d0c0e3e5e49a61551a2074b8.patch

@proddy
Copy link

proddy commented Jan 24, 2022

or just zip the payload into a ZIP file and add to a comment (it accepts zips). I can also test here if needed

@Xploder
Copy link
Author

Xploder commented Jan 24, 2022

or just zip the payload into a ZIP file and add to a comment (it accepts zips). I can also test here if needed

Do you have an idea about how to retrieve the payload? I've tried to write the payload to a file using mosquitto_sub but the output doesn't seem to make sense.
mapdata.txt

@proddy
Copy link

proddy commented Jan 24, 2022

it's probably using msgpack to compress the JSON. See if you can decode it. If you're handy with javascript/typescript there's an npm library that does the encode and decoding.

@Xploder
Copy link
Author

Xploder commented Jan 24, 2022

I'm not a dev sadly, but I've provided a way to access the payload to Philipp.

@philipparndt
Copy link
Owner

It is reproducible with the first three bytes of the message. I've created an issue in CocoaMQTT:
emqx/CocoaMQTT#433

@philipparndt
Copy link
Owner

@Xploder Apple approved the update, you should see an update to version 1.10 in the App Store 😄

@Xploder
Copy link
Author

Xploder commented Jan 29, 2022

Awesome. Thank you for investigating this and for mentioning me in the patchnotes. Much appreciated! 👍

@proddy
Copy link

proddy commented Jan 29, 2022

yeah really cool how you handled this fix. love this project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants