Undoing causes disconnect

[fcassin]

Hello Etherpad Team,

We've found a reproducible bug with the clear authorship action that causes a disconnect.
To reproduce this bug :

• Log in to a pad with user A, activate author colors and type something
• Log in to the same pad with user B, activate author colors and type something
• As user B, click on clear authorship colors
• As user B, click on undo

User B should be disconnected from the pad.

Here is a relevant stack trace :

[2015-10-12 12:37:47.276] [WARN] console - Error: Can't apply USER_CHANGES, because Trying to submit changes as another author in changeset Z:151>0=2k*0|2=2*0=7*1|2=2*1=4$
    at /home/webapps/etherpad/src/node/handler/PadMessageHandler.js:683:25
    at /home/webapps/etherpad/src/node_modules/async/lib/async.js:610:21
    at /home/webapps/etherpad/src/node_modules/async/lib/async.js:249:17
    at iterate (/home/webapps/etherpad/src/node_modules/async/lib/async.js:149:13)
    at /home/webapps/etherpad/src/node_modules/async/lib/async.js:160:25
    at /home/webapps/etherpad/src/node_modules/async/lib/async.js:251:21
    at /home/webapps/etherpad/src/node_modules/async/lib/async.js:615:34
    at /home/webapps/etherpad/src/node/handler/PadMessageHandler.js:633:9
    at Object.exports.getPad (/home/webapps/etherpad/src/node/db/PadManager.js:161:5)
    at /home/webapps/etherpad/src/node/handler/PadMessageHandler.js:629:18

The issue seems to be in PadMessageHandler.js. The clear authorship message does not specify any author, thus not triggering the test on line 699.
However, when hitting the Undo button, two authors are specified, triggerring the error message.

Regards,
François

[webzwo0i]

Thank you for the detailed bug report.

clear authorship (undo that, respectively) is the only changeset that can has zero (deletion) or multiple (undo) authors at once. However we cannot simply disable the check, because this would introduce impersonation scenarios where an author can send changes as another author.

Maybe we should do this server-side instead of sending changesets.

This also needs a test, but at the moment the test infrastructure cannot easily express scenarios that involve multiple clients.

[webzwo0i]

also we might want that anybody can reimport the authorship colours and not only the client that deleted them

[muxator]

The issue still exists: replicated now on current develop branch (58c3154, 1.6.6+).

[2018-07-13 20:48:29.503] [WARN] console - Error: Can't apply USER_CHANGES, because Trying to submit changes as another author in changeset Z:q>0*0=9|1=1*1=f$
    at /opt/etherpad-lite/src/node/handler/PadMessageHandler.js:708:25
    at /opt/etherpad-lite/src/node_modules/async/lib/async.js:610:21
    at /opt/etherpad-lite/src/node_modules/async/lib/async.js:249:17
    at iterate (/opt/etherpad-lite/src/node_modules/async/lib/async.js:149:13)
    at /opt/etherpad-lite/src/node_modules/async/lib/async.js:160:25
    at /opt/etherpad-lite/src/node_modules/async/lib/async.js:251:21
    at /opt/etherpad-lite/src/node_modules/async/lib/async.js:615:34
    at /opt/etherpad-lite/src/node/handler/PadMessageHandler.js:658:9
    at Object.exports.getPad (/opt/etherpad-lite/src/node/db/PadManager.js:161:5)
    at /opt/etherpad-lite/src/node/handler/PadMessageHandler.js:654:18

[JohnMcLear]

I think there is a more serious bug/outcome of this:

VM1982:299 Uncaught Error: setAuthorInfo: author (undefined) is not a string
    at setAuthorInfo (eval at _compileFunction (faw:227), <anonymous>:299:13)
    at Object.Ace2Inner.editorInfo.ace_setAuthorInfo (eval at _compileFunction (faw:227), <anonymous>:1037:5)
    at Object.<anonymous> (pad.js?callback=require.define:2850)
    at action (pad.js?callback=require.define:2810)
    at pad.js?callback=require.define:2830
    at Function._.each._.forEach (ace2_common.js?callback=require.define:12709)
    at doActionsPendingInit (pad.js?callback=require.define:2829)
    at Object.info.onEditorReady (pad.js?callback=require.define:2954)
    at readyFunc (faw:17)
    at eval (eval at _compileFunction (faw:227), <anonymous>:5511:16)

Causes pad to fail loading.

Is fixed by restarting Etherpad...

[JohnMcLear]

I put a fix in that stops a user from being able to undo a clear authorship event. This seems like the only sane way of doing this.

[muxator]

People at Framasoft: would disallowing the undoing of clear autorship colors break some of your use cases? I think it's better than a crash.

[Chocobozzz]

People at Framasoft: would disallowing the undoing of clear autorship colors break some of your use cases?

Nope, it's good for us 👍