Fix copy command going out of bounds during delta creation

When creating a delta, the copy command could copy from rows that are
bigger than the height of the tile. This would go unnoticed on light
mode because the js side that applied the delta would apply these out of
bounds rows with a white color, but it is noticable in dark mode. Made
it so the copy command stops copying from out of bounds.

Signed-off-by: Paris Oplopoios <paris.oplopoios@collabora.com>
Change-Id: I4d05cb411958d0945933edb5c812de2dfd9c1abd

kit/ChildSession.cpp

@@ -438,7 +438,7 @@ bool ChildSession::_handleInput(const char *buffer, int length)
                tokens.equals(0, "rendersearchresult") ||
                tokens.equals(0, "contentcontrolevent") ||
                tokens.equals(0, "geta11yfocusedparagraph") ||
-               tokens.equals(0, "geta11ycaretposition"));
+               tokens.equals(0, "geta11ycaretposition") ||
                tokens.equals(0, "toggletiledumping"));
 
         std::string pzName("ChildSession::_handleInput:" + tokens[0]);
@@ -636,6 +636,7 @@ bool ChildSession::_handleInput(const char *buffer, int length)
         else if (tokens.equals(0, "geta11ycaretposition"))
         {
             return getA11yCaretPosition();
+        }
         else if (tokens.equals(0, "toggletiledumping"))
         {
             setDumpTiles(tokens[1] == "true");

kit/Delta.hpp

@@ -319,7 +319,8 @@ class DeltaGenerator {
         LOG_TRC("building delta of a " << cur.getWidth() << 'x' << cur.getHeight() << " bitmap " <<
                 "between old wid " << prev.getWid() << " and " << cur.getWid());
 
-        std::vector<char> output;
+        // let's use uint8_t instead of char to avoid implicit sign extension
+        std::vector<uint8_t> output;
         // guestimated upper-bound delta size
         output.reserve(cur.getWidth() * (cur.getHeight() + 4) * 4);
 
@@ -347,9 +348,12 @@ class DeltaGenerator {
                     // TODO: if offsets are >256 - use 16bits?
                     if (lastCopy > 0)
                     {
-                        char cnt = output[lastCopy];
-                        if (output[lastCopy + 1] + cnt == (char)(match) &&
-                            output[lastCopy + 2] + cnt == (char)(y))
+                        // check if we can extend the last copy
+                        uint8_t cnt = output[lastCopy];
+                        if (output[lastCopy + 1] + cnt == match &&
+                            output[lastCopy + 2] + cnt == y &&
+                            // make sure we're not copying from out of bounds of the previous tile
+                            output[lastCopy + 1] + cnt + 1 < prev.getHeight())
                         {
                             output[lastCopy]++;
                             matched = true;
@@ -552,12 +556,14 @@ class DeltaGenerator {
         TileWireId wid, bool forceKeyframe,
         bool dumpTiles, LibreOfficeKitTileMode mode)
     {
-
+        #if !ENABLE_DEBUG
+        dumpTiles = false;
+        #endif
         // Dump the tiles to the child sessions chroot jail
         int dumpedIndex = 1;
         if (dumpTiles)
         {
-            std::string path = FileUtil::getSysTempDirectoryPath() + "/tiledump";
+            std::string path = "/tmp/tiledump";
             bool directoryExists = !FileUtil::isEmptyDirectory(path);
             if (!directoryExists)
             {
@@ -662,8 +668,7 @@ class DeltaGenerator {
             // Dump the delta
             if (dumpTiles)
             {
-                std::string path(FileUtil::getSysTempDirectoryPath());
-                path += std::string("tiledump");
+                std::string path = "/tmp/tiledump";
                 std::ostringstream oss;
                 // filename format: tile-delta-<viewid>-<part>-<left>-<top>-<prev_index>_to_<index>.zstd
                 oss << "tile-delta-" << loc._canonicalViewId << "-" << loc._part << "-" << loc._left << "-" << loc._top

test/DeltaTests.cpp

@@ -23,12 +23,14 @@ class DeltaTests : public CPPUNIT_NS::TestFixture
 #if ENABLE_DELTAS
     CPPUNIT_TEST(testDeltaSequence);
     CPPUNIT_TEST(testRandomDeltas);
+    CPPUNIT_TEST(testDeltaCopyOutOfBounds);
 #endif
 
     CPPUNIT_TEST_SUITE_END();
 
     void testDeltaSequence();
     void testRandomDeltas();
+    void testDeltaCopyOutOfBounds();
 
     std::vector<char> loadPng(const char *relpath,
                               png_uint_32& height,
@@ -95,12 +97,14 @@ std::vector<char> DeltaTests::applyDelta(
         {
         case 'c': // copy row.
         {
-            int count = (uint8_t)(delta[i+1]);
-            int srcRow = (uint8_t)(delta[i+2]);
-            int destRow = (uint8_t)(delta[i+3]);
+            size_t count = static_cast<uint8_t>(delta[i+1]);
+            size_t srcRow = static_cast<uint8_t>(delta[i+2]);
+            size_t destRow = static_cast<uint8_t>(delta[i+3]);
+
+            LOK_ASSERT(srcRow + count <= height);
 
 //            std::cout << "copy " << count <<" row(s) " << srcRow << " to " << destRow << '\n';
-            for (int cnt = 0; cnt < count; ++cnt)
+            for (size_t cnt = 0; cnt < count; ++cnt)
             {
                 const char *src = &pixmap[width * (srcRow + cnt) * 4];
                 char *dest = &output[width * (destRow + cnt) * 4];
@@ -112,17 +116,24 @@ std::vector<char> DeltaTests::applyDelta(
         }
         case 'd': // new run
         {
-            int destRow = (uint8_t)(delta[i+1]);
-            int destCol = (uint8_t)(delta[i+2]);
-            size_t length = (uint8_t)(delta[i+3]);
+            size_t destRow = static_cast<uint8_t>(delta[i+1]);
+            size_t destCol = static_cast<uint8_t>(delta[i+2]);
+            size_t length = static_cast<uint8_t>(delta[i+3]);
             i += 4;
 
 //            std::cout << "new " << length << " at " << destCol << ", " << destRow << '\n';
             LOK_ASSERT(length <= width - destCol);
 
             char *dest = &output[width * destRow * 4 + destCol * 4];
-            for (size_t j = 0; j < length * 4 && i < delta.size(); ++j)
-                *dest++ = delta[i++];
+            for (size_t j = 0; j < length * 4 && i < delta.size(); j += 4)
+            {
+                // Swap BGR to RGB
+                *dest++ = delta[i + 2];
+                *dest++ = delta[i + 1];
+                *dest++ = delta[i];
+                *dest++ = delta[i + 3];
+                i += 4;
+            }
             break;
         }
         case 't': // termination
@@ -222,6 +233,47 @@ void DeltaTests::testRandomDeltas()
 {
 }
 
+void DeltaTests::testDeltaCopyOutOfBounds()
+{
+    constexpr auto testname = __func__;
+
+    DeltaGenerator gen;
+
+    png_uint_32 height, width, rowBytes;
+    const TileWireId textWid = 1;
+    std::vector<char> text =
+        DeltaTests::loadPng(TDOC "/delta-graphic.png",
+                            height, width, rowBytes);
+    LOK_ASSERT(height == 256 && width == 256 && rowBytes == 256*4);
+    LOK_ASSERT_EQUAL(size_t(256 * 256 * 4), text.size());
+
+    const TileWireId text2Wid = 2;
+    std::vector<char> text2 =
+        DeltaTests::loadPng(TDOC "/delta-graphic2.png",
+                            height, width, rowBytes);
+    LOK_ASSERT(height == 256 && width == 256 && rowBytes == 256*4);
+    LOK_ASSERT_EQUAL(size_t(256 * 256 * 4), text2.size());
+
+    std::vector<char> delta;
+    // Stash it in the cache
+    LOK_ASSERT(gen.createDelta(
+                       reinterpret_cast<unsigned char *>(&text[0]),
+                       0, 0, width, height, width, height,
+                       TileLocation(1, 2, 3, 0, 1), delta, textWid, false) == false);
+    LOK_ASSERT(delta.empty());
+
+    // Build a delta between the two frames
+    LOK_ASSERT(gen.createDelta(
+                       reinterpret_cast<unsigned char *>(&text2[0]),
+                       0, 0, width, height, width, height,
+                       TileLocation(1, 2, 3, 0, 1), delta, text2Wid, false) == true);
+    LOK_ASSERT(delta.size() > 0);
+
+    // Apply it to move to the second frame
+    std::vector<char> reText2 = applyDelta(text, width, height, delta, testname);
+    assertEqual(reText2, text2, width, height, testname);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(DeltaTests);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */