Home Explore Help
Register Sign In
steve
/
ladybird
mirror of https://github.com/LadybirdBrowser/ladybird
1
0
Fork 0
Files Issues 0 Wiki
Browse Source

LibDiff: Fix wrong index used when prepending context lines

`i` is used as the index for 'old lines' in diff generation, not 'new
lines'. Using the wrong index would mean that for certain diffs the
prefixed context information would have wrong content, and could even
result in a crash.

Fix this, and add a test for an input which was previously crashing.
Shannon Booth 1 year ago
parent
4adbf1d041
commit
b738929195
4 changed files with 117 additions and 1 deletions
Split View Show Diff Stats
  1. 1 0
      Tests/CMakeLists.txt
  2. 7 0
      Tests/LibDiff/CMakeLists.txt
  3. 108 0
      Tests/LibDiff/TestDiff.cpp
  4. 1 1
      Userland/Libraries/LibDiff/Generator.cpp

+ 1 - 0
Tests/CMakeLists.txt
View File 

@@ -5,6 +5,7 @@ add_subdirectory(LibC)
 
 add_subdirectory(LibCompress)
 
 add_subdirectory(LibCore)
 
 add_subdirectory(LibCpp)
 
+add_subdirectory(LibDiff)
 
 add_subdirectory(LibEDID)
 
 add_subdirectory(LibELF)
 
 add_subdirectory(LibGfx)

+ 7 - 0
Tests/LibDiff/CMakeLists.txt
View File 

@@ -0,0 +1,7 @@
 
+set(TEST_SOURCES
 
+    TestDiff.cpp
 
+)
 
+
 
+foreach(source IN LISTS TEST_SOURCES)
 
+    serenity_test("${source}" LibDiff LIBS LibDiff)
 
+endforeach()

+ 108 - 0
Tests/LibDiff/TestDiff.cpp
View File 

@@ -0,0 +1,108 @@
 
+/*
 
+ * Copyright (c) 2023, Shannon Booth <shannon@serenityos.org>
 
+ *
 
+ * SPDX-License-Identifier: BSD-2-Clause
 
+ */
 
+
 
+#include <AK/MemoryStream.h>
 
+#include <LibDiff/Format.h>
 
+#include <LibDiff/Generator.h>
 
+#include <LibTest/TestCase.h>
 
+
 
+TEST_CASE(test_generate_unified_diff)
 
+{
 
+    StringView old_text = R"(Viewport <#document> at (0,0) content-size 800x600 children: not-inline
 
+  BlockContainer <html> at (0,0) content-size 800x600 [BFC] children: not-inline
 
+    BlockContainer <body> at (8,8) content-size 784x150 children: not-inline
 
+      BlockContainer <(anonymous)> at (8,8) content-size 784x0 children: inline
 
+        TextNode <#text>
 
+        TextNode <#text>
 
+      BlockContainer <div> at (8,8) content-size 784x150 children: inline
 
+        line 0 width: 300, height: 150, bottom: 150, baseline: 150
 
+          frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 300x150]
 
+        TextNode <#text>
 
+        SVGSVGBox <svg> at (8,8) content-size 300x150 [SVG] children: inline
 
+          TextNode <#text>
 
+          Box <use> at (8,8) content-size 0x0 children: inline
 
+            Box <symbol#braces> at (8,8) content-size 0x0 children: inline
 
+              TextNode <#text>
 
+              SVGGeometryBox <path> at (92.375,26.75) content-size 131.25x112.15625 children: inline
 
+                TextNode <#text>
 
+              TextNode <#text>
 
+          TextNode <#text>
 
+        TextNode <#text>
 
+
 
+ViewportPaintable (Viewport<#document>) [0,0 800x600]
 
+  PaintableWithLines (BlockContainer<HTML>) [0,0 800x600]
 
+    PaintableWithLines (BlockContainer<BODY>) [8,8 784x150]
 
+      PaintableWithLines (BlockContainer(anonymous)) [8,8 784x0]
 
+      PaintableWithLines (BlockContainer<DIV>) [8,8 784x150]
 
+        SVGSVGPaintable (SVGSVGBox<svg>) [8,8 300x150]
 
+          PaintableBox (Box<use>) [8,8 0x0]
 
+            PaintableBox (Box<symbol>#braces) [8,8 0x0]
 
+              SVGGeometryPaintable (SVGGeometryBox<path>) [92.375,26.75 131.25x112.15625]
 
+
 
+)"sv;
 
+
 
+    StringView new_text = R"(Viewport <#document> at (0,0) content-size 800x600 children: not-inline
 
+  BlockContainer <html> at (0,0) content-size 800x600 [BFC] children: not-inline
 
+    BlockContainer <body> at (8,8) content-size 784x150 children: not-inline
 
+      BlockContainer <(anonymous)> at (8,8) content-size 784x0 children: inline
 
+        TextNode <#text>
 
+        TextNode <#text>
 
+      BlockContainer <div> at (8,8) content-size 784x150 children: inline
 
+        line 0 width: 300, height: 150, bottom: 150, baseline: 150
 
+          frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 300x150]
 
+        TextNode <#text>
 
+        SVGSVGBox <svg> at (8,8) content-size 300x150 [SVG] children: inline
 
+          TextNode <#text>
 
+          Box <use> at (8,8) content-size 0x0 children: not-inline
 
+          TextNode <#text>
 
+        TextNode <#text>
 
+
 
+ViewportPaintable (Viewport<#document>) [0,0 800x600]
 
+  PaintableWithLines (BlockContainer<HTML>) [0,0 800x600]
 
+    PaintableWithLines (BlockContainer<BODY>) [8,8 784x150]
 
+      PaintableWithLines (BlockContainer(anonymous)) [8,8 784x0]
 
+      PaintableWithLines (BlockContainer<DIV>) [8,8 784x150]
 
+        SVGSVGPaintable (SVGSVGBox<svg>) [8,8 300x150]
 
+          PaintableBox (Box<use>) [8,8 0x0]
 
+
 
+)"sv;
 
+
 
+    auto result = MUST(Diff::from_text(old_text, new_text, 3));
 
+    EXPECT_EQ(result.size(), 2U);
 
+
 
+    auto hunk1_stream = make<AllocatingMemoryStream>();
 
+    MUST(Diff::write_unified(result[0], *hunk1_stream));
 
+
 
+    auto hunk1 = MUST(hunk1_stream->read_until_eof());
 
+    EXPECT_EQ(StringView { hunk1 }, R"(@@ -10,12 +10,7 @@
 
+         TextNode <#text>
 
+         SVGSVGBox <svg> at (8,8) content-size 300x150 [SVG] children: inline
 
+           TextNode <#text>
 
+-          Box <use> at (8,8) content-size 0x0 children: inline
 
+-            Box <symbol#braces> at (8,8) content-size 0x0 children: inline
 
+-              TextNode <#text>
 
+-              SVGGeometryBox <path> at (92.375,26.75) content-size 131.25x112.15625 children: inline
 
+-                TextNode <#text>
 
+-              TextNode <#text>
 
++          Box <use> at (8,8) content-size 0x0 children: not-inline
 
+           TextNode <#text>
 
+         TextNode <#text>
 
+ 
+)"sv);
 
+
 
+    auto hunk2_stream = make<AllocatingMemoryStream>();
 
+    MUST(Diff::write_unified(result[1], *hunk2_stream));
 
+
 
+    auto hunk2 = MUST(hunk2_stream->read_until_eof());
 
+    EXPECT_EQ(StringView { hunk2 }, R"(@@ -26,6 +21,4 @@
 
+       PaintableWithLines (BlockContainer<DIV>) [8,8 784x150]
 
+         SVGSVGPaintable (SVGSVGBox<svg>) [8,8 300x150]
 
+           PaintableBox (Box<use>) [8,8 0x0]
 
+-            PaintableBox (Box<symbol>#braces) [8,8 0x0]
 
+-              SVGGeometryPaintable (SVGGeometryBox<path>) [92.375,26.75 131.25x112.15625]
 
+ 
+)"sv);
 
+}

+ 1 - 1
Userland/Libraries/LibDiff/Generator.cpp
View File 

@@ -90,7 +90,7 @@ ErrorOr<Vector<Hunk>> from_text(StringView old_text, StringView new_text, size_t
 
 
         for (size_t offset = 0; offset < available_context; ++offset) {
 
             size_t context_line = i + offset - available_context;
 
-            TRY(hunk.lines.try_append(Line { Line::Operation::Context, TRY(String::from_utf8(new_lines[context_line])) }));
 
+            TRY(hunk.lines.try_append(Line { Line::Operation::Context, TRY(String::from_utf8(old_lines[context_line])) }));
 
         }
 
 
         return {};