Fix interpretation of permissions in PDFiumPermissions.

In the PDF viewer, correctly interpret the concept for copy for
accessibility for older PDFs that were created without this concept in
mind. See bug for details.

Restructure the code to make it clearer how PDFs with security handler
revision 2 are handled vs. newer revisions. Improve comments and give a
constant a better name along the way.

Bug: 989408
Change-Id: I3f6ebbef3f371fb53ad25af814ab84a903e288e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728414
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716753}

pdf/pdfium/pdfium_permissions.cc

@@ -32,27 +32,35 @@ bool PDFiumPermissions::HasPermission(
   if (permissions_handler_revision_ < 2)
     return true;
 
-  // Handle high quality printing permission separately for security handler
-  // revision 3+. See table 3.20 in the PDF 1.7 spec.
-  if (permission == PDFEngine::PERMISSION_PRINT_HIGH_QUALITY &&
-      permissions_handler_revision_ >= 3) {
-    return (permission_bits_ & kPDFPermissionPrintMask) != 0 &&
-           (permission_bits_ & kPDFPermissionPrintHighQualityMask) != 0;
-  }
-
-  switch (permission) {
-    case PDFEngine::PERMISSION_COPY:
-      return (permission_bits_ & kPDFPermissionCopyMask) != 0;
-    case PDFEngine::PERMISSION_COPY_ACCESSIBLE:
-      return (permission_bits_ & kPDFPermissionCopyAccessibleMask) != 0;
-    case PDFEngine::PERMISSION_PRINT_LOW_QUALITY:
-    case PDFEngine::PERMISSION_PRINT_HIGH_QUALITY:
-      // With security handler revision 2 rules, check the same bit for high
-      // and low quality. See table 3.20 in the PDF 1.7 spec.
-      return (permission_bits_ & kPDFPermissionPrintMask) != 0;
-    default:
-      return true;
+  if (permissions_handler_revision_ == 2) {
+    // Security handler revision 2 rules are simple.
+    switch (permission) {
+      case PDFEngine::PERMISSION_COPY:
+      case PDFEngine::PERMISSION_COPY_ACCESSIBLE:
+        // Check the same copy bit for all copying permissions.
+        return (permission_bits_ & kPDFPermissionCopyMask) != 0;
+      case PDFEngine::PERMISSION_PRINT_LOW_QUALITY:
+      case PDFEngine::PERMISSION_PRINT_HIGH_QUALITY:
+        // Check the same printing bit for all printing permissions.
+        return (permission_bits_ & kPDFPermissionPrintMask) != 0;
+    }
+  } else {
+    // Security handler revision 3+ have different rules for interpreting the
+    // bits in |permission_bits_|.
+    switch (permission) {
+      case PDFEngine::PERMISSION_COPY:
+        return (permission_bits_ & kPDFPermissionCopyMask) != 0;
+      case PDFEngine::PERMISSION_COPY_ACCESSIBLE:
+        return (permission_bits_ & kPDFPermissionCopyAccessibleMask) != 0;
+      case PDFEngine::PERMISSION_PRINT_LOW_QUALITY:
+        return (permission_bits_ & kPDFPermissionPrintMask) != 0;
+      case PDFEngine::PERMISSION_PRINT_HIGH_QUALITY:
+        return (permission_bits_ & kPDFPermissionPrintMask) != 0 &&
+               (permission_bits_ & kPDFPermissionPrintHighQualityMask) != 0;
+    }
   }
+  NOTREACHED() << "Unknown permission " << permission;
+  return true;
 }
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_permissions_unittest.cc

@@ -66,8 +66,7 @@ TEST(PDFiumPermissionTest, Revision2SecurityHandler) {
   uint32_t permissions = GeneratePermissions2(0);
   auto no_perms = PDFiumPermissions::CreateForTesting(2, permissions);
   EXPECT_FALSE(no_perms.HasPermission(kPermCopy));
-  // TODO(crbug.com/989408) Should be the same as |PERMISSION_COPY|.
-  EXPECT_TRUE(no_perms.HasPermission(kPermCopya11y));
+  EXPECT_FALSE(no_perms.HasPermission(kPermCopya11y));
   EXPECT_FALSE(no_perms.HasPermission(kPermPrintLow));
   EXPECT_FALSE(no_perms.HasPermission(kPermPrintHigh));
 
@@ -89,8 +88,7 @@ TEST(PDFiumPermissionTest, Revision2SecurityHandler) {
   permissions = GeneratePermissions2(kPDFPermissionPrintMask);
   auto no_copy_perms = PDFiumPermissions::CreateForTesting(2, permissions);
   EXPECT_FALSE(no_copy_perms.HasPermission(kPermCopy));
-  // TODO(crbug.com/989408) Should be the same as |PERMISSION_COPY|.
-  EXPECT_TRUE(no_copy_perms.HasPermission(kPermCopya11y));
+  EXPECT_FALSE(no_copy_perms.HasPermission(kPermCopya11y));
   EXPECT_TRUE(no_copy_perms.HasPermission(kPermPrintLow));
   EXPECT_TRUE(no_copy_perms.HasPermission(kPermPrintHigh));
 }