sveith
Recon's restore function processes the segments backwards?
First off, GREAT project. I'm using it for a real-time collaborative screenwriting app, www.WriterDuet.com. I believe I have discovered a bug in it, but fortunately I think the solution is simple.
I don't fully understand the code, so apologies if I'm missing something. One of our users hit the "Buffer splice operation out of bounds" exception, due to a fairly complex state which I believe included a loss of connectivity from one writer (and thus a number of changes needing to be applied to merge the states for their updates).
The recon segments buffer is clearly backwards for the splice operations to make sense:
JSON.stringify(this.segments) "[{"offset":14,"buffer":{"segments":[{"user":39,"text":"r"}]}},{"offset":13,"buffer":{"segments":[{"user":39,"text":"t"}]}},{"offset":12,"buffer":{"segments":[{"user":39,"text":" "}]}},{"offset":11,"buffer":{"segments":[{"user":39,"text":","}]}},{"offset":10,"buffer":{"segments":[{"user":39,"text":"y"}]}},{"offset":9,"buffer":{"segments":[{"user":39,"text":"a"}]}},{"offset":8,"buffer":{"segments":[{"user":39,"text":"w"}]}}]"
Offsets are from biggest to smallest, and the buffer we're splicing only has 8 characters, so only if we did the splicing in reverse order it would be fine.
Therefore I propose changing the restore function to this: Recon.prototype.restore = function(buffer) { var reversed = this.segments.slice(0).reverse(); for(var index in reversed) { var segment = reversed[index]; buffer.splice(segment.offset, 0, segment.buffer); } };
What do you think? Thanks for all the work creating this excellent OT library!
Thanks for the feedback, and I'm sorry for the delay in answering!
Your observation looks correct; the segments are indeed in the wrong order. I am currently not sure though which part of the code is incorrect ‒ it might be that we need to change the processing order in restore(), as you suggested, or that some parts of the code are adding the segments in the wrong order.
The current test cases don't seem to cover this problem, and it will take me some time to come up with one. If you have more information on how to reproduce this, that would help a lot.
In libinfinity there is this commit which seems to mention the same problem, although I don't remember the details: https://github.com/gobby/libinfinity/commit/ace2d2f92b6cea1b80f1faed877d98641f9005d2
More importantly, it adds a test case that you could easily use to see whether it is actually the same problem, and if yes, to reproduce it. There are also 4 more tests which uncovered some problems I have found and fixed with the algorithm within the last years.
