Check for NULL XFA context even when XFA

Use strict typing for FPDF_Page to ensure we don't fall into
code that expects the other page type when continuing from null
context case.

Change-Id: I7f028ef3e3d733f5557620030a87e22997da00d5
Reviewed-on: https://pdfium-review.googlesource.com/31770
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

9 files changed, 89 insertions, 49 deletions

diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
index 6007ac9443..599aed3cf2 100644
--- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp
+++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp
@@ -234,8 +234,8 @@ CPDFSDK_FormFillEnvironment::GetInteractiveFormFiller() {
 void CPDFSDK_FormFillEnvironment::Invalidate(UnderlyingPageType* page,
                                              const FX_RECT& rect) {
   if (m_pInfo && m_pInfo->FFI_Invalidate) {
-    m_pInfo->FFI_Invalidate(m_pInfo, page, rect.left, rect.top, rect.right,
-                            rect.bottom);
+    m_pInfo->FFI_Invalidate(m_pInfo, FPDFPageFromUnderlying(page), rect.left,
+                            rect.top, rect.right, rect.bottom);
   }
 }
 
@@ -243,8 +243,9 @@ void CPDFSDK_FormFillEnvironment::OutputSelectedRect(
     UnderlyingPageType* page,
     const CFX_FloatRect& rect) {
   if (m_pInfo && m_pInfo->FFI_OutputSelectedRect) {
-    m_pInfo->FFI_OutputSelectedRect(m_pInfo, page, rect.left, rect.top,
-                                    rect.right, rect.bottom);
+    m_pInfo->FFI_OutputSelectedRect(m_pInfo, FPDFPageFromUnderlying(page),
+                                    rect.left, rect.top, rect.right,
+                                    rect.bottom);
   }
 }
 
@@ -331,8 +332,8 @@ void CPDFSDK_FormFillEnvironment::DisplayCaret(CPDFXFA_Page* page,
                                                double right,
                                                double bottom) {
   if (m_pInfo && m_pInfo->FFI_DisplayCaret) {
-    m_pInfo->FFI_DisplayCaret(m_pInfo, page, bVisible, left, top, right,
-                              bottom);
+    m_pInfo->FFI_DisplayCaret(m_pInfo, FPDFPageFromUnderlying(page), bVisible,
+                              left, top, right, bottom);
   }
 }
 
@@ -386,7 +387,8 @@ void CPDFSDK_FormFillEnvironment::GetPageViewRect(CPDFXFA_Page* page,
   double top;
   double right;
   double bottom;
-  m_pInfo->FFI_GetPageViewRect(m_pInfo, page, &left, &top, &right, &bottom);
+  m_pInfo->FFI_GetPageViewRect(m_pInfo, FPDFPageFromUnderlying(page), &left,
+                               &top, &right, &bottom);
 
   dstRect.left = static_cast<float>(left);
   dstRect.top = static_cast<float>(top);
@@ -399,7 +401,8 @@ bool CPDFSDK_FormFillEnvironment::PopupMenu(CPDFXFA_Page* page,
                                             int menuFlag,
                                             CFX_PointF pt) {
   return m_pInfo && m_pInfo->FFI_PopupMenu &&
-         m_pInfo->FFI_PopupMenu(m_pInfo, page, hWidget, menuFlag, pt.x, pt.y);
+         m_pInfo->FFI_PopupMenu(m_pInfo, FPDFPageFromUnderlying(page), hWidget,
+                                menuFlag, pt.x, pt.y);
 }
 
 void CPDFSDK_FormFillEnvironment::Alert(FPDF_WIDESTRING Msg,
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index 01d88f9f59..bffc6eb240 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -145,7 +145,11 @@ bool FPDF_FileHandlerContext::Flush() {
 }  // namespace
 
 UnderlyingPageType* UnderlyingFromFPDFPage(FPDF_PAGE page) {
-  return static_cast<UnderlyingPageType*>(page);
+  return reinterpret_cast<UnderlyingPageType*>(page);
+}
+
+FPDF_PAGE FPDFPageFromUnderlying(UnderlyingPageType* page) {
+  return reinterpret_cast<FPDF_PAGE>(page);
 }
 
 CPDF_Document* CPDFDocumentFromFPDFDocument(FPDF_DOCUMENT doc) {
diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h
index afb6dde239..937ceed75e 100644
--- a/fpdfsdk/cpdfsdk_helpers.h
+++ b/fpdfsdk/cpdfsdk_helpers.h
@@ -47,6 +47,7 @@ using UnderlyingPageType = CPDFXFA_Page;
 
 // Conversions to/from underlying types.
 UnderlyingPageType* UnderlyingFromFPDFPage(FPDF_PAGE page);
+FPDF_PAGE FPDFPageFromUnderlying(UnderlyingPageType* page);
 
 // Conversions to/from FPDF_ types.
 CPDF_Document* CPDFDocumentFromFPDFDocument(FPDF_DOCUMENT doc);
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index 3acbe76d10..19a61bcdc6 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -156,14 +156,17 @@ FPDF_EXPORT void FPDF_CALLCONV FPDFPage_Delete(FPDF_DOCUMENT document,
   auto* pDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pDoc)
     return;
+
 #ifdef PDF_ENABLE_XFA
   CPDFXFA_Context* pContext =
       static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
-  if (pContext)
+  if (pContext) {
     pContext->DeletePage(page_index);
-#else
+    return;
+  }
+#endif  // PDF_ENABLE_XFA
+
   pDoc->DeletePage(page_index);
-#endif
 }
 
 FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document,
@@ -184,14 +187,18 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document,
   pPageDict->SetNewFor<CPDF_Dictionary>("Resources");
 
 #ifdef PDF_ENABLE_XFA
-  auto pXFAPage = pdfium::MakeRetain<CPDFXFA_Page>(
-      static_cast<CPDFXFA_Context*>(pDoc->GetExtension()), page_index);
-  pXFAPage->LoadPDFPage(pPageDict);
-  return pXFAPage.Leak();  // Caller takes ownership.
+  auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
+  if (pContext) {
+    auto pXFAPage = pdfium::MakeRetain<CPDFXFA_Page>(pContext, page_index);
+    pXFAPage->LoadPDFPage(pPageDict);
+    return FPDFPageFromUnderlying(pXFAPage.Leak());  // Caller takes ownership.
+  }
+  // Eventually, fallthru into non-XFA case once page type is consistent.
+  return nullptr;
 #else  // PDF_ENABLE_XFA
   auto pPage = pdfium::MakeUnique<CPDF_Page>(pDoc, pPageDict, true);
   pPage->ParseContent();
-  return pPage.release();  // Caller takes ownership.
+  return FPDFPageFromUnderlying(pPage.release());  // Caller takes ownership.
 #endif  // PDF_ENABLE_XFA
 }
 
diff --git a/fpdfsdk/fpdf_formfill.cpp b/fpdfsdk/fpdf_formfill.cpp
index 3170627cd5..202835cb57 100644
--- a/fpdfsdk/fpdf_formfill.cpp
+++ b/fpdfsdk/fpdf_formfill.cpp
@@ -185,7 +185,8 @@ void FFLCommon(FPDF_FORMHANDLE hHandle,
 #else   // PDF_ENABLE_XFA
     options.SetOCContext(pdfium::MakeRetain<CPDF_OCContext>(
         pPage->m_pDocument.Get(), CPDF_OCContext::View));
-    if (CPDFSDK_PageView* pPageView = FormHandleToPageView(hHandle, pPage))
+    if (CPDFSDK_PageView* pPageView =
+            FormHandleToPageView(hHandle, FPDFPageFromUnderlying(pPage)))
       pPageView->PageView_OnDraw(pDevice.get(), &matrix, &options);
 #endif  // PDF_ENABLE_XFA
   }
@@ -296,7 +297,7 @@ FPDFDOC_InitFormFillEnvironment(FPDF_DOCUMENT document,
   // this and can just return the old Env. Otherwise, we'll end up setting a new
   // environment into the XFADocument and, that could get weird.
   auto* pContext = static_cast<CPDFXFA_Context*>(pDocument->GetExtension());
-  if (pContext->GetFormFillEnv())
+  if (pContext && pContext->GetFormFillEnv())
     return pContext->GetFormFillEnv();
 #endif
 
@@ -304,7 +305,8 @@ FPDFDOC_InitFormFillEnvironment(FPDF_DOCUMENT document,
       CPDFDocumentFromFPDFDocument(document), formInfo);
 
 #ifdef PDF_ENABLE_XFA
-  pContext->SetFormFillEnv(pFormFillEnv.get());
+  if (pContext)
+    pContext->SetFormFillEnv(pFormFillEnv.get());
 #endif  // PDF_ENABLE_XFA
 
   return pFormFillEnv.release();  // Caller takes ownership.
diff --git a/fpdfsdk/fpdf_save.cpp b/fpdfsdk/fpdf_save.cpp
index 2844f5a965..e4a1a607bb 100644
--- a/fpdfsdk/fpdf_save.cpp
+++ b/fpdfsdk/fpdf_save.cpp
@@ -227,10 +227,11 @@ bool FPDF_Doc_Save(FPDF_DOCUMENT document,
     return 0;
 
 #ifdef PDF_ENABLE_XFA
-  CPDFXFA_Context* pContext =
-      static_cast<CPDFXFA_Context*>(pPDFDoc->GetExtension());
-  std::vector<RetainPtr<IFX_SeekableStream>> fileList;
-  SendPreSaveToXFADoc(pContext, &fileList);
+  auto* pContext = static_cast<CPDFXFA_Context*>(pPDFDoc->GetExtension());
+  if (pContext) {
+    std::vector<RetainPtr<IFX_SeekableStream>> fileList;
+    SendPreSaveToXFADoc(pContext, &fileList);
+  }
 #endif  // PDF_ENABLE_XFA
 
   if (flags < FPDF_INCREMENTAL || flags > FPDF_REMOVE_SECURITY)
@@ -246,9 +247,11 @@ bool FPDF_Doc_Save(FPDF_DOCUMENT document,
   }
 
   bool bRet = fileMaker.Create(flags);
+
 #ifdef PDF_ENABLE_XFA
   SendPostSaveToXFADoc(pContext);
 #endif  // PDF_ENABLE_XFA
+
   return bRet;
 }
 
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 00c59bbda5..1eb014a427 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -257,8 +257,11 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetFormType(FPDF_DOCUMENT document) {
 #ifdef PDF_ENABLE_XFA
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDF_LoadXFA(FPDF_DOCUMENT document) {
   auto* pDoc = CPDFDocumentFromFPDFDocument(document);
-  return pDoc &&
-         static_cast<CPDFXFA_Context*>(pDoc->GetExtension())->LoadXFADoc();
+  if (!pDoc)
+    return false;
+
+  auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
+  return pContext && pContext->LoadXFADoc();
 }
 #endif  // PDF_ENABLE_XFA
 
@@ -325,12 +328,14 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageCount(FPDF_DOCUMENT document) {
   auto* pDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pDoc)
     return 0;
+
 #ifdef PDF_ENABLE_XFA
   auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
-  return pContext ? pContext->GetPageCount() : 0;
-#else
+  if (pContext)
+    return pContext->GetPageCount();
+#endif  // PDF_ENABLE_XFA
+
   return pDoc->GetPageCount();
-#endif
 }
 
 FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document,
@@ -343,9 +348,12 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document,
     return nullptr;
 
 #ifdef PDF_ENABLE_XFA
-  return static_cast<CPDFXFA_Context*>(pDoc->GetExtension())
-      ->GetXFAPage(page_index)
-      .Leak();
+  auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
+  if (pContext)
+    return FPDFPageFromUnderlying(pContext->GetXFAPage(page_index).Leak());
+
+  // Eventually, fallthrough into non-xfa case once page type made consistent.
+  return nullptr;
 #else   // PDF_ENABLE_XFA
   CPDF_Dictionary* pDict = pDoc->GetPage(page_index);
   if (!pDict)
@@ -353,7 +361,7 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document,
 
   CPDF_Page* pPage = new CPDF_Page(pDoc, pDict, true);
   pPage->ParseContent();
-  return pPage;
+  return FPDFPageFromUnderlying(pPage);
 #endif  // PDF_ENABLE_XFA
 }
 
@@ -752,13 +760,16 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) {
 
 FPDF_EXPORT void FPDF_CALLCONV FPDF_CloseDocument(FPDF_DOCUMENT document) {
   auto* pDoc = CPDFDocumentFromFPDFDocument(document);
+
 #if PDF_ENABLE_XFA
   // Deleting the extension will delete the document
-  if (pDoc)
+  if (pDoc && pDoc->GetExtension()) {
     delete pDoc->GetExtension();
-#else
+    return;
+  }
+#endif  // PDF_ENABLE_XFA
+
   delete pDoc;
-#endif
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV FPDF_GetLastError() {
@@ -943,14 +954,19 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document,
 #ifdef PDF_ENABLE_XFA
   if (page_index < 0 || page_index >= FPDF_GetPageCount(document))
     return false;
-  RetainPtr<CPDFXFA_Page> pPage =
-      static_cast<CPDFXFA_Context*>(pDoc->GetExtension())
-          ->GetXFAPage(page_index);
-  if (!pPage)
-    return false;
-  *width = pPage->GetPageWidth();
-  *height = pPage->GetPageHeight();
-#else   // PDF_ENABLE_XFA
+
+  auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
+  if (pContext) {
+    RetainPtr<CPDFXFA_Page> pPage = pContext->GetXFAPage(page_index);
+    if (!pPage)
+      return false;
+
+    *width = pPage->GetPageWidth();
+    *height = pPage->GetPageHeight();
+    return true;
+  }
+#endif  // PDF_ENABLE_XFA
+
   CPDF_Dictionary* pDict = pDoc->GetPage(page_index);
   if (!pDict)
     return false;
@@ -958,8 +974,6 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document,
   CPDF_Page page(pDoc, pDict, true);
   *width = page.GetPageWidth();
   *height = page.GetPageHeight();
-#endif  // PDF_ENABLE_XFA
-
   return true;
 }
 
diff --git a/public/fpdfview.h b/public/fpdfview.h
index 9ebfd8d99d..817da6cda1 100644
--- a/public/fpdfview.h
+++ b/public/fpdfview.h
@@ -43,17 +43,17 @@ typedef void* FPDF_DEST;
 typedef struct fpdf_document_t__* FPDF_DOCUMENT;
 typedef void* FPDF_FONT;
 typedef void* FPDF_LINK;
-typedef void* FPDF_PAGE;
+typedef struct fpdf_page_t__* FPDF_PAGE;
 typedef void* FPDF_PAGELINK;
 typedef void* FPDF_PAGEOBJECT;  // Page object(text, path, etc)
-typedef void const* FPDF_PAGEOBJECTMARK;
+typedef const void* FPDF_PAGEOBJECTMARK;
 typedef void* FPDF_PAGERANGE;
 typedef void* FPDF_RECORDER;
 typedef void* FPDF_SCHHANDLE;
 typedef void* FPDF_STRUCTELEMENT;
 typedef void* FPDF_STRUCTTREE;
 typedef void* FPDF_TEXTPAGE;
-typedef void const* FPDF_PATHSEGMENT;
+typedef const void* FPDF_PATHSEGMENT;
 
 #ifdef PDF_ENABLE_XFA
 typedef void* FPDF_WIDGET;
diff --git a/testing/xfa_js_embedder_test.cpp b/testing/xfa_js_embedder_test.cpp
index 375497f9ea..fe54621a2b 100644
--- a/testing/xfa_js_embedder_test.cpp
+++ b/testing/xfa_js_embedder_test.cpp
@@ -45,7 +45,13 @@ void XFAJSEmbedderTest::TearDown() {
 
 CXFA_Document* XFAJSEmbedderTest::GetXFADocument() {
   auto* pDoc = CPDFDocumentFromFPDFDocument(document());
+  if (!pDoc)
+    return nullptr;
+
   auto* pContext = static_cast<CPDFXFA_Context*>(pDoc->GetExtension());
+  if (!pContext)
+    return nullptr;
+
   return pContext->GetXFADoc()->GetXFADoc();
 }