Crash with mosca.persistence.Mongo when parse topic error;

[keanuyaoo]

mosca with:{ backend:mongo,persistence:mongo}

crash when receive a error topic: ("msg":"published packet","packet":{"messageId":"Bkae1zBVx","topic":"1601000!\u0000\r160"},"client":"160100610","v":1);

(backend:mongo) work well and ingore this error and go on; but (persistence:mongo) throw it and server crash;

I think the trouble message should be ignored instead of causing crash;

here is the stack: C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\persistence\utils.js:40 var parts = topic.split("/"); ^

TypeError: Cannot read property 'split' of null at _topicPatterns (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\persistence\utils.js:40:20) at topicPatterns (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\persistence\utils.js:87:14) at MongoPersistence.storeOfflinePacket (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\persistence\mongo.js:290:18) at EventEmitter.server.storePacket (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\persistence\abstract.js:73:12) at EventEmitter.publish (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\server.js:366:8) at Client.handleAuthorizePublish (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\client.js:537:15) at C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\client.js:107:14 at EventEmitter.Server.authorizePublish (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\server.js:433:3) at Connection. (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\client.js:106:19) at emitOne (events.js:96:13) at Connection.emit (events.js:188:7) at Connection.emitPacket (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\connection.js:13:8) at emitOne (events.js:96:13) at Connection.emit (events.js:188:7) at Connection. (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib_streamreadable.js:786:14) at emitNone (events.js:86:13) at Connection.emit (events.js:185:7) at emitReadable (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib_stream_readable.js:448:10) at emitReadable (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib_stream_readable.js:444:5) at readableAddChunk (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib_stream_readable.js:187:9) at Connection.Readable.push (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib_stream_readable.js:149:10) at Writable.dummyWrite [as _write] (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\index.js:103:14)

[mcollina]

Would you mind in sending a PR to solve this issue?

[keanuyaoo]

I will try , this is my first PR on github,a little nervous !but I will try my best.

[keanuyaoo]

Please review my code; Thanks!!! https://github.com/keanuyaoo/mosca/commit/5c35c405a06fb2386ef7e276ddd15712e36ff1b8

[mcollina]

That should be already covered by the parsing logic. Can you add a unit test as well?

[keanuyaoo]

That should be already covered by the parsing logic.

I agress with you; The parsing logic "mqtt-packet" -> parse.js -> _parsePublish :

Parser.prototype._parsePublish` = function () { var packet = this.packet packet.topic = this._parseString()

if(packet.topic === null) return this.emit('error', new Error('cannot parse topic'))

// Parse message ID if (packet.qos > 0) { if (!this._parseMessageId()) { return } }

packet.payload = this._list.slice(this._pos, packet.length) }

This logic just emit a error event, it don't stop the process and the publish logic will go on; then my validation work; It not easy for me to add a js unit test ,because the js Client Lib check the publish packet; But my Embed Client (A Smart Device) can make it; Here is the log,you will find that my validation work, even the parse logic emit a error event:

{"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"Bkb683hiEx","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.783Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"B1mTL22jVl","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.788Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"SyVpU3niNe","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.823Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"B1r6832sNl","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.828Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"SJLpI3noVg","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.835Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"SyDa8h3jVl","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.841Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"Syua8n3o4l","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:56.871Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"timeout":180000,"msg":"setting keepalive timeout","time":"2016-12-24T09:13:56.874Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":40,"err":{"message":"cannot parse topic","name":"Error","stack":"Error: cannot parse topic\n at Parser._parsePublish (C:\Users\Yaochi\AppData\ \Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-packet\parser.js:263:31)\n at Parser._parsePayload (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server \node_modules\mqtt-packet\parser.js:117:14)\n at Parser.parse (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-packet\parser.js:43:48)\n at DestroyableTrans form.process [as _transform] (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\lib\parseStream.js:13:12)\n at DestroyableTransform.Transform._read (C:\ \Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\readable-stream\lib\_stream_transform.js:184:10)\n at DestroyableTransform.Transform._writ e (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\readable-stream\lib\_stream_transform.js:172:12)\n at doWrite (C:\Users\Yaochi\Ap pData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\readable-stream\lib\_stream_writable.js:237:10)\n at writeOrBuffer (C:\Users\Yaochi\AppData\Roaming\HBu ilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\readable-stream\lib\_stream_writable.js:227:5)\n at DestroyableTransform.Writable.write (C:\Users\Yaochi\AppData\Roaming\H Builder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\readable-stream\lib\_stream_writable.js:194:11)\n at Socket.ondata (_stream_readable.js:556:20)\n at emitOne (events.js:96 :13)\n at Socket.emit (events.js:188:7)\n at readableAddChunk (_stream_readable.js:177:18)\n at Socket.Readable.push (_stream_readable.js:135:10)\n at TCP.onread (net.js:542:20)"},"msg":"cannot parse topic","time":"2 016-12-24T09:13:56.878Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"msg":"closing client, reason: cannot parse topic","time":"2016-12-24T09:13:56.921Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"timeout":180000,"msg":"setting keepalive timeout","time":"2016-12-24T09:13:56.923Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":40,"err":{"message":"Publish Packet lack of necessary information","name":"Error","stack":"Error: Publish Packet lack of necessary information\n at Connection. (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mosca\lib\client.js:108:31)\n at emitOne (events.js:96:13)\n at Connection.emit (events. js:188:7)\n at Connection.emitPacket (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\connection.js:13:8)\n at emitOne (events.js:96:13)\n at Con nection.emit (events.js:188:7)\n at Connection. (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_streamread able.js:786:14)\n at emitNone (events.js:86:13)\n at Connection.emit (events.js:185:7)\n at emitReadable (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduple xer\node_modules\readable-stream\lib\_stream_readable.js:448:10)\n at emitReadable (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readab le-stream\lib\_stream_readable.js:444:5)\n at readableAddChunk (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_strea m_readable.js:187:9)\n at Connection.Readable.push (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_stream_readable.js: 149:10)\n at Writable.dummyWrite [as _write] (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\index.js:103:14)\n at doWrite (C:\Users\Yaochi\AppData\ Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_stream_writable.js:237:10)\n at writeOrBuffer (C:\Users\Yaochi\AppData\Roaming\HBuilder\userp rofiles\admin@kyygo.com\mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_stream_writable.js:227:5)\n at Writable.write (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com \mqtt_server\node_modules\reduplexer\node_modules\readable-stream\lib\_stream_writable.js:194:11)\n at write (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqt t-connection\node_modules\readable-stream\lib\_stream_readable.js:623:24)\n at flow (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules\ readable-stream\lib\_stream_readable.js:632:7)\n at DestroyableTransform.pipeOnReadable (C:\Users\Yaochi\AppData\Roaming\HBuilder\userprofiles\admin@kyygo.com\mqtt_server\node_modules\mqtt-connection\node_modules \readable-stream\lib\_stream_readable.js:664:5)\n at emitNone (events.js:86:13)\n at DestroyableTransform.emit (events.js:185:7)"},"msg":"Publish Packet lack of necessary information","time":"2016-12-24T09:13:56.943Z"," v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":20,"packet":{"messageId":"r1FpUh2iEl","topic":"16010061_cc_r"},"msg":"published packet","time":"2016-12-24T09:13:57.110Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":30,"topic":"ALL_glc","msg":"unsubscribed","time":"2016-12-24T09:13:57.115Z","v":0} {"name":"KTTMsgSvr","hostname":"KTT1014","pid":544,"client":"160100610","level":30,"topic":"16010061_cc","msg":"unsubscribed","time":"2016-12-24T09:13:57.123Z","v":0}

[mcollina]

Then the bug is that the parsing continues after an error.

[keanuyaoo]

Seems to be; I don't know the 'mqtt-packet' Lib is : just parse Buffer to Packet Object only , or handle some process logic still in design! I am more inclined to 'mqtt-packet' should provide trusted Packet;

[mcollina]

Can you please upload a script to reproduce this issue? It might be a malformed packet from the other side.

[ddresch]

I guess it's a malformed packet cause I ran into the same issue with a error dropped and a crashing Meteor app. We used mqtt.js version 2.2.1.

Error: Cannot parse topic

Which is probably not perfect and I'm with @keanuyaoo to change the process flow instead of dropping an error directly.

I tried to publish a topic with this signature topic/is/including/slashes it worked with an old version of Paho which connected via websocket to the mosquitto broker.

After a restart of mosquitto the app is running again.

[keanuyaoo]

First , Sorry for my deferred response ! This mouth is too busy because of All-Work-End before Chinese New Year; @ddresch @mcollina
Yes, a malformed packet cause it; This malformed packet is send by a Embed C MQTT Client ( a PIC32-based Device),It have logic error that will cause it sometimes! I fix the client code and I change the server code too;

Because: I use mosca for a Smart Home Platform! This Platform will be open for our partner! So I must make sure our server is strong and can handle error message correctly!

Even, I think that a Server's life should not be effected by a client; If the client is illegal,just refuse it, not crash.

Last, Sorry for my poor English, I expect you can understand what I mean; Thanks!

[mcollina]

I have updated mqtt-packet, and I think this should not happen again. You will need to remove your node_modules folder and reinstall.

If it does happen again, would you mind sending me a packet capture of the offeding one, ideally via email, so that I can develop a proper fix for the parser?

[mcollina]

it should be fixed, feel free to reopen.