ssbc/muxrpc

It's possible to crash `ssb-server` by sending a message to close a stream too many times

Happy0 opened this issue · 3 comments

While doing some work with the scuttlebutt Java client in tuweni (https://github.com/apache/incubator-tuweni), my code erroneously closed a stream twice.

I believe the flow went like:

  • Client closed the connection.
  • The server confirms that it has closed its side of the connection.
  • The client closed the connection again.
  • ssb-server crashed with the following stack trace
happy0@geogaddi:~/projects/openlaw/server/dev/ssb/ssb-launcher$ node index.js --dir "/home/happy0/.ssb" -p 8008
(node:26143) Warning: N-API is an experimental feature and could change at any time.
ssb-friends: stream legacy api used
/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/muxrpc/stream.js:57
        var name = data.name
                        ^

TypeError: Cannot read property 'name' of null
    at PacketStreamSubstream.stream.read (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/muxrpc/stream.js:57:25)
    at PacketStream._onstream (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/packet-stream/index.js:224:13)
    at PacketStream.write (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/packet-stream/index.js:135:41)
    at /home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/muxrpc/pull-weird.js:56:15
    at /home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-stream/sinks/drain.js:24:37
    at /home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-goodbye/node_modules/pull-stream/throughs/filter.js:17:11
    at Object.cb (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/packet-stream-codec/index.js:111:11)
    at drain (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:39:14)
    at more (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:55:13)
    at Function.reader.read (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:99:7)
    at Object.cb (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/packet-stream-codec/index.js:104:16)
    at drain (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:39:14)
    at more (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:55:13)
    at /home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:66:9
    at Object.cb (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-box-stream/index.js:141:11)
    at drain (/home/happy0/projects/openlaw/server/dev/ssb/ssb-launcher/node_modules/ssb-server/node_modules/pull-reader/index.js:39:14)

Expected behaviour:

  • The server responds with an error like 'no stream with request number '
  • Perhaps the server also closes the connection, perhaps not.

Actual behaviour:

  • The whole server crashes.

I don't think there is really a way to respond with an error, because the stream has already ended, where do you send the error? we could log the error, ignore it, and closing the stream could be an option.

Aye, right enough... I think logging the error is perhaps the best approach. Closing the connection might be a bit of a severe way to handle it.

stale commented

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?