Both functions did the same thing, the first function was buggy. Might be worth someone looking through the history of this file and pointing out when I made this heinous error ;)
The server was reusing the client's message when broadcasting userinfo
updates. This would allow a malicious client to insert arbitrary fields
into a message that the other clients would trust as coming from the
server. For example, adding "disconnect" or renaming other authors.
This commit fixes it by having the server construct a new message with
known fields before broadcasting.
The client might have disconnected between callbacks so don't try to
write to the session before checking this. The main callback of this
function now has a single check at its top.
Removed a redundant check halfway through the callback.
Also normalized use of client.id for the session index instead of a mix of
client.id and sessionId.
Added some explanatory comments.
We had a problem with the server running out of stack space if a client
submitted a changeset based on a revision more than about 1000 revs old.
(944 was our cutoff but yours may vary). This happened in the wild with
about 30 people editing via flaky wifi. A disconnected client would try
to submit a fairly old changeset when reconnecting, and a few minutes
was enough for 30 people to generate that many revs.
The stack kept growing because pad.getRevisionChangeset was being answered
from the cache, so no I/O interrupted the callback chain. (This was seen with
mysql, I don't know about other backends.)
This patch forces a nextTick every 200 revisions to solve this problem.
When stress testing etherpad-lite we occasionally got this error:
TypeError: Cannot read property 'author' of undefined
at /home/etherpad/etherpad-lite/src/node/handler/PadMessageHandler.js:556:47
handleUserChanges was accessing sessioninfos[client.id].author in a callback,
after spending some time in the loop that updates the changeset to the
latest revision. It's possible for a disconnect request to be processed
during that loop so the session might no longer be there.
This patch fixes it by looking up the author at the start of the function.
The utility functions colorutils.js assume that background colors are in
CSS hex format, so require userColor to do the same, rather than
allowing inputs like "red" and "rgba(...)", to insure that inversion
checks will succeed.
johnyma22