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 <hnakashima@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Ryan Harrison <rharrison@chromium.org>
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 <hnakashima@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
diff --git a/core/fpdfdoc/cpdf_aaction.cpp b/core/fpdfdoc/cpdf_aaction.cpp
index 8d95469..9a95340 100644
--- a/core/fpdfdoc/cpdf_aaction.cpp
+++ b/core/fpdfdoc/cpdf_aaction.cpp
@@ -32,9 +32,8 @@
"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 @@
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 90049f5..11eca01 100644
--- a/core/fpdfdoc/cpdf_aaction.h
+++ b/core/fpdfdoc/cpdf_aaction.h
@@ -35,7 +35,6 @@
DocumentSaved,
PrintDocument,
DocumentPrinted,
- DocumentOpen,
NumberOfActions // Must be last.
};
@@ -47,8 +46,6 @@
CPDF_Action GetAction(AActionType eType) const;
const CPDF_Dictionary* GetDict() const { return m_pDict.Get(); }
- static bool IsUserClick(AActionType eType);
-
private:
UnownedPtr<const CPDF_Dictionary> const m_pDict;
};
diff --git a/fpdfsdk/cpdfsdk_actionhandler.cpp b/fpdfsdk/cpdfsdk_actionhandler.cpp
index f98585f..f951bfb 100644
--- a/fpdfsdk/cpdfsdk_actionhandler.cpp
+++ b/fpdfsdk/cpdfsdk_actionhandler.cpp
@@ -82,7 +82,7 @@
CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv) {
std::set<const CPDF_Dictionary*> 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_Link(
const CPDF_Action& action,
- CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv) {
std::set<const CPDF_Dictionary*> visited;
- return ExecuteLinkAction(action, type, pFormFillEnv, &visited);
+ return ExecuteLinkAction(action, pFormFillEnv, &visited);
}
bool CPDFSDK_ActionHandler::DoAction_Field(
@@ -131,8 +130,7 @@
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::ExecuteLinkAction(
const CPDF_Action& action,
- CPDF_AAction::AActionType eType,
CPDFSDK_FormFillEnvironment* pFormFillEnv,
std::set<const CPDF_Dictionary*>* visited) {
const CPDF_Dictionary* pDict = action.GetDict();
@@ -162,12 +159,12 @@
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 @@
RunDocumentPageJavaScript(pFormFillEnv, type, swJS);
}
} else {
- DoAction_NoJs(action, type, pFormFillEnv);
+ DoAction_NoJs(action, pFormFillEnv);
}
ASSERT(pFormFillEnv);
@@ -241,7 +238,7 @@
}
}
} 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 @@
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::ExecuteBookMark(
const CPDF_Action& action,
- CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv,
CPDF_Bookmark* pBookmark,
std::set<const CPDF_Dictionary*>* visited) {
@@ -300,12 +296,12 @@
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 @@
void CPDFSDK_ActionHandler::DoAction_NoJs(
const CPDF_Action& action,
- CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv) {
ASSERT(pFormFillEnv);
@@ -323,8 +318,7 @@
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 @@
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 94b347f..14cbc9c 100644
--- a/fpdfsdk/cpdfsdk_actionhandler.h
+++ b/fpdfsdk/cpdfsdk_actionhandler.h
@@ -44,7 +44,6 @@
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 @@
CPDFSDK_Annot* pScreen,
std::set<const CPDF_Dictionary*>* visited);
bool ExecuteBookMark(const CPDF_Action& action,
- CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv,
CPDF_Bookmark* pBookmark,
std::set<const CPDF_Dictionary*>* visited);
bool ExecuteLinkAction(const CPDF_Action& action,
- CPDF_AAction::AActionType type,
CPDFSDK_FormFillEnvironment* pFormFillEnv,
std::set<const CPDF_Dictionary*>* 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 82776e3..8bd1f10 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -174,7 +174,6 @@
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 3e53753..8ff3a84 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -379,25 +379,6 @@
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 43f077c..215009c 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -241,8 +241,6 @@
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;
@@ -480,13 +478,6 @@
}
// static
-void EmbedderTest::DoURIActionTrampoline(FPDF_FORMFILLINFO* info,
- FPDF_BYTESTRING uri) {
- EmbedderTest* test = static_cast<EmbedderTest*>(info);
- return test->delegate_->DoURIAction(uri);
-}
-
-// static
std::string EmbedderTest::HashBitmap(FPDF_BITMAP bitmap) {
uint8_t digest[16];
CRYPT_MD5Generate(static_cast<uint8_t*>(FPDFBitmap_GetBuffer(bitmap)),
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index a6a4cf5..6470763 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -62,9 +62,6 @@
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 @@
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 2dcc23b..0000000
--- 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
