Crash when using time slider

[ghost]

Someone on my server used the "import" function on an SVG file. This put the XML source in the document which screwed it up. Trying to revert, they opened the history. When sliding the slider, Etherpad crashes:

2018-04-08_21:39:49.94947 [2018-04-08 23:39:49.949] [ERROR] console - TypeError: Cannot read property 'substring' of undefined
2018-04-08_21:39:49.94951     at Object.insert (/opt/etherpad/6a38826/src/static/js/Changeset.js:794:45)
2018-04-08_21:39:49.94951     at Object.exports.mutateTextLines (/opt/etherpad/6a38826/src/static/js/Changeset.js:966:11)
2018-04-08_21:39:49.94952     at /opt/etherpad/6a38826/src/node/handler/PadMessageHandler.js:1519:19
2018-04-08_21:39:49.94953     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:610:21
2018-04-08_21:39:49.94953     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:249:17
2018-04-08_21:39:49.94954     at iterate (/opt/etherpad/6a38826/src/node_modules/async/lib/async.js:149:13)
2018-04-08_21:39:49.94954     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:160:25
2018-04-08_21:39:49.94955     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:251:21
2018-04-08_21:39:49.94956     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:615:34
2018-04-08_21:39:49.94956     at /opt/etherpad/6a38826/src/node_modules/async/lib/async.js:254:17
2018-04-08_21:39:49.94957 [2018-04-08 23:39:49.949] [INFO] console - graceful shutdown...
2018-04-08_21:39:50.08340 [2018-04-08 23:39:50.083] [INFO] console - db sucessfully closed.

The server was not supervised by a supervisor so it remained dead. I didn't start it again.

EPL version: 1.5.0 (89ad3cb)

[ghost]

I then upgraded to a new version. When EPL is now restarted by my supervisor, it crashes again when the slider for that document is used again. History views for other documents work fine.

EPL version: 1.6.4 (6a38826)

[marcovtwout]

Having the exact same error, reproduced on 1.6.6

[marcovtwout]

This could be related to: #2836

[muxator]

Is the crash still observed? I am trying to find what are the bugs that still need to be fixed.

[marcovtwout]

@muxator Yes, I can still trigger this error in a specific document. Unfornutately I cannot give you the exact content, but the stacktrace hopefully provides enough information:

[2018-08-20 09:10:12.186] [ERROR] console - TypeError: Cannot read property 'substring' of undefined
    at Object.insert (/opt/etherpad/etherpad-lite/src/static/js/Changeset.js:794:45)
    at Object.exports.mutateTextLines (/opt/etherpad/etherpad-lite/src/static/js/Changeset.js:966:11)
    at /opt/etherpad/etherpad-lite/src/node/handler/PadMessageHandler.js:1614:19
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:610:21
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:249:17
    at iterate (/opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:149:13)
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:160:25
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:251:21
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:615:34
    at /opt/etherpad/etherpad-lite/src/node_modules/async/lib/async.js:254:17

[JohnMcLear]

@marcovtwout I'm going to assume since 1.8.4 you can't replicate this? I will close but if you can still replicate in 1.8.4 let me know and I will re-open and solve ASAP.

[marcovtwout]

I don't have the document readily available to reproduce the original error, but I'm not sure it is fixed yet. Comparing the code from two years ago and now, I see an extra condition check that calls console.error when curSplice[sline] is undefined. But note that console.error will not stop execution: the code continues and will probably trigger the same error as above, this time in line 797.

[webzwo0i]

ack, this should be fixed when #2911 lands. I'm on it now.

This bug here is in the timeslider (calculating forward changesets) so I think I will be able to reproduce when importing crafted changesets. Of course it should never crash the instance (because malformed files could always be imported...), however I suspect this bug can happen during normal editing. Right now I only can reproduce within unit tests (easysync_tests.js)

So it would be great in case you happen to get to this document again to at least run bin/checkPad.js on it

There are two checks disabled in ace2_inner.js
https://github.com/ether/etherpad-lite/blob/develop/src/static/js/ace2_inner.js#L517 and https://github.com/ether/etherpad-lite/blob/develop/src/static/js/ace2_inner.js#L2075 that might be relevant when submitting changes. They might prevent those broken changesets from being submitted.

However, enabling those checks might render pads slow at least for checkALines and large documents

[JohnMcLear]

I disagree.

Note: Etherpad provide a setting to disallow unsupported files to be imported.

I have fuzz testing for import, file contents and pretty significant contentcollector in unit and integration coverage.

While it's plausible this case exists I'd say since the recent changes its improbable it can be reproduced.

The edge case I can consider is if the pad contents was written in a previous version with some plugins enabled and settings changed from default.

[webzwo0i]

OK. Will look into this a little bit and report back.

[marcovtwout]

@JohnMcLear Alright! If I ever encounter the error again I will report back :)

[JohnMcLear]

Awesome thanks!

[webzwo0i]

@JohnMcLear you are right. https://github.com/ether/etherpad-lite/blob/develop/src/node/handler/PadMessageHandler.js#L1306 getChangesetInfo is wrapped in try-catch so those exceptions don't make it crash anymore

[JohnMcLear]

Thanks for the spot @webzwo0i .. Been balls deep in EP code for 6 month now so nice to know I can spot something I handled without a deep dive.