search

xmppjs / xmpp.js

XMPP for JavaScript
ISC License
2.19k stars 373 forks source link

xmpp connection should reset parser state for every message #756

Closed wichert closed 5 years ago

wichert commented 5 years ago

The Connection class roughly works like this:

  1. when you call Connection.open() a Parser instance is created
  2. when a message comes in on the websocket Connection._onData() is called, which writes it to the parser

Now something can go wrong during the parsing or processing of the message. If that happens the parser is left in an inconsistent state, which results in all future messages failing to parse.

Since every message is a complete XML fragment the code should make sure it always starts with a clean parser state when it starts parsing a new message, or it should create a new parser for every message.

wichert commented 5 years ago

For reference: we had a case today where this happened due to an exception being thrown in a plugin.

sonnyp commented 5 years ago

The expected (and standard) behavior if the XML is incorrect is for the Connection to send a stream error and close the socket. Anything else is a bug. Could you provide a test case to reproduce?

wichert commented 5 years ago

In our case the XML was correct. But because a xmpp.js plugin failed and threw an exception xmpp.js left the parser in the wrong state. This resulted in xmpp.js then failing to parse the next message coming in, and closing the connection.

sonnyp commented 5 years ago

I see.

Could you provide a simple test case to reproduce and explore options?