From b1f9205bb1a0671c31e44e7362784c770bf2a948 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 14 Sep 2017 17:03:12 -0700 Subject: OnBeforeKeystroke may invalidate the widget's window. And along with it any data that the window may have been carrying. Tidy some return codes while we're at it. Bug: 765384 Change-Id: Id16ec5f82b3d9273ba8f2edb1e4645a9145df4f6 Reviewed-on: https://pdfium-review.googlesource.com/14050 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- fpdfsdk/formfiller/cffl_interactiveformfiller.cpp | 79 +++++------ fpdfsdk/fpdfformfill_embeddertest.cpp | 11 ++ testing/resources/bug_765384.in | 105 ++++++++++++++ testing/resources/bug_765384.pdf | 159 ++++++++++++++++++++++ 4 files changed, 315 insertions(+), 39 deletions(-) create mode 100644 testing/resources/bug_765384.in create mode 100644 testing/resources/bug_765384.pdf diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp index eabb899c27..10cefd204b 100644 --- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp +++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp @@ -863,33 +863,33 @@ std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( int nSelEnd, bool bKeyDown, uint32_t nFlag) { - bool bRC = true; - bool bExit = false; - CFFL_PrivateData* pData = reinterpret_cast(pPrivateData); - ASSERT(pData->pWidget); + // Copy the private data since the window owning it may not survive. + CFFL_PrivateData privateData = + *reinterpret_cast(pPrivateData); + ASSERT(privateData.pWidget); - CFFL_FormFiller* pFormFiller = GetFormFiller(pData->pWidget, false); + CFFL_FormFiller* pFormFiller = GetFormFiller(privateData.pWidget, false); #ifdef PDF_ENABLE_XFA - if (pFormFiller->IsFieldFull(pData->pPageView)) { - CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget); - if (OnFull(&pObserved, pData->pPageView, nFlag) || !pObserved) - return {bRC, true}; + if (pFormFiller->IsFieldFull(privateData.pPageView)) { + CPDFSDK_Annot::ObservedPtr pObserved(privateData.pWidget); + if (OnFull(&pObserved, privateData.pPageView, nFlag) || !pObserved) + return {true, true}; } #endif // PDF_ENABLE_XFA if (m_bNotifying || - !pData->pWidget->GetAAction(CPDF_AAction::KeyStroke).GetDict()) { - return {bRC, bExit}; + !privateData.pWidget->GetAAction(CPDF_AAction::KeyStroke).GetDict()) { + return {true, false}; } CFX_AutoRestorer restorer(&m_bNotifying); m_bNotifying = true; - int nAge = pData->pWidget->GetAppearanceAge(); - int nValueAge = pData->pWidget->GetValueAge(); + int nAge = privateData.pWidget->GetAppearanceAge(); + int nValueAge = privateData.pWidget->GetValueAge(); CPDFSDK_FormFillEnvironment* pFormFillEnv = - pData->pPageView->GetFormFillEnv(); + privateData.pPageView->GetFormFillEnv(); PDFSDK_FieldAction fa; fa.bModifier = CPDFSDK_FormFillEnvironment::IsCTRLKeyDown(nFlag); @@ -901,37 +901,38 @@ std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( fa.bRC = true; fa.nSelStart = nSelStart; fa.nSelEnd = nSelEnd; + pFormFiller->GetActionData(privateData.pPageView, CPDF_AAction::KeyStroke, + fa); + pFormFiller->SaveState(privateData.pPageView); - pFormFiller->GetActionData(pData->pPageView, CPDF_AAction::KeyStroke, fa); - pFormFiller->SaveState(pData->pPageView); + CPDFSDK_Annot::ObservedPtr pObserved(privateData.pWidget); + bool action_status = privateData.pWidget->OnAAction( + CPDF_AAction::KeyStroke, fa, privateData.pPageView); - CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget); - if (!pData->pWidget->OnAAction(CPDF_AAction::KeyStroke, fa, - pData->pPageView)) { - if (!IsValidAnnot(pData->pPageView, pData->pWidget)) - bExit = true; - return {bRC, bExit}; - } + if (!pObserved || !IsValidAnnot(privateData.pPageView, privateData.pWidget)) + return {true, true}; - if (!pObserved || !IsValidAnnot(pData->pPageView, pData->pWidget)) - return {bRC, true}; + if (!action_status) + return {true, false}; - if (nAge != pData->pWidget->GetAppearanceAge()) { + bool bExit = false; + if (nAge != privateData.pWidget->GetAppearanceAge()) { CPWL_Wnd* pWnd = pFormFiller->ResetPDFWindow( - pData->pPageView, nValueAge == pData->pWidget->GetValueAge()); - pData = reinterpret_cast(pWnd->GetAttachedData()); + privateData.pPageView, nValueAge == privateData.pWidget->GetValueAge()); + if (!pWnd) + return {true, true}; + privateData = *reinterpret_cast(pWnd->GetAttachedData()); bExit = true; } + if (fa.bRC) { + pFormFiller->SetActionData(privateData.pPageView, CPDF_AAction::KeyStroke, + fa); + } else { + pFormFiller->RestoreState(privateData.pPageView); + } + if (pFormFillEnv->GetFocusAnnot() == privateData.pWidget) + return {false, bExit}; - if (fa.bRC) - pFormFiller->SetActionData(pData->pPageView, CPDF_AAction::KeyStroke, fa); - else - pFormFiller->RestoreState(pData->pPageView); - bRC = false; - - if (pFormFillEnv->GetFocusAnnot() == pData->pWidget) - return {bRC, bExit}; - - pFormFiller->CommitData(pData->pPageView, nFlag); - return {bRC, true}; + pFormFiller->CommitData(privateData.pPageView, nFlag); + return {false, true}; } diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp index 93350874dc..56e166252e 100644 --- a/fpdfsdk/fpdfformfill_embeddertest.cpp +++ b/fpdfsdk/fpdfformfill_embeddertest.cpp @@ -506,6 +506,17 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_707673) { EXPECT_EQ(0u, alerts.size()); } +TEST_F(FPDFFormFillEmbeddertest, BUG_765384) { + EXPECT_TRUE(OpenDocument("bug_765384.pdf")); + FPDF_PAGE page = LoadPage(0); + EXPECT_TRUE(page); + + DoOpenActions(); + FORM_OnLButtonDown(form_handle(), page, 0, 140, 590); + FORM_OnLButtonUp(form_handle(), page, 0, 140, 590); + UnloadPage(page); +} + #endif // PDF_ENABLE_V8 TEST_F(FPDFFormFillEmbeddertest, FormText) { diff --git a/testing/resources/bug_765384.in b/testing/resources/bug_765384.in new file mode 100644 index 0000000000..1de85d31ba --- /dev/null +++ b/testing/resources/bug_765384.in @@ -0,0 +1,105 @@ +{{header}} +{{object 1 0}} << + /Type /Catalog + /Pages 2 0 R + /AcroForm 4 0 R + /OpenAction 10 0 R +>> +endobj +{{object 2 0}} << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +% Page number 0. +{{object 3 0}} << + /Type /Page + /Parent 2 0 R + /Annots [ + 8 0 R + 9 0 R + ] +>> +endobj +% Forms +{{object 4 0}} << + /XFA [ + (xdp:xdp) 43 0 R + (form) 49 0 R + () 50 0 R + ] + /Fields [ + 8 0 R + 9 0 R + ] +>> +endobj +% Fields +{{object 8 0}} << + /FT /Ch + /Ff 0 + /T (MyField) + /Type /Annot + /Subtype /Widget + /Rect [0 0 800 800] + /Opt [(test 1) (test 2)] + /AA << /K 20 0 R >> +>> +endobj +% Fields +{{object 9 0}} << + /FT /Tx + /T (MyField2) + /Type /Annot + /Subtype /Widget + /V (myfield_2) + /Rect [0 0 1 1] +>> +endobj +% JS Action +{{object 20 0}} << + /Type /Action + /S /JavaScript + /JS 21 0 R +>> +endobj +% JS program to exexute +{{object 21 0}} << +>> +stream +function gc() { + tmp = []; + for (var i = 0; i < 0x200000; i++) + tmp.push(new Uint8Array(10)); + tmp = null; +} +this.getField("MyField2").setFocus(); +this.getField("MyField").borderStyle="dashed"; +this.getField("MyField").setFocus(); +gc(); +endstream +endobj +{{object 43 0}} << +>>stream + + +endstream +endobj +{{object 49 0}} << +>>stream + + +endstream +endobj +{{object 50 0}} << +>>stream + +endstream +endobj +{{xref}} +{{trailer}} +{{startxref}} +%%EOF diff --git a/testing/resources/bug_765384.pdf b/testing/resources/bug_765384.pdf new file mode 100644 index 0000000000..fcaa5f4305 --- /dev/null +++ b/testing/resources/bug_765384.pdf @@ -0,0 +1,159 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /Type /Catalog + /Pages 2 0 R + /AcroForm 4 0 R + /OpenAction 10 0 R +>> +endobj +2 0 obj << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +% Page number 0. +3 0 obj << + /Type /Page + /Parent 2 0 R + /Annots [ + 8 0 R + 9 0 R + ] +>> +endobj +% Forms +4 0 obj << + /XFA [ + (xdp:xdp) 43 0 R + (form) 49 0 R + () 50 0 R + ] + /Fields [ + 8 0 R + 9 0 R + ] +>> +endobj +% Fields +8 0 obj << + /FT /Ch + /Ff 0 + /T (MyField) + /Type /Annot + /Subtype /Widget + /Rect [0 0 800 800] + /Opt [(test 1) (test 2)] + /AA << /K 20 0 R >> +>> +endobj +% Fields +9 0 obj << + /FT /Tx + /T (MyField2) + /Type /Annot + /Subtype /Widget + /V (myfield_2) + /Rect [0 0 1 1] +>> +endobj +% JS Action +20 0 obj << + /Type /Action + /S /JavaScript + /JS 21 0 R +>> +endobj +% JS program to exexute +21 0 obj << +>> +stream +function gc() { + tmp = []; + for (var i = 0; i < 0x200000; i++) + tmp.push(new Uint8Array(10)); + tmp = null; +} +this.getField("MyField2").setFocus(); +this.getField("MyField").borderStyle="dashed"; +this.getField("MyField").setFocus(); +gc(); +endstream +endobj +43 0 obj << +>>stream + + +endstream +endobj +49 0 obj << +>>stream + + +endstream +endobj +50 0 obj << +>>stream + +endstream +endobj +xref +0 51 +0000000000 65535 f +0000000015 00000 n +0000000107 00000 n +0000000195 00000 n +0000000292 00000 n +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000436 00000 n +0000000604 00000 n +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000732 00000 n +0000000824 00000 n +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000001106 00000 n +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000000000 65535 f +0000001230 00000 n +0000001308 00000 n +trailer<< /Root 1 0 R /Size 51 >> +startxref +1357 +%%EOF -- cgit v1.2.3