From 7bf816420aa79b23c37ad433183f01dc5af653fa Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 10 Oct 2018 17:27:33 +0000 Subject: 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 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- fpdfsdk/fpdf_doc.cpp | 14 ++++++++++---- fpdfsdk/fpdf_doc_embeddertest.cpp | 24 +++++++++++++++++++++--- public/fpdf_doc.h | 22 +++++++++++++--------- 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(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(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. -- cgit v1.2.3