Reinstate CPDF_FormField DCHECKs and improve SetValue() Undo a small portion of https://pdfium-review.googlesource.com/111552 so SetItemSelection() and IsItemSelected() get their DCHECKs back. Instead, fix the issue by changing the caller to avoid tripping the DCHECKs. As part of this work: - Use CHECK instead of DCHECK, per modern coding guidelines. - Add an IsComboOrListField() helper function and use that everywhere. - Add a HasOptField() helper function and use that as an additional guardrail to make sure CPDF_FormField only reads /Opt values when appropriate. - Update FPDFAnnot_IsOptionSelected() to avoid tripping over the /Opt checks. Bug: chromium:1477093 Change-Id: Id774393afcd8e976ffa7f2ae869ceac9c9680b06 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/111670 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp index f4a62bd..bfcb797 100644 --- a/core/fpdfdoc/cpdf_formfield.cpp +++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -50,6 +50,28 @@ nLevel + 1); } +bool IsComboOrListField(CPDF_FormField::Type type) { + switch (type) { + case CPDF_FormField::kComboBox: + case CPDF_FormField::kListBox: + return true; + default: + return false; + } +} + +bool HasOptField(CPDF_FormField::Type type) { + switch (type) { + case CPDF_FormField::kCheckBox: + case CPDF_FormField::kRadioButton: + case CPDF_FormField::kComboBox: + case CPDF_FormField::kListBox: + return true; + default: + return false; + } +} + } // namespace // static @@ -348,7 +370,7 @@ bool CPDF_FormField::SetValue(const WideString& value, bool bDefault, NotificationOption notify) { - switch (m_Type) { + switch (GetType()) { case kCheckBox: case kRadioButton: { SetCheckValue(value, bDefault, notify); @@ -366,7 +388,13 @@ ByteString key(bDefault ? pdfium::form_fields::kDV : pdfium::form_fields::kV); m_pDict->SetNewFor<CPDF_String>(key, csValue.AsStringView()); - int iIndex = FindOption(csValue); + + int iIndex; + if (GetType() == kComboBox) { + iIndex = FindOption(csValue); + } else { + iIndex = -1; + } if (iIndex < 0) { if (m_Type == kRichText && !bDefault) { m_pDict->SetFor(pdfium::form_fields::kRV, @@ -493,9 +521,7 @@ } bool CPDF_FormField::IsItemSelected(int index) const { - if (GetType() != kComboBox && GetType() != kListBox) { - return false; - } + CHECK(IsComboOrListField(GetType())); if (index < 0 || index >= CountOptions()) { return false; } @@ -505,9 +531,7 @@ } bool CPDF_FormField::SetItemSelection(int index, NotificationOption notify) { - if (GetType() != kComboBox && GetType() != kListBox) { - return false; - } + CHECK(IsComboOrListField(GetType())); if (index < 0 || index >= CountOptions()) { return false; } @@ -554,7 +578,7 @@ } int CPDF_FormField::GetDefaultSelectedItem() const { - DCHECK(GetType() == kComboBox || GetType() == kListBox); + CHECK(IsComboOrListField(GetType())); RetainPtr<const CPDF_Object> pValue = GetDefaultValueObject(); if (!pValue) return -1; @@ -569,11 +593,13 @@ } int CPDF_FormField::CountOptions() const { + CHECK(HasOptField(GetType())); RetainPtr<const CPDF_Array> pArray = ToArray(GetFieldAttrInternal("Opt")); return pArray ? fxcrt::CollectionSize<int>(*pArray) : 0; } WideString CPDF_FormField::GetOptionText(int index, int sub_index) const { + CHECK(HasOptField(GetType())); RetainPtr<const CPDF_Array> pArray = ToArray(GetFieldAttrInternal("Opt")); if (!pArray) return WideString(); @@ -776,7 +802,7 @@ } bool CPDF_FormField::UseSelectedIndicesObject() const { - DCHECK(GetType() == kComboBox || GetType() == kListBox); + CHECK(IsComboOrListField(GetType())); RetainPtr<const CPDF_Object> pSelectedIndicesObject = GetSelectedIndicesObject(); @@ -895,13 +921,13 @@ } RetainPtr<const CPDF_Object> CPDF_FormField::GetSelectedIndicesObject() const { - DCHECK(GetType() == kComboBox || GetType() == kListBox); + CHECK(IsComboOrListField(GetType())); return GetFieldAttrInternal("I"); } RetainPtr<const CPDF_Object> CPDF_FormField::GetValueOrSelectedIndicesObject() const { - DCHECK(GetType() == kComboBox || GetType() == kListBox); + CHECK(IsComboOrListField(GetType())); RetainPtr<const CPDF_Object> pValue = GetValueObject(); return pValue ? pValue : GetSelectedIndicesObject(); }
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp index cb7c96d..0cba33e 100644 --- a/fpdfsdk/fpdf_annot.cpp +++ b/fpdfsdk/fpdf_annot.cpp
@@ -1300,19 +1300,22 @@ FPDFAnnot_IsOptionSelected(FPDF_FORMHANDLE handle, FPDF_ANNOTATION annot, int index) { - if (index < 0) + if (index < 0) { return false; + } const CPDF_FormField* form_field = GetFormField(handle, annot); - if (!form_field || index >= form_field->CountOptions()) + if (!form_field) { return false; + } if (form_field->GetFieldType() != FormFieldType::kComboBox && form_field->GetFieldType() != FormFieldType::kListBox) { return false; } - return form_field->IsItemSelected(index); + return index < form_field->CountOptions() && + form_field->IsItemSelected(index); } FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV