Better test FPDFAction_* public functions.

Update public documentation.
Make implementation match documentation for bad argument types.

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

3 files changed, 44 insertions, 16 deletions

diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index 614462fa47..28c670c5dd 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -164,11 +164,14 @@ FPDF_EXPORT unsigned long FPDF_CALLCONV FPDFAction_GetType(FPDF_ACTION pDict) {
 
 FPDF_EXPORT FPDF_DEST FPDF_CALLCONV FPDFAction_GetDest(FPDF_DOCUMENT document,
                                                        FPDF_ACTION pDict) {
-  if (!pDict)
-    return nullptr;
   CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pDoc)
     return nullptr;
+
+  unsigned long type = FPDFAction_GetType(pDict);
+  if (type != PDFACTION_GOTO && type != PDFACTION_REMOTEGOTO)
+    return nullptr;
+
   CPDF_Action action(CPDFDictionaryFromFPDFAction(pDict));
   return FPDFDestFromCPDFArray(action.GetDest(pDoc).GetArray());
 }
@@ -192,11 +195,14 @@ FPDFAction_GetURIPath(FPDF_DOCUMENT document,
                       FPDF_ACTION pDict,
                       void* buffer,
                       unsigned long buflen) {
-  if (!pDict)
-    return 0;
   CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pDoc)
     return 0;
+
+  unsigned long type = FPDFAction_GetType(pDict);
+  if (type != PDFACTION_URI)
+    return 0;
+
   CPDF_Action action(CPDFDictionaryFromFPDFAction(pDict));
   ByteString path = action.GetURI(pDoc);
   unsigned long len = path.GetLength() + 1;
diff --git a/fpdfsdk/fpdf_doc_embeddertest.cpp b/fpdfsdk/fpdf_doc_embeddertest.cpp
index 3d582bcca0..91a2333241 100644
--- a/fpdfsdk/fpdf_doc_embeddertest.cpp
+++ b/fpdfsdk/fpdf_doc_embeddertest.cpp
@@ -225,7 +225,19 @@ TEST_F(FPDFDocEmbeddertest, BUG_821454) {
   UnloadPage(page);
 }
 
-TEST_F(FPDFDocEmbeddertest, ActionGetFilePath) {
+TEST_F(FPDFDocEmbeddertest, ActionBadArguments) {
+  EXPECT_TRUE(OpenDocument("launch_action.pdf"));
+  EXPECT_EQ(static_cast<unsigned long>(PDFACTION_UNSUPPORTED),
+            FPDFAction_GetType(nullptr));
+
+  EXPECT_EQ(nullptr, FPDFAction_GetDest(nullptr, nullptr));
+  EXPECT_EQ(nullptr, FPDFAction_GetDest(document(), nullptr));
+  EXPECT_EQ(0u, FPDFAction_GetFilePath(nullptr, nullptr, 0));
+  EXPECT_EQ(0u, FPDFAction_GetURIPath(nullptr, nullptr, nullptr, 0));
+  EXPECT_EQ(0u, FPDFAction_GetURIPath(document(), nullptr, nullptr, 0));
+}
+
+TEST_F(FPDFDocEmbeddertest, ActionLaunch) {
   EXPECT_TRUE(OpenDocument("launch_action.pdf"));
 
   FPDF_PAGE page = LoadPage(0);
@@ -237,15 +249,21 @@ TEST_F(FPDFDocEmbeddertest, ActionGetFilePath) {
 
   FPDF_ACTION action = FPDFLink_GetAction(link);
   ASSERT_TRUE(action);
+  EXPECT_EQ(static_cast<unsigned long>(PDFACTION_LAUNCH),
+            FPDFAction_GetType(action));
 
   const char kExpectedResult[] = "test.pdf";
   const unsigned long kExpectedLength = sizeof(kExpectedResult);
   unsigned long bufsize = FPDFAction_GetFilePath(action, nullptr, 0);
-  ASSERT_EQ(kExpectedLength, bufsize);
+  EXPECT_EQ(kExpectedLength, bufsize);
 
   char buf[kExpectedLength];
   EXPECT_EQ(bufsize, FPDFAction_GetFilePath(action, buf, bufsize));
-  EXPECT_EQ(std::string(kExpectedResult), std::string(buf));
+  EXPECT_STREQ(kExpectedResult, buf);
+
+  // Other public methods are not appropriate for this action type.
+  EXPECT_EQ(nullptr, FPDFAction_GetDest(document(), action));
+  EXPECT_EQ(0u, FPDFAction_GetURIPath(document(), action, buf, bufsize));
 
   UnloadPage(page);
 }
diff --git a/public/fpdf_doc.h b/public/fpdf_doc.h
index 4bd9d44169..f5a270a24e 100644
--- a/public/fpdf_doc.h
+++ b/public/fpdf_doc.h
@@ -136,23 +136,25 @@ FPDF_EXPORT unsigned long FPDF_CALLCONV FPDFAction_GetType(FPDF_ACTION action);
 //   action   - handle to the action. |action| must be a |PDFACTION_GOTO| or
 //              |PDFACTION_REMOTEGOTO|.
 //
-// Returns a handle to the destination data.
+// Returns a handle to the destination data, or NULL on error, typically
+// because the arguments were bad or the action was of the wrong type.
 //
-// In the case of |PDFACTION_REMOTEGOTO|, you should first call
-// FPDFAction_GetFilePath() then load that document, the document handle from
-// that document should pass as |document| to FPDFAction_GetDest().
+// In the case of |PDFACTION_REMOTEGOTO|, you must first call
+// FPDFAction_GetFilePath(), then load the document at that path, then pass
+// the document handle from that document as |document| to FPDFAction_GetDest().
 FPDF_EXPORT FPDF_DEST FPDF_CALLCONV FPDFAction_GetDest(FPDF_DOCUMENT document,
                                                        FPDF_ACTION action);
 
-// Get file path of a |PDFACTION_REMOTEGOTO| |action|.
+// Get the file path of |action|.
 //
 //   action - handle to the action. |action| must be a |PDFACTION_LAUNCH| or
-//            |PDFACTION_REMOTEGOTO|
+//            |PDFACTION_REMOTEGOTO|.
 //   buffer - a buffer for output the path string. May be NULL.
 //   buflen - the length of the buffer, in bytes. May be 0.
 //
 // Returns the number of bytes in the file path, including the trailing NUL
-// character.
+// character, or 0 on error, typically because the arguments were bad or the
+// action was of the wrong type.
 //
 // Regardless of the platform, the |buffer| is always in UTF-8 encoding.
 // If |buflen| is less than the returned length, or |buffer| is NULL, |buffer|
@@ -160,14 +162,16 @@ FPDF_EXPORT FPDF_DEST FPDF_CALLCONV FPDFAction_GetDest(FPDF_DOCUMENT document,
 FPDF_EXPORT unsigned long FPDF_CALLCONV
 FPDFAction_GetFilePath(FPDF_ACTION action, void* buffer, unsigned long buflen);
 
-// Get the URI path of a |PDFACTION_URI| |action|.
+// Get the URI path of |action|.
 //
 //   document - handle to the document.
 //   action   - handle to the action. Must be a |PDFACTION_URI|.
 //   buffer   - a buffer for the path string. May be NULL.
 //   buflen   - the length of the buffer, in bytes. May be 0.
 //
-// Returns the number of bytes in the URI path, including trailing zeros.
+// Returns the number of bytes in the URI path, including the trailing NUL
+// character, or 0 on error, typically because the arguments were bad or the
+// action was of the wrong type.
 //
 // The |buffer| is always encoded in 7-bit ASCII. If |buflen| is less than the
 // returned length, or |buffer| is NULL, |buffer| will not be modified.