From 97f4483de007c2ff248696f24d34634e0adbf894 Mon Sep 17 00:00:00 2001 From: Henrique Nakashima Date: Mon, 24 Sep 2018 17:23:27 +0000 Subject: Revert "Make potentially dangerous Actions require a user click." This reverts commit 9d784c291714b703b16185e69860a3797de85b6c. Reason for revert: Roll into chromium is stuck, this is a potential culprit. Original change's description: > Make potentially dangerous Actions require a user click. > > URI and SubmitForm actions are only handled if the event was > ButtonUp or ButtonDown. > > Bug: 851821 > Change-Id: If6eb0ff44f6d62ac6df50b552c0bdc582885ab5d > Reviewed-on: https://pdfium-review.googlesource.com/42731 > Commit-Queue: Henrique Nakashima > Reviewed-by: Tom Sepez > Reviewed-by: Ryan Harrison TBR=tsepez@chromium.org,hnakashima@chromium.org,rharrison@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 851821 Change-Id: I6f1bc0a02f65a24fbd49d53526b985f8a4ea0b4f Reviewed-on: https://pdfium-review.googlesource.com/42990 Reviewed-by: Henrique Nakashima Commit-Queue: Henrique Nakashima --- core/fpdfdoc/cpdf_aaction.cpp | 16 ++-------------- core/fpdfdoc/cpdf_aaction.h | 3 --- fpdfsdk/cpdfsdk_actionhandler.cpp | 31 ++++++++++++------------------- fpdfsdk/cpdfsdk_actionhandler.h | 4 ---- fpdfsdk/cpdfsdk_widget.cpp | 1 - fpdfsdk/fpdf_formfill_embeddertest.cpp | 19 ------------------- testing/embedder_test.cpp | 9 --------- testing/embedder_test.h | 5 ----- testing/resources/redirect.pdf | 22 ---------------------- 9 files changed, 14 insertions(+), 96 deletions(-) delete mode 100644 testing/resources/redirect.pdf diff --git a/core/fpdfdoc/cpdf_aaction.cpp b/core/fpdfdoc/cpdf_aaction.cpp index 8d954697b4..9a95340114 100644 --- a/core/fpdfdoc/cpdf_aaction.cpp +++ b/core/fpdfdoc/cpdf_aaction.cpp @@ -32,9 +32,8 @@ constexpr const char* g_sAATypes[] = { "DP", // DocumentPrinted }; -// |g_sAATypes| should have one less element than enum AActionType due to -// DocumentOpen, which is an artificial type. -static_assert(FX_ArraySize(g_sAATypes) == CPDF_AAction::NumberOfActions - 1, +// |g_sAATypes| should have as many elements as enum AActionType. +static_assert(FX_ArraySize(g_sAATypes) == CPDF_AAction::NumberOfActions, "g_sAATypes count mismatch"); } // namespace @@ -53,14 +52,3 @@ CPDF_Action CPDF_AAction::GetAction(AActionType eType) const { return CPDF_Action(m_pDict ? m_pDict->GetDictFor(g_sAATypes[eType]) : nullptr); } - -// static -bool CPDF_AAction::IsUserClick(AActionType eType) { - switch (eType) { - case ButtonUp: - case ButtonDown: - return true; - default: - return false; - } -} diff --git a/core/fpdfdoc/cpdf_aaction.h b/core/fpdfdoc/cpdf_aaction.h index 90049f5ac4..11eca01f8f 100644 --- a/core/fpdfdoc/cpdf_aaction.h +++ b/core/fpdfdoc/cpdf_aaction.h @@ -35,7 +35,6 @@ class CPDF_AAction { DocumentSaved, PrintDocument, DocumentPrinted, - DocumentOpen, NumberOfActions // Must be last. }; @@ -47,8 +46,6 @@ class CPDF_AAction { CPDF_Action GetAction(AActionType eType) const; const CPDF_Dictionary* GetDict() const { return m_pDict.Get(); } - static bool IsUserClick(AActionType eType); - private: UnownedPtr const m_pDict; }; diff --git a/fpdfsdk/cpdfsdk_actionhandler.cpp b/fpdfsdk/cpdfsdk_actionhandler.cpp index f98585f2a1..f951bfb9fe 100644 --- a/fpdfsdk/cpdfsdk_actionhandler.cpp +++ b/fpdfsdk/cpdfsdk_actionhandler.cpp @@ -82,7 +82,7 @@ bool CPDFSDK_ActionHandler::DoAction_BookMark( CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv) { std::set visited; - return ExecuteBookMark(action, type, pFormFillEnv, pBookMark, &visited); + return ExecuteBookMark(action, pFormFillEnv, pBookMark, &visited); } bool CPDFSDK_ActionHandler::DoAction_Screen( @@ -96,10 +96,9 @@ bool CPDFSDK_ActionHandler::DoAction_Screen( bool CPDFSDK_ActionHandler::DoAction_Link( const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv) { std::set visited; - return ExecuteLinkAction(action, type, pFormFillEnv, &visited); + return ExecuteLinkAction(action, pFormFillEnv, &visited); } bool CPDFSDK_ActionHandler::DoAction_Field( @@ -131,8 +130,7 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentOpenAction( RunDocumentOpenJavaScript(pFormFillEnv, L"", swJS); } } else { - DoAction_NoJs(action, CPDF_AAction::AActionType::DocumentOpen, - pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); } for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) { @@ -146,7 +144,6 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentOpenAction( bool CPDFSDK_ActionHandler::ExecuteLinkAction( const CPDF_Action& action, - CPDF_AAction::AActionType eType, CPDFSDK_FormFillEnvironment* pFormFillEnv, std::set* visited) { const CPDF_Dictionary* pDict = action.GetDict(); @@ -162,12 +159,12 @@ bool CPDFSDK_ActionHandler::ExecuteLinkAction( context->OnLink_MouseUp(pFormFillEnv); }); } else { - DoAction_NoJs(action, eType, pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); } for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) { CPDF_Action subaction = action.GetSubAction(i); - if (!ExecuteLinkAction(subaction, eType, pFormFillEnv, visited)) + if (!ExecuteLinkAction(subaction, pFormFillEnv, visited)) return false; } @@ -193,7 +190,7 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentPageAction( RunDocumentPageJavaScript(pFormFillEnv, type, swJS); } } else { - DoAction_NoJs(action, type, pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); } ASSERT(pFormFillEnv); @@ -241,7 +238,7 @@ bool CPDFSDK_ActionHandler::ExecuteFieldAction( } } } else { - DoAction_NoJs(action, type, pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); } for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) { @@ -270,7 +267,7 @@ bool CPDFSDK_ActionHandler::ExecuteScreenAction( if (action.GetType() == CPDF_Action::JavaScript) RunScriptForAction(action, pFormFillEnv, [](IJS_EventContext*) {}); else - DoAction_NoJs(action, type, pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) { CPDF_Action subaction = action.GetSubAction(i); @@ -283,7 +280,6 @@ bool CPDFSDK_ActionHandler::ExecuteScreenAction( bool CPDFSDK_ActionHandler::ExecuteBookMark( const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv, CPDF_Bookmark* pBookmark, std::set* visited) { @@ -300,12 +296,12 @@ bool CPDFSDK_ActionHandler::ExecuteBookMark( context->OnBookmark_MouseUp(pBookmark); }); } else { - DoAction_NoJs(action, type, pFormFillEnv); + DoAction_NoJs(action, pFormFillEnv); } for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) { CPDF_Action subaction = action.GetSubAction(i); - if (!ExecuteBookMark(subaction, type, pFormFillEnv, pBookmark, visited)) + if (!ExecuteBookMark(subaction, pFormFillEnv, pBookmark, visited)) return false; } @@ -314,7 +310,6 @@ bool CPDFSDK_ActionHandler::ExecuteBookMark( void CPDFSDK_ActionHandler::DoAction_NoJs( const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv) { ASSERT(pFormFillEnv); @@ -323,8 +318,7 @@ void CPDFSDK_ActionHandler::DoAction_NoJs( DoAction_GoTo(pFormFillEnv, action); break; case CPDF_Action::URI: - if (CPDF_AAction::IsUserClick(type)) - DoAction_URI(pFormFillEnv, action); + DoAction_URI(pFormFillEnv, action); break; case CPDF_Action::Hide: DoAction_Hide(action, pFormFillEnv); @@ -333,8 +327,7 @@ void CPDFSDK_ActionHandler::DoAction_NoJs( DoAction_Named(pFormFillEnv, action); break; case CPDF_Action::SubmitForm: - if (CPDF_AAction::IsUserClick(type)) - DoAction_SubmitForm(action, pFormFillEnv); + DoAction_SubmitForm(action, pFormFillEnv); break; case CPDF_Action::ResetForm: DoAction_ResetForm(action, pFormFillEnv); diff --git a/fpdfsdk/cpdfsdk_actionhandler.h b/fpdfsdk/cpdfsdk_actionhandler.h index 94b347f630..14cbc9c5c5 100644 --- a/fpdfsdk/cpdfsdk_actionhandler.h +++ b/fpdfsdk/cpdfsdk_actionhandler.h @@ -44,7 +44,6 @@ class CPDFSDK_ActionHandler { CPDFSDK_FormFillEnvironment* pFormFillEnv, CPDFSDK_Annot* pScreen); bool DoAction_Link(const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv); bool DoAction_Field(const CPDF_Action& action, CPDF_AAction::AActionType type, @@ -86,17 +85,14 @@ class CPDFSDK_ActionHandler { CPDFSDK_Annot* pScreen, std::set* visited); bool ExecuteBookMark(const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv, CPDF_Bookmark* pBookmark, std::set* visited); bool ExecuteLinkAction(const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv, std::set* visited); void DoAction_NoJs(const CPDF_Action& action, - CPDF_AAction::AActionType type, CPDFSDK_FormFillEnvironment* pFormFillEnv); void RunDocumentPageJavaScript(CPDFSDK_FormFillEnvironment* pFormFillEnv, CPDF_AAction::AActionType type, diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp index 82776e3298..8bd1f10177 100644 --- a/fpdfsdk/cpdfsdk_widget.cpp +++ b/fpdfsdk/cpdfsdk_widget.cpp @@ -174,7 +174,6 @@ static XFA_EVENTTYPE GetXFAEventType(CPDF_AAction::AActionType eAAT, case CPDF_AAction::PrintDocument: case CPDF_AAction::DocumentPrinted: break; - case CPDF_AAction::DocumentOpen: case CPDF_AAction::NumberOfActions: NOTREACHED(); break; diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp index 3e53753b48..8ff3a84291 100644 --- a/fpdfsdk/fpdf_formfill_embeddertest.cpp +++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp @@ -379,25 +379,6 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_514690) { UnloadPage(page); } -class DoURIActionBlockedDelegate final : public EmbedderTest::Delegate { - public: - void DoURIAction(FPDF_BYTESTRING uri) override { - FAIL() << "Navigated to " << uri; - } -}; - -TEST_F(FPDFFormFillEmbeddertest, BUG_851821) { - DoURIActionBlockedDelegate delegate; - SetDelegate(&delegate); - - EXPECT_TRUE(OpenDocument("redirect.pdf")); - FPDF_PAGE page = LoadPage(0); - EXPECT_TRUE(page); - DoOpenActions(); - - UnloadPage(page); -} - #ifdef PDF_ENABLE_V8 TEST_F(FPDFFormFillEmbeddertest, DisableJavaScript) { // Test that timers and intervals can't fire without JS. diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp index 43f077ca01..215009ceac 100644 --- a/testing/embedder_test.cpp +++ b/testing/embedder_test.cpp @@ -241,8 +241,6 @@ FPDF_FORMHANDLE EmbedderTest::SetupFormFillEnvironment( formfillinfo->FFI_SetTimer = SetTimerTrampoline; formfillinfo->FFI_KillTimer = KillTimerTrampoline; formfillinfo->FFI_GetPage = GetPageTrampoline; - formfillinfo->FFI_DoURIAction = DoURIActionTrampoline; - if (javascript_option == JavaScriptOption::kEnableJavaScript) formfillinfo->m_pJsPlatform = platform; @@ -479,13 +477,6 @@ FPDF_PAGE EmbedderTest::GetPageTrampoline(FPDF_FORMFILLINFO* info, page_index); } -// static -void EmbedderTest::DoURIActionTrampoline(FPDF_FORMFILLINFO* info, - FPDF_BYTESTRING uri) { - EmbedderTest* test = static_cast(info); - return test->delegate_->DoURIAction(uri); -} - // static std::string EmbedderTest::HashBitmap(FPDF_BITMAP bitmap) { uint8_t digest[16]; diff --git a/testing/embedder_test.h b/testing/embedder_test.h index a6a4cf51e4..64707631b5 100644 --- a/testing/embedder_test.h +++ b/testing/embedder_test.h @@ -62,9 +62,6 @@ class EmbedderTest : public ::testing::Test, virtual FPDF_PAGE GetPage(FPDF_FORMFILLINFO* info, FPDF_DOCUMENT document, int page_index); - - // Equivalent to FPDF_FORMFILLINFO::FFI_DoURIAction(). - virtual void DoURIAction(FPDF_BYTESTRING uri) {} }; EmbedderTest(); @@ -248,8 +245,6 @@ class EmbedderTest : public ::testing::Test, static FPDF_PAGE GetPageTrampoline(FPDF_FORMFILLINFO* info, FPDF_DOCUMENT document, int page_index); - static void DoURIActionTrampoline(FPDF_FORMFILLINFO* info, - FPDF_BYTESTRING uri); static int WriteBlockCallback(FPDF_FILEWRITE* pFileWrite, const void* data, unsigned long size); diff --git a/testing/resources/redirect.pdf b/testing/resources/redirect.pdf deleted file mode 100644 index 2dcc23b0be..0000000000 --- a/testing/resources/redirect.pdf +++ /dev/null @@ -1,22 +0,0 @@ -%PDF-1.7 -trailer -<< -/Root 1 0 R ->> -1 0 obj -<< -/Type /Catalog -/Pages 2 0 R -/OpenAction 2 0 R ->> -endobj - -2 0 obj -<< -/Type /Action -/S /URI -/URI (http://evilzone.org) // URL HERE ->> -endobj - -%%EOF \ No newline at end of file -- cgit v1.2.3