commit 252eda1bd814891dd0f1ea5f1a28cb011faf347d
author Lei Zhang <thestig@chromium.org> Sat May 09 03:44:54 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Sat May 09 03:44:54 2020 +0000
tree e38c273ce73d6c29a3108271024768e2bbe9791b
parent 64c59fdef601ce985ce7597e82c5a8015d500572

Fix FPDF_GetNamedDest() for old style destinations.

Bug: chromium:1080663
Change-Id: I830fa071ca840b88411c073cb52a67699a3b7286
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/69755
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

fpdfsdk/fpdf_view.cpp

diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 4ae33f6..e838e49 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1105,24 +1105,23 @@
   if (!pRoot)
     return nullptr;
 
+  auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
+  size_t name_tree_count = name_tree ? name_tree->GetCount() : 0;
   CPDF_Object* pDestObj = nullptr;
   WideString wsName;
-  auto name_tree = CPDF_NameTree::Create(pDoc, "Dests");
-  if (!name_tree)
-    return nullptr;
-
-  size_t count = name_tree->GetCount();
-  if (static_cast<size_t>(index) >= count) {
+  if (static_cast<size_t>(index) >= name_tree_count) {
+    // If |index| is out of bounds, then try to retrieve the Nth old style named
+    // destination. Where N is 0-indexed, with N = index - name_tree_count.
     const CPDF_Dictionary* pDest = pRoot->GetDictFor("Dests");
     if (!pDest)
       return nullptr;
 
-    FX_SAFE_INT32 checked_count = count;
+    FX_SAFE_INT32 checked_count = name_tree_count;
     checked_count += pDest->size();
     if (!checked_count.IsValid() || index >= checked_count.ValueOrDie())
       return nullptr;
 
-    index -= count;
+    index -= name_tree_count;
     int i = 0;
     ByteStringView bsName;
     CPDF_DictionaryLocker locker(pDest);

fpdfsdk/fpdf_view_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 17c2d96..2fe6e39 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -16,6 +16,7 @@
 #include "public/fpdfview.h"
 #include "testing/embedder_test.h"
 #include "testing/embedder_test_constants.h"
+#include "testing/fx_string_testhelpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/utils/file_util.h"
 #include "testing/utils/path_service.h"
@@ -688,6 +689,9 @@
 }
 
 TEST_F(FPDFViewEmbedderTest, NamedDestsOldStyle) {
+  static constexpr char kFirstAlternate[] = "FirstAlternate";
+  static constexpr char kLastAlternate[] = "LastAlternate";
+
   EXPECT_TRUE(OpenDocument("named_dests_old_style.pdf"));
   EXPECT_EQ(2u, FPDF_CountNamedDests(document()));
 
@@ -697,8 +701,8 @@
   EXPECT_FALSE(FPDF_GetNamedDestByName(document(), "NoSuchName"));
 
   // These should return a valid destination.
-  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), "FirstAlternate"));
-  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), "LastAlternate"));
+  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), kFirstAlternate));
+  EXPECT_TRUE(FPDF_GetNamedDestByName(document(), kLastAlternate));
 
   char buffer[512];
   constexpr long kBufferSize = sizeof(buffer);
@@ -711,13 +715,17 @@
   EXPECT_FALSE(FPDF_GetNamedDest(document(), 2, buffer, &size));
   EXPECT_EQ(kBufferSize, size);
 
-  // TODO(crbug.com/1080663): These should return a valid destination.
+  // These should return a valid destination.
   size = kBufferSize;
-  EXPECT_FALSE(FPDF_GetNamedDest(document(), 0, buffer, &size));
-  EXPECT_EQ(kBufferSize, size);
+  ASSERT_TRUE(FPDF_GetNamedDest(document(), 0, buffer, &size));
+  ASSERT_EQ(static_cast<int>(sizeof(kFirstAlternate) * 2), size);
+  EXPECT_EQ(kFirstAlternate,
+            GetPlatformString(reinterpret_cast<FPDF_WIDESTRING>(buffer)));
   size = kBufferSize;
-  EXPECT_FALSE(FPDF_GetNamedDest(document(), 1, buffer, &size));
-  EXPECT_EQ(kBufferSize, size);
+  ASSERT_TRUE(FPDF_GetNamedDest(document(), 1, buffer, &size));
+  ASSERT_EQ(static_cast<int>(sizeof(kLastAlternate) * 2), size);
+  EXPECT_EQ(kLastAlternate,
+            GetPlatformString(reinterpret_cast<FPDF_WIDESTRING>(buffer)));
 }
 
 // The following tests pass if the document opens without crashing.