Explorar o código

LibPDF: Propagate errors in Parser and Document

Matthew Olsson %!s(int64=3) %!d(string=hai) anos
pai
achega
73cf8205b4

+ 4 - 4
Meta/Lagom/Fuzzers/FuzzPDF.cpp

@@ -10,12 +10,12 @@
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
 {
     ReadonlyBytes bytes { data, size };
-    auto doc = PDF::Document::create(bytes);
 
-    if (doc) {
-        auto pages = doc->get_page_count();
+    if (auto maybe_document = PDF::Document::create(bytes); !maybe_document.is_error()) {
+        auto document = maybe_document.release_value();
+        auto pages = document->get_page_count();
         for (size_t i = 0; i < pages; ++i) {
-            (void)doc->get_page(i);
+            (void)document->get_page(i);
         }
     }
 

+ 8 - 5
Tests/LibPDF/TestPDF.cpp

@@ -15,33 +15,36 @@ TEST_CASE(linearized_pdf)
 {
     auto file = Core::MappedFile::map("linearized.pdf").release_value();
     auto document = PDF::Document::create(file->bytes());
-    EXPECT_EQ(document->get_page_count(), 1U);
+    EXPECT(!document.is_error());
+    EXPECT_EQ(document.value()->get_page_count(), 1U);
 }
 
 TEST_CASE(non_linearized_pdf)
 {
     auto file = Core::MappedFile::map("non-linearized.pdf").release_value();
     auto document = PDF::Document::create(file->bytes());
-    EXPECT_EQ(document->get_page_count(), 1U);
+    EXPECT(!document.is_error());
+    EXPECT_EQ(document.value()->get_page_count(), 1U);
 }
 
 TEST_CASE(complex_pdf)
 {
     auto file = Core::MappedFile::map("complex.pdf").release_value();
     auto document = PDF::Document::create(file->bytes());
-    EXPECT_EQ(document->get_page_count(), 3U);
+    EXPECT(!document.is_error());
+    EXPECT_EQ(document.value()->get_page_count(), 3U);
 }
 
 TEST_CASE(empty_file_issue_10702)
 {
     AK::ReadonlyBytes empty;
     auto document = PDF::Document::create(empty);
-    EXPECT(document.is_null());
+    EXPECT(document.is_error());
 }
 
 TEST_CASE(truncated_pdf_header_issue_10717)
 {
     AK::String string { "%PDF-2.11%" };
     auto document = PDF::Document::create(string.bytes());
-    EXPECT(document.is_null());
+    EXPECT(document.is_error());
 }

+ 2 - 1
Userland/Applications/PDFViewer/PDFViewer.cpp

@@ -63,7 +63,8 @@ RefPtr<Gfx::Bitmap> PDFViewer::get_rendered_page(u32 index)
     if (existing_rendered_page.has_value() && existing_rendered_page.value().rotation == m_rotations)
         return existing_rendered_page.value().bitmap;
 
-    auto rendered_page = render_page(m_document->get_page(index));
+    // FIXME: Propogate errors in the Renderer
+    auto rendered_page = render_page(MUST(m_document->get_page(index)));
     rendered_page_map.set(m_zoom_level, { rendered_page, m_rotations });
     return rendered_page;
 }

+ 7 - 4
Userland/Applications/PDFViewer/PDFViewerWidget.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  * Copyright (c) 2021, Mustafa Quraish <mustafa@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -151,12 +151,15 @@ void PDFViewerWidget::open_file(Core::File& file)
     window()->set_title(String::formatted("{} - PDF Viewer", file.filename()));
 
     m_buffer = file.read_all();
-    auto document = PDF::Document::create(m_buffer);
-    if (!document) {
-        GUI::MessageBox::show_error(nullptr, String::formatted("Couldn't load PDF: {}", file.filename()));
+    auto maybe_document = PDF::Document::create(m_buffer);
+    if (maybe_document.is_error()) {
+        auto error = maybe_document.release_error();
+        GUI::MessageBox::show_error(nullptr, String::formatted("Couldn't load PDF {}:\n{}", file.filename(), error.message()));
         return;
     }
 
+    auto document = maybe_document.release_value();
+
     m_viewer->set_document(document);
     m_total_page_label->set_text(String::formatted("of {}", document->get_page_count()));
 

+ 8 - 7
Userland/Libraries/LibPDF/ColorSpace.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -60,14 +60,15 @@ RefPtr<CalRGBColorSpace> CalRGBColorSpace::create(RefPtr<Document> document, Vec
         return {};
 
     auto param = parameters[0];
-    if (!param.has<NonnullRefPtr<Object>>() || !param.get<NonnullRefPtr<Object>>()->is_dict())
+    if (!param.has<NonnullRefPtr<Object>>() || !param.get<NonnullRefPtr<Object>>()->is<DictObject>())
         return {};
 
-    auto dict = object_cast<DictObject>(param.get<NonnullRefPtr<Object>>());
+    auto dict = param.get<NonnullRefPtr<Object>>()->cast<DictObject>();
     if (!dict->contains(CommonNames::WhitePoint))
         return {};
 
-    auto white_point_array = dict->get_array(document, CommonNames::WhitePoint);
+    // FIXME: Propagate errors
+    auto white_point_array = MUST(dict->get_array(document, CommonNames::WhitePoint));
     if (white_point_array->size() != 3)
         return {};
 
@@ -81,7 +82,7 @@ RefPtr<CalRGBColorSpace> CalRGBColorSpace::create(RefPtr<Document> document, Vec
         return {};
 
     if (dict->contains(CommonNames::BlackPoint)) {
-        auto black_point_array = dict->get_array(document, CommonNames::BlackPoint);
+        auto black_point_array = MUST(dict->get_array(document, CommonNames::BlackPoint));
         if (black_point_array->size() == 3) {
             color_space->m_blackpoint[0] = black_point_array->at(0).to_float();
             color_space->m_blackpoint[1] = black_point_array->at(1).to_float();
@@ -90,7 +91,7 @@ RefPtr<CalRGBColorSpace> CalRGBColorSpace::create(RefPtr<Document> document, Vec
     }
 
     if (dict->contains(CommonNames::Gamma)) {
-        auto gamma_array = dict->get_array(document, CommonNames::Gamma);
+        auto gamma_array = MUST(dict->get_array(document, CommonNames::Gamma));
         if (gamma_array->size() == 3) {
             color_space->m_gamma[0] = gamma_array->at(0).to_float();
             color_space->m_gamma[1] = gamma_array->at(1).to_float();
@@ -99,7 +100,7 @@ RefPtr<CalRGBColorSpace> CalRGBColorSpace::create(RefPtr<Document> document, Vec
     }
 
     if (dict->contains(CommonNames::Matrix)) {
-        auto matrix_array = dict->get_array(document, CommonNames::Matrix);
+        auto matrix_array = MUST(dict->get_array(document, CommonNames::Matrix));
         if (matrix_array->size() == 3) {
             color_space->m_matrix[0] = matrix_array->at(0).to_float();
             color_space->m_matrix[1] = matrix_array->at(1).to_float();

+ 46 - 51
Userland/Libraries/LibPDF/Document.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -34,17 +34,16 @@ String OutlineItem::to_string(int indent) const
     return builder.to_string();
 }
 
-RefPtr<Document> Document::create(ReadonlyBytes bytes)
+PDFErrorOr<NonnullRefPtr<Document>> Document::create(ReadonlyBytes bytes)
 {
     auto parser = adopt_ref(*new Parser({}, bytes));
     auto document = adopt_ref(*new Document(parser));
 
-    if (!parser->initialize())
-        return {};
+    TRY(parser->initialize());
 
-    document->m_catalog = parser->trailer()->get_dict(document, CommonNames::Root);
-    document->build_page_tree();
-    document->build_outline();
+    document->m_catalog = TRY(parser->trailer()->get_dict(document, CommonNames::Root));
+    TRY(document->build_page_tree());
+    TRY(document->build_outline());
 
     return document;
 }
@@ -55,13 +54,13 @@ Document::Document(NonnullRefPtr<Parser> const& parser)
     m_parser->set_document(this);
 }
 
-Value Document::get_or_load_value(u32 index)
+PDFErrorOr<Value> Document::get_or_load_value(u32 index)
 {
     auto value = get_value(index);
     if (!value.has<Empty>()) // FIXME: Use Optional instead?
         return value;
 
-    auto object = m_parser->parse_object_with_index(index);
+    auto object = TRY(m_parser->parse_object_with_index(index));
     m_values.set(index, object);
     return object;
 }
@@ -78,7 +77,7 @@ u32 Document::get_page_count() const
     return m_page_object_indices.size();
 }
 
-Page Document::get_page(u32 index)
+PDFErrorOr<Page> Document::get_page(u32 index)
 {
     VERIFY(index < m_page_object_indices.size());
 
@@ -87,17 +86,18 @@ Page Document::get_page(u32 index)
         return cached_page.value();
 
     auto page_object_index = m_page_object_indices[index];
-    auto raw_page_object = resolve_to<DictObject>(get_or_load_value(page_object_index));
+    auto page_object = TRY(get_or_load_value(page_object_index));
+    auto raw_page_object = TRY(resolve_to<DictObject>(page_object));
 
     if (!raw_page_object->contains(CommonNames::Resources)) {
         // This page inherits its resource dictionary
         TODO();
     }
 
-    auto resources = raw_page_object->get_dict(this, CommonNames::Resources);
-    auto contents = raw_page_object->get_object(this, CommonNames::Contents);
+    auto resources = TRY(raw_page_object->get_dict(this, CommonNames::Resources));
+    auto contents = TRY(raw_page_object->get_object(this, CommonNames::Contents));
 
-    auto media_box_array = raw_page_object->get_array(this, CommonNames::MediaBox);
+    auto media_box_array = TRY(raw_page_object->get_array(this, CommonNames::MediaBox));
     auto media_box = Rectangle {
         media_box_array->at(0).to_float(),
         media_box_array->at(1).to_float(),
@@ -107,7 +107,7 @@ Page Document::get_page(u32 index)
 
     auto crop_box = media_box;
     if (raw_page_object->contains(CommonNames::CropBox)) {
-        auto crop_box_array = raw_page_object->get_array(this, CommonNames::CropBox);
+        auto crop_box_array = TRY(raw_page_object->get_array(this, CommonNames::CropBox));
         crop_box = Rectangle {
             crop_box_array->at(0).to_float(),
             crop_box_array->at(1).to_float(),
@@ -131,7 +131,7 @@ Page Document::get_page(u32 index)
     return page;
 }
 
-Value Document::resolve(Value const& value)
+PDFErrorOr<Value> Document::resolve(Value const& value)
 {
     if (value.has<Reference>()) {
         // FIXME: Surely indirect PDF objects can't contain another indirect PDF object,
@@ -145,26 +145,21 @@ Value Document::resolve(Value const& value)
 
     auto& obj = value.get<NonnullRefPtr<Object>>();
 
-    if (obj->is_indirect_value())
+    if (obj->is<IndirectValue>())
         return static_ptr_cast<IndirectValue>(obj)->value();
 
     return value;
 }
 
-bool Document::build_page_tree()
+PDFErrorOr<void> Document::build_page_tree()
 {
-    if (!m_catalog->contains(CommonNames::Pages))
-        return false;
-    auto page_tree = m_catalog->get_dict(this, CommonNames::Pages);
+    auto page_tree = TRY(m_catalog->get_dict(this, CommonNames::Pages));
     return add_page_tree_node_to_page_tree(page_tree);
 }
 
-bool Document::add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const& page_tree)
+PDFErrorOr<void> Document::add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const& page_tree)
 {
-    if (!page_tree->contains(CommonNames::Kids) || !page_tree->contains(CommonNames::Count))
-        return false;
-
-    auto kids_array = page_tree->get_array(this, CommonNames::Kids);
+    auto kids_array = TRY(page_tree->get_array(this, CommonNames::Kids));
     auto page_count = page_tree->get(CommonNames::Count).value().get<int>();
 
     if (static_cast<size_t>(page_count) != kids_array->elements().size()) {
@@ -173,13 +168,9 @@ bool Document::add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const&
 
         for (auto& value : *kids_array) {
             auto reference_index = value.as_ref_index();
-            bool ok;
-            auto maybe_page_tree_node = m_parser->conditionally_parse_page_tree_node(reference_index, ok);
-            if (!ok)
-                return false;
+            auto maybe_page_tree_node = TRY(m_parser->conditionally_parse_page_tree_node(reference_index));
             if (maybe_page_tree_node) {
-                if (!add_page_tree_node_to_page_tree(maybe_page_tree_node.release_nonnull()))
-                    return false;
+                TRY(add_page_tree_node_to_page_tree(maybe_page_tree_node.release_nonnull()));
             } else {
                 m_page_object_indices.append(reference_index);
             }
@@ -190,33 +181,35 @@ bool Document::add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const&
             m_page_object_indices.append(value.as_ref_index());
     }
 
-    return true;
+    return {};
 }
 
-void Document::build_outline()
+PDFErrorOr<void> Document::build_outline()
 {
     if (!m_catalog->contains(CommonNames::Outlines))
-        return;
+        return {};
 
-    auto outline_dict = m_catalog->get_dict(this, CommonNames::Outlines);
+    auto outline_dict = TRY(m_catalog->get_dict(this, CommonNames::Outlines));
     if (!outline_dict->contains(CommonNames::First))
-        return;
+        return {};
     if (!outline_dict->contains(CommonNames::Last))
-        return;
+        return {};
 
     auto first_ref = outline_dict->get_value(CommonNames::First);
     auto last_ref = outline_dict->get_value(CommonNames::Last);
 
-    auto children = build_outline_item_chain(first_ref, last_ref);
+    auto children = TRY(build_outline_item_chain(first_ref, last_ref));
 
     m_outline = adopt_ref(*new OutlineDict());
     m_outline->children = move(children);
 
     if (outline_dict->contains(CommonNames::Count))
         m_outline->count = outline_dict->get_value(CommonNames::Count).get<int>();
+
+    return {};
 }
 
-NonnullRefPtr<OutlineItem> Document::build_outline_item(NonnullRefPtr<DictObject> const& outline_item_dict)
+PDFErrorOr<NonnullRefPtr<OutlineItem>> Document::build_outline_item(NonnullRefPtr<DictObject> const& outline_item_dict)
 {
     auto outline_item = adopt_ref(*new OutlineItem {});
 
@@ -225,19 +218,20 @@ NonnullRefPtr<OutlineItem> Document::build_outline_item(NonnullRefPtr<DictObject
         auto first_ref = outline_item_dict->get_value(CommonNames::First);
         auto last_ref = outline_item_dict->get_value(CommonNames::Last);
 
-        auto children = build_outline_item_chain(first_ref, last_ref);
+        auto children = TRY(build_outline_item_chain(first_ref, last_ref));
         outline_item->children = move(children);
     }
 
-    outline_item->title = outline_item_dict->get_string(this, CommonNames::Title)->string();
+    outline_item->title = TRY(outline_item_dict->get_string(this, CommonNames::Title))->string();
 
     if (outline_item_dict->contains(CommonNames::Count))
         outline_item->count = outline_item_dict->get_value(CommonNames::Count).get<int>();
 
     if (outline_item_dict->contains(CommonNames::Dest)) {
-        auto dest_arr = outline_item_dict->get_array(this, CommonNames::Dest);
+        auto dest_arr = TRY(outline_item_dict->get_array(this, CommonNames::Dest));
+        dbgln("IS ARRAY = {}", dest_arr->is_array());
         auto page_ref = dest_arr->at(0);
-        auto type_name = dest_arr->get_name_at(this, 1)->name();
+        auto type_name = TRY(dest_arr->get_name_at(this, 1))->name();
 
         Vector<float> parameters;
         for (size_t i = 2; i < dest_arr->size(); i++)
@@ -268,7 +262,7 @@ NonnullRefPtr<OutlineItem> Document::build_outline_item(NonnullRefPtr<DictObject
     }
 
     if (outline_item_dict->contains(CommonNames::C)) {
-        auto color_array = outline_item_dict->get_array(this, CommonNames::C);
+        auto color_array = TRY(outline_item_dict->get_array(this, CommonNames::C));
         auto r = static_cast<int>(255.0f * color_array->at(0).get<float>());
         auto g = static_cast<int>(255.0f * color_array->at(1).get<float>());
         auto b = static_cast<int>(255.0f * color_array->at(2).get<float>());
@@ -284,16 +278,16 @@ NonnullRefPtr<OutlineItem> Document::build_outline_item(NonnullRefPtr<DictObject
     return outline_item;
 }
 
-NonnullRefPtrVector<OutlineItem> Document::build_outline_item_chain(Value const& first_ref, Value const& last_ref)
+PDFErrorOr<NonnullRefPtrVector<OutlineItem>> Document::build_outline_item_chain(Value const& first_ref, Value const& last_ref)
 {
     VERIFY(first_ref.has<Reference>());
     VERIFY(last_ref.has<Reference>());
 
     NonnullRefPtrVector<OutlineItem> children;
 
-    auto first_dict = object_cast<DictObject>(
-        get_or_load_value(first_ref.as_ref_index()).get<NonnullRefPtr<Object>>());
-    auto first = build_outline_item(first_dict);
+    auto first_value = TRY(get_or_load_value(first_ref.as_ref_index())).get<NonnullRefPtr<Object>>();
+    auto first_dict = first_value->cast<DictObject>();
+    auto first = TRY(build_outline_item(first_dict));
     children.append(first);
 
     auto current_child_dict = first_dict;
@@ -302,8 +296,9 @@ NonnullRefPtrVector<OutlineItem> Document::build_outline_item_chain(Value const&
     while (current_child_dict->contains(CommonNames::Next)) {
         auto next_child_dict_ref = current_child_dict->get_value(CommonNames::Next);
         current_child_index = next_child_dict_ref.as_ref_index();
-        auto next_child_dict = object_cast<DictObject>(get_or_load_value(current_child_index).get<NonnullRefPtr<Object>>());
-        auto next_child = build_outline_item(next_child_dict);
+        auto next_child_value = TRY(get_or_load_value(current_child_index)).get<NonnullRefPtr<Object>>();
+        auto next_child_dict = next_child_value->cast<DictObject>();
+        auto next_child = TRY(build_outline_item(next_child_dict));
         children.append(next_child);
 
         current_child_dict = move(next_child_dict);

+ 19 - 16
Userland/Libraries/LibPDF/Document.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -11,6 +11,7 @@
 #include <AK/RefCounted.h>
 #include <AK/Weakable.h>
 #include <LibGfx/Color.h>
+#include <LibPDF/Error.h>
 #include <LibPDF/ObjectDerivatives.h>
 #include <LibPDF/Parser.h>
 
@@ -75,17 +76,17 @@ class Document final
     : public RefCounted<Document>
     , public Weakable<Document> {
 public:
-    static RefPtr<Document> create(ReadonlyBytes bytes);
+    static PDFErrorOr<NonnullRefPtr<Document>> create(ReadonlyBytes bytes);
 
     ALWAYS_INLINE RefPtr<OutlineDict> const& outline() const { return m_outline; }
 
-    [[nodiscard]] Value get_or_load_value(u32 index);
+    [[nodiscard]] PDFErrorOr<Value> get_or_load_value(u32 index);
 
     [[nodiscard]] u32 get_first_page_index() const;
 
     [[nodiscard]] u32 get_page_count() const;
 
-    [[nodiscard]] Page get_page(u32 index);
+    [[nodiscard]] PDFErrorOr<Page> get_page(u32 index);
 
     ALWAYS_INLINE Value get_value(u32 index) const
     {
@@ -95,23 +96,25 @@ public:
     // Strips away the layer of indirection by turning indirect value
     // refs into the value they reference, and indirect values into
     // the value being wrapped.
-    Value resolve(Value const& value);
+    PDFErrorOr<Value> resolve(Value const& value);
 
     // Like resolve, but unwraps the Value into the given type. Accepts
     // any object type, and the three primitive Value types.
     template<IsValueType T>
-    UnwrappedValueType<T> resolve_to(Value const& value)
+    PDFErrorOr<UnwrappedValueType<T>> resolve_to(Value const& value)
     {
-        auto resolved = resolve(value);
+        auto resolved = TRY(resolve(value));
 
         if constexpr (IsSame<T, bool>)
             return resolved.get<bool>();
-        if constexpr (IsSame<T, int>)
+        else if constexpr (IsSame<T, int>)
             return resolved.get<int>();
-        if constexpr (IsSame<T, float>)
+        else if constexpr (IsSame<T, float>)
             return resolved.get<float>();
-        if constexpr (IsObject<T>)
-            return object_cast<T>(resolved.get<NonnullRefPtr<Object>>());
+        else if constexpr (IsSame<T, Object>)
+            return resolved.get<NonnullRefPtr<Object>>();
+        else if constexpr (IsObject<T>)
+            return resolved.get<NonnullRefPtr<Object>>()->cast<T>();
 
         VERIFY_NOT_REACHED();
     }
@@ -125,12 +128,12 @@ private:
     // parsing, as good PDF writers will layout the page tree in a balanced tree to
     // improve lookup time. This would reduce the initial overhead by not loading
     // every page tree node of, say, a 1000+ page PDF file.
-    bool build_page_tree();
-    bool add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const& page_tree);
+    PDFErrorOr<void> build_page_tree();
+    PDFErrorOr<void> add_page_tree_node_to_page_tree(NonnullRefPtr<DictObject> const& page_tree);
 
-    void build_outline();
-    NonnullRefPtr<OutlineItem> build_outline_item(NonnullRefPtr<DictObject> const& outline_item_dict);
-    NonnullRefPtrVector<OutlineItem> build_outline_item_chain(Value const& first_ref, Value const& last_ref);
+    PDFErrorOr<void> build_outline();
+    PDFErrorOr<NonnullRefPtr<OutlineItem>> build_outline_item(NonnullRefPtr<DictObject> const& outline_item_dict);
+    PDFErrorOr<NonnullRefPtrVector<OutlineItem>> build_outline_item_chain(Value const& first_ref, Value const& last_ref);
 
     NonnullRefPtr<Parser> m_parser;
     RefPtr<DictObject> m_catalog;

+ 48 - 0
Userland/Libraries/LibPDF/Error.h

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2022, Matthew Olsson <mattco@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/String.h>
+
+namespace PDF {
+
+class Error {
+public:
+    enum class Type {
+        Parse,
+        Internal,
+        MalformedPDF,
+    };
+
+    Error(Type type, String const& message)
+        : m_type(type)
+    {
+        switch (type) {
+        case Type::Parse:
+            m_message = String::formatted("Failed to parse PDF file: {}", message);
+            break;
+        case Type::Internal:
+            m_message = String::formatted("Internal error while processing PDF file: {}", message);
+            break;
+        case Type::MalformedPDF:
+            m_message = String::formatted("Malformed PDF file: {}", message);
+            break;
+        }
+    }
+
+    Type type() const { return m_type; }
+    String const& message() const { return m_message; }
+
+private:
+    Type m_type;
+    String m_message;
+};
+
+template<typename T>
+using PDFErrorOr = ErrorOr<T, Error>;
+
+}

+ 55 - 26
Userland/Libraries/LibPDF/Object.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -11,9 +11,29 @@
 #include <AK/Format.h>
 #include <AK/RefCounted.h>
 #include <AK/SourceLocation.h>
+#include <LibPDF/Error.h>
 #include <LibPDF/Forward.h>
 #include <LibPDF/Value.h>
 
+#ifdef PDF_DEBUG
+namespace {
+
+template<PDF::IsObject T>
+const char* object_name()
+{
+#    define ENUMERATE_TYPE(class_name, snake_name)  \
+        if constexpr (IsSame<PDF::class_name, T>) { \
+            return #class_name;                     \
+        }
+    ENUMERATE_OBJECT_TYPES(ENUMERATE_TYPE)
+#    undef ENUMERATE_TYPE
+
+    VERIFY_NOT_REACHED();
+}
+
+}
+#endif
+
 namespace PDF {
 
 class Object : public RefCounted<Object> {
@@ -23,39 +43,48 @@ public:
     [[nodiscard]] ALWAYS_INLINE u32 generation_index() const { return m_generation_index; }
     ALWAYS_INLINE void set_generation_index(u32 generation_index) { m_generation_index = generation_index; }
 
-#define DEFINE_ID(_, name) \
-    virtual bool is_##name() const { return false; }
-    ENUMERATE_OBJECT_TYPES(DEFINE_ID)
-#undef DEFINE_ID
-
-    virtual const char* type_name() const = 0;
-    virtual String to_string(int indent) const = 0;
+    template<IsObject T>
+    bool is() const requires(!IsSame<T, Object>)
+    {
+#define ENUMERATE_TYPE(class_name, snake_name) \
+    if constexpr (IsSame<class_name, T>) {     \
+        return is_##snake_name();              \
+    }
+        ENUMERATE_OBJECT_TYPES(ENUMERATE_TYPE)
+#undef ENUMERATE_TYPE
 
-private:
-    u32 m_generation_index { 0 };
-};
+        VERIFY_NOT_REACHED();
+    }
 
-template<IsObject To, IsObject From>
-[[nodiscard]] ALWAYS_INLINE static NonnullRefPtr<To> object_cast(NonnullRefPtr<From> obj
+    template<IsObject T>
+    [[nodiscard]] ALWAYS_INLINE NonnullRefPtr<T> cast(
 #ifdef PDF_DEBUG
-    ,
-    SourceLocation loc = SourceLocation::current()
+        SourceLocation loc = SourceLocation::current()
 #endif
-)
-{
+    ) const requires(!IsSame<T, Object>)
+    {
 #ifdef PDF_DEBUG
-#    define ENUMERATE_TYPES(class_name, snake_name)                                                \
-        if constexpr (IsSame<To, class_name>) {                                                    \
-            if (!obj->is_##snake_name()) {                                                         \
-                dbgln("{} invalid cast from type {} to type " #snake_name, loc, obj->type_name()); \
-            }                                                                                      \
+        if (!is<T>()) {
+            dbgln("{} invalid cast from {} to {}", loc, type_name(), object_name<T>());
+            VERIFY_NOT_REACHED();
         }
-    ENUMERATE_OBJECT_TYPES(ENUMERATE_TYPES)
-#    undef ENUMERATE_TYPES
 #endif
 
-    return static_ptr_cast<To>(obj);
-}
+        return NonnullRefPtr<T>(static_cast<T const&>(*this));
+    }
+
+    virtual const char* type_name() const = 0;
+    virtual String to_string(int indent) const = 0;
+
+protected:
+#define ENUMERATE_TYPE(_, name) \
+    virtual bool is_##name() const { return false; }
+    ENUMERATE_OBJECT_TYPES(ENUMERATE_TYPE)
+#undef ENUMERATE_TYPE
+
+private:
+    u32 m_generation_index { 0 };
+};
 
 }
 

+ 13 - 16
Userland/Libraries/LibPDF/ObjectDerivatives.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -10,25 +10,22 @@
 
 namespace PDF {
 
-NonnullRefPtr<Object> ArrayObject::get_object_at(Document* document, size_t index) const
-{
-    return document->resolve_to<Object>(m_elements[index]);
-}
-
-NonnullRefPtr<Object> DictObject::get_object(Document* document, FlyString const& key) const
+PDFErrorOr<NonnullRefPtr<Object>> DictObject::get_object(Document* document, FlyString const& key) const
 {
     return document->resolve_to<Object>(get_value(key));
 }
 
-#define DEFINE_ACCESSORS(class_name, snake_name)                                                           \
-    NonnullRefPtr<class_name> ArrayObject::get_##snake_name##_at(Document* document, size_t index) const   \
-    {                                                                                                      \
-        return document->resolve_to<class_name>(m_elements[index]);                                        \
-    }                                                                                                      \
-                                                                                                           \
-    NonnullRefPtr<class_name> DictObject::get_##snake_name(Document* document, FlyString const& key) const \
-    {                                                                                                      \
-        return document->resolve_to<class_name>(get(key).value());                                         \
+#define DEFINE_ACCESSORS(class_name, snake_name)                                                                       \
+    PDFErrorOr<NonnullRefPtr<class_name>> ArrayObject::get_##snake_name##_at(Document* document, size_t index) const   \
+    {                                                                                                                  \
+        if (index >= m_elements.size())                                                                                \
+            return Error { Error::Type::Internal, "Out of bounds array access" };                                      \
+        return document->resolve_to<class_name>(m_elements[index]);                                                    \
+    }                                                                                                                  \
+                                                                                                                       \
+    PDFErrorOr<NonnullRefPtr<class_name>> DictObject::get_##snake_name(Document* document, FlyString const& key) const \
+    {                                                                                                                  \
+        return document->resolve_to<class_name>(get(key).value());                                                     \
     }
 ENUMERATE_OBJECT_TYPES(DEFINE_ACCESSORS)
 #undef DEFINE_INDEXER

+ 30 - 20
Userland/Libraries/LibPDF/ObjectDerivatives.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  * Copyright (c) 2021, Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -30,10 +30,12 @@ public:
     [[nodiscard]] ALWAYS_INLINE String const& string() const { return m_string; }
     [[nodiscard]] ALWAYS_INLINE bool is_binary() const { return m_is_binary; }
 
-    ALWAYS_INLINE bool is_string() const override { return true; }
-    ALWAYS_INLINE const char* type_name() const override { return "string"; }
+    const char* type_name() const override { return "string"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_string() const override { return true; }
+
 private:
     String m_string;
     bool m_is_binary;
@@ -50,10 +52,12 @@ public:
 
     [[nodiscard]] ALWAYS_INLINE FlyString const& name() const { return m_name; }
 
-    ALWAYS_INLINE bool is_name() const override { return true; }
-    ALWAYS_INLINE const char* type_name() const override { return "name"; }
+    const char* type_name() const override { return "name"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_name() const override { return true; }
+
 private:
     FlyString m_name;
 };
@@ -76,20 +80,20 @@ public:
     ALWAYS_INLINE Value const& operator[](size_t index) const { return at(index); }
     ALWAYS_INLINE Value const& at(size_t index) const { return m_elements[index]; }
 
-    NonnullRefPtr<Object> get_object_at(Document*, size_t index) const;
-
 #define DEFINE_INDEXER(class_name, snake_name) \
-    NonnullRefPtr<class_name> get_##snake_name##_at(Document*, size_t index) const;
+    PDFErrorOr<NonnullRefPtr<class_name>> get_##snake_name##_at(Document*, size_t index) const;
     ENUMERATE_OBJECT_TYPES(DEFINE_INDEXER)
 #undef DEFINE_INDEXER
 
-    ALWAYS_INLINE bool is_array() const override
+    const char* type_name() const override
     {
-        return true;
+        return "array";
     }
-    ALWAYS_INLINE const char* type_name() const override { return "array"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_array() const override { return true; }
+
 private:
     Vector<Value> m_elements;
 };
@@ -117,20 +121,22 @@ public:
         return value.value();
     }
 
-    NonnullRefPtr<Object> get_object(Document*, FlyString const& key) const;
+    PDFErrorOr<NonnullRefPtr<Object>> get_object(Document*, FlyString const& key) const;
 
 #define DEFINE_GETTER(class_name, snake_name) \
-    NonnullRefPtr<class_name> get_##snake_name(Document*, FlyString const& key) const;
+    PDFErrorOr<NonnullRefPtr<class_name>> get_##snake_name(Document*, FlyString const& key) const;
     ENUMERATE_OBJECT_TYPES(DEFINE_GETTER)
 #undef DEFINE_GETTER
 
-    ALWAYS_INLINE bool is_dict() const override
+    const char* type_name() const override
     {
-        return true;
+        return "dict";
     }
-    ALWAYS_INLINE const char* type_name() const override { return "dict"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_dict() const override { return true; }
+
 private:
     HashMap<FlyString, Value> m_map;
 };
@@ -147,10 +153,12 @@ public:
     [[nodiscard]] ALWAYS_INLINE NonnullRefPtr<DictObject> dict() const { return m_dict; }
     [[nodiscard]] virtual ReadonlyBytes bytes() const = 0;
 
-    ALWAYS_INLINE bool is_stream() const override { return true; }
-    ALWAYS_INLINE const char* type_name() const override { return "stream"; }
+    const char* type_name() const override { return "stream"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_stream() const override { return true; }
+
 private:
     NonnullRefPtr<DictObject> m_dict;
 };
@@ -201,10 +209,12 @@ public:
     [[nodiscard]] ALWAYS_INLINE u32 index() const { return m_index; }
     [[nodiscard]] ALWAYS_INLINE Value const& value() const { return m_value; }
 
-    ALWAYS_INLINE bool is_indirect_value() const override { return true; }
-    ALWAYS_INLINE const char* type_name() const override { return "indirect_object"; }
+    const char* type_name() const override { return "indirect_object"; }
     String to_string(int indent) const override;
 
+protected:
+    bool is_indirect_value() const override { return true; }
+
 private:
     u32 m_index;
     Value m_value;

+ 163 - 211
Userland/Libraries/LibPDF/Parser.cpp

@@ -22,7 +22,7 @@ static NonnullRefPtr<T> make_object(Args... args) requires(IsBaseOf<Object, T>)
     return adopt_ref(*new T(forward<Args>(args)...));
 }
 
-Vector<Command> Parser::parse_graphics_commands(ReadonlyBytes bytes)
+PDFErrorOr<Vector<Command>> Parser::parse_graphics_commands(ReadonlyBytes bytes)
 {
     auto parser = adopt_ref(*new Parser(bytes));
     return parser->parse_graphics_commands();
@@ -43,16 +43,13 @@ void Parser::set_document(WeakPtr<Document> const& document)
     m_document = document;
 }
 
-bool Parser::initialize()
+PDFErrorOr<void> Parser::initialize()
 {
-    if (!parse_header())
-        return {};
+    TRY(parse_header());
 
-    const auto result = initialize_linearization_dict();
-    if (result == LinearizationResult::Error)
-        return {};
+    const auto linearization_result = TRY(initialize_linearization_dict());
 
-    if (result == LinearizationResult::NotLinearized)
+    if (linearization_result == LinearizationResult::NotLinearized)
         return initialize_non_linearized_xref_table();
 
     bool is_linearized = m_linearization_dictionary.has_value();
@@ -75,37 +72,40 @@ bool Parser::initialize()
     return initialize_non_linearized_xref_table();
 }
 
-Value Parser::parse_object_with_index(u32 index)
+PDFErrorOr<Value> Parser::parse_object_with_index(u32 index)
 {
     VERIFY(m_xref_table->has_object(index));
     auto byte_offset = m_xref_table->byte_offset_for_object(index);
     m_reader.move_to(byte_offset);
-    auto indirect_value = parse_indirect_value();
-    VERIFY(indirect_value);
+    auto indirect_value = TRY(parse_indirect_value());
     VERIFY(indirect_value->index() == index);
     return indirect_value->value();
 }
 
-bool Parser::parse_header()
+PDFErrorOr<void> Parser::parse_header()
 {
     // FIXME: Do something with the version?
     m_reader.set_reading_forwards();
     if (m_reader.remaining() == 0)
-        return false;
+        return error("Empty PDF document");
+
     m_reader.move_to(0);
     if (m_reader.remaining() < 8 || !m_reader.matches("%PDF-"))
-        return false;
+        return error("Not a PDF document");
+
     m_reader.move_by(5);
 
     char major_ver = m_reader.read();
     if (major_ver != '1' && major_ver != '2')
-        return false;
+        return error(String::formatted("Unknown major version \"{}\"", major_ver));
+
     if (m_reader.read() != '.')
-        return false;
+        return error("Malformed PDF version");
 
     char minor_ver = m_reader.read();
     if (minor_ver < '0' || minor_ver > '7')
-        return false;
+        return error(String::formatted("Unknown minor version \"{}\"", minor_ver));
+
     consume_eol();
 
     // Parse optional high-byte comment, which signifies a binary file
@@ -119,27 +119,28 @@ bool Parser::parse_header()
         }
     }
 
-    return true;
+    return {};
 }
 
-Parser::LinearizationResult Parser::initialize_linearization_dict()
+PDFErrorOr<Parser::LinearizationResult> Parser::initialize_linearization_dict()
 {
     // parse_header() is called immediately before this, so we are at the right location
-    auto dict_value = m_document->resolve(parse_indirect_value());
+    auto indirect_value = Value(*TRY(parse_indirect_value()));
+    auto dict_value = TRY(m_document->resolve(indirect_value));
     if (!dict_value.has<NonnullRefPtr<Object>>())
-        return LinearizationResult::Error;
+        return error("Expected linearization object to be a dictionary");
 
     auto dict_object = dict_value.get<NonnullRefPtr<Object>>();
-    if (!dict_object->is_dict())
+    if (!dict_object->is<DictObject>())
         return LinearizationResult::NotLinearized;
 
-    auto dict = object_cast<DictObject>(dict_object);
+    auto dict = dict_object->cast<DictObject>();
 
     if (!dict->contains(CommonNames::Linearized))
         return LinearizationResult::NotLinearized;
 
     if (!dict->contains(CommonNames::L, CommonNames::H, CommonNames::O, CommonNames::E, CommonNames::N, CommonNames::T))
-        return LinearizationResult::Error;
+        return error("Malformed linearization dictionary");
 
     auto length_of_file = dict->get_value(CommonNames::L);
     auto hint_table = dict->get_value(CommonNames::H);
@@ -156,17 +157,13 @@ Parser::LinearizationResult Parser::initialize_linearization_dict()
         || !number_of_pages.has_u16()
         || !offset_of_main_xref_table.has_u32()
         || (!first_page.has<Empty>() && !first_page.has_u32())) {
-        return LinearizationResult::Error;
+        return error("Malformed linearization dictionary parameters");
     }
 
-    auto hint_table_object = hint_table.get<NonnullRefPtr<Object>>();
-    if (!hint_table_object->is_array())
-        return LinearizationResult::Error;
-
-    auto hint_table_array = object_cast<ArrayObject>(hint_table_object);
+    auto hint_table_array = hint_table.get<NonnullRefPtr<Object>>()->cast<ArrayObject>();
     auto hint_table_size = hint_table_array->size();
     if (hint_table_size != 2 && hint_table_size != 4)
-        return LinearizationResult::Error;
+        return error("Expected hint table to be of length 2 or 4");
 
     auto primary_hint_stream_offset = hint_table_array->at(0);
     auto primary_hint_stream_length = hint_table_array->at(1);
@@ -182,7 +179,7 @@ Parser::LinearizationResult Parser::initialize_linearization_dict()
         || !primary_hint_stream_length.has_u32()
         || (!overflow_hint_stream_offset.has<Empty>() && !overflow_hint_stream_offset.has_u32())
         || (!overflow_hint_stream_length.has<Empty>() && !overflow_hint_stream_length.has_u32())) {
-        return LinearizationResult::Error;
+        return error("Malformed hint stream");
     }
 
     m_linearization_dictionary = LinearizationDictionary {
@@ -201,20 +198,12 @@ Parser::LinearizationResult Parser::initialize_linearization_dict()
     return LinearizationResult::Linearized;
 }
 
-bool Parser::initialize_linearized_xref_table()
+PDFErrorOr<void> Parser::initialize_linearized_xref_table()
 {
     // The linearization parameter dictionary has just been parsed, and the xref table
     // comes immediately after it. We are in the correct spot.
-    if (!m_reader.matches("xref"))
-        return false;
-
-    m_xref_table = parse_xref_table();
-    if (!m_xref_table)
-        return false;
-
-    m_trailer = parse_file_trailer();
-    if (!m_trailer)
-        return false;
+    m_xref_table = TRY(parse_xref_table());
+    m_trailer = TRY(parse_file_trailer());
 
     // Also parse the main xref table and merge into the first-page xref table. Note
     // that we don't use the main xref table offset from the linearization dict because
@@ -222,14 +211,12 @@ bool Parser::initialize_linearized_xref_table()
     // index start and length? So it's much easier to do it this way.
     auto main_xref_table_offset = m_trailer->get_value(CommonNames::Prev).to_int();
     m_reader.move_to(main_xref_table_offset);
-    auto main_xref_table = parse_xref_table();
-    if (!main_xref_table)
-        return false;
-
-    return m_xref_table->merge(move(*main_xref_table));
+    auto main_xref_table = TRY(parse_xref_table());
+    TRY(m_xref_table->merge(move(*main_xref_table)));
+    return {};
 }
 
-bool Parser::initialize_hint_tables()
+PDFErrorOr<void> Parser::initialize_hint_tables()
 {
     auto linearization_dict = m_linearization_dictionary.value();
     auto primary_offset = linearization_dict.primary_hint_stream_offset;
@@ -238,23 +225,23 @@ bool Parser::initialize_hint_tables()
     auto parse_hint_table = [&](size_t offset) -> RefPtr<StreamObject> {
         m_reader.move_to(offset);
         auto stream_indirect_value = parse_indirect_value();
-        if (!stream_indirect_value)
+        if (stream_indirect_value.is_error())
             return {};
 
-        auto stream_value = stream_indirect_value->value();
+        auto stream_value = stream_indirect_value.value()->value();
         if (!stream_value.has<NonnullRefPtr<Object>>())
             return {};
 
         auto stream_object = stream_value.get<NonnullRefPtr<Object>>();
-        if (!stream_object->is_stream())
+        if (!stream_object->is<StreamObject>())
             return {};
 
-        return object_cast<StreamObject>(stream_object);
+        return stream_object->cast<StreamObject>();
     };
 
     auto primary_hint_stream = parse_hint_table(primary_offset);
     if (!primary_hint_stream)
-        return false;
+        return error("Invalid primary hint stream");
 
     RefPtr<StreamObject> overflow_hint_stream;
     if (overflow_offset != NumericLimits<u32>::max())
@@ -270,66 +257,49 @@ bool Parser::initialize_hint_tables()
 
         auto buffer_result = ByteBuffer::create_uninitialized(total_size);
         if (buffer_result.is_error())
-            return false;
+            return Error { Error::Type::Internal, "Failed to allocate hint stream buffer" };
         possible_merged_stream_buffer = buffer_result.release_value();
-        auto ok = !possible_merged_stream_buffer.try_append(primary_hint_stream->bytes()).is_error();
-        ok = ok && !possible_merged_stream_buffer.try_append(overflow_hint_stream->bytes()).is_error();
-        if (!ok)
-            return false;
+        MUST(possible_merged_stream_buffer.try_append(primary_hint_stream->bytes()));
+        MUST(possible_merged_stream_buffer.try_append(overflow_hint_stream->bytes()));
         hint_stream_bytes = possible_merged_stream_buffer.bytes();
     } else {
         hint_stream_bytes = primary_hint_stream->bytes();
     }
 
-    auto hint_table = parse_page_offset_hint_table(hint_stream_bytes);
-    if (!hint_table.has_value())
-        return false;
-
-    dbgln("hint table: {}", hint_table.value());
+    auto hint_table = TRY(parse_page_offset_hint_table(hint_stream_bytes));
+    auto hint_table_entries = parse_all_page_offset_hint_table_entries(hint_table, hint_stream_bytes);
 
-    auto hint_table_entries = parse_all_page_offset_hint_table_entries(hint_table.value(), hint_stream_bytes);
-    if (!hint_table_entries.has_value())
-        return false;
-
-    auto entries = hint_table_entries.value();
-    dbgln("hint table entries size: {}", entries.size());
-    for (auto& entry : entries)
-        dbgln("{}", entry);
-
-    return true;
+    // FIXME: Do something with the hint tables
+    return {};
 }
 
-bool Parser::initialize_non_linearized_xref_table()
+PDFErrorOr<void> Parser::initialize_non_linearized_xref_table()
 {
     m_reader.move_to(m_reader.bytes().size() - 1);
     if (!navigate_to_before_eof_marker())
-        return false;
+        return error("No EOF marker");
     if (!navigate_to_after_startxref())
-        return false;
-    if (m_reader.done())
-        return false;
+        return error("No xref");
 
     m_reader.set_reading_forwards();
     auto xref_offset_value = parse_number();
-    if (!xref_offset_value.has<int>())
-        return false;
-    auto xref_offset = xref_offset_value.get<int>();
+    if (xref_offset_value.is_error() || !xref_offset_value.value().has<int>())
+        return error("Invalid xref offset");
+    auto xref_offset = xref_offset_value.value().get<int>();
 
     m_reader.move_to(xref_offset);
-    m_xref_table = parse_xref_table();
-    if (!m_xref_table)
-        return false;
-    m_trailer = parse_file_trailer();
-    return m_trailer;
+    m_xref_table = TRY(parse_xref_table());
+    m_trailer = TRY(parse_file_trailer());
+    return {};
 }
 
-RefPtr<XRefTable> Parser::parse_xref_table()
+PDFErrorOr<NonnullRefPtr<XRefTable>> Parser::parse_xref_table()
 {
     if (!m_reader.matches("xref"))
-        return {};
+        return error("Expected \"xref\"");
     m_reader.move_by(4);
     if (!consume_eol())
-        return {};
+        return error("Expected newline after \"xref\"");
 
     auto table = adopt_ref(*new XRefTable());
 
@@ -339,25 +309,25 @@ RefPtr<XRefTable> Parser::parse_xref_table()
 
         Vector<XRefEntry> entries;
 
-        auto starting_index_value = parse_number();
+        auto starting_index_value = TRY(parse_number());
         auto starting_index = starting_index_value.get<int>();
-        auto object_count_value = parse_number();
+        auto object_count_value = TRY(parse_number());
         auto object_count = object_count_value.get<int>();
 
         for (int i = 0; i < object_count; i++) {
             auto offset_string = String(m_reader.bytes().slice(m_reader.offset(), 10));
             m_reader.move_by(10);
             if (!consume(' '))
-                return {};
+                return error("Malformed xref entry");
 
             auto generation_string = String(m_reader.bytes().slice(m_reader.offset(), 5));
             m_reader.move_by(5);
             if (!consume(' '))
-                return {};
+                return error("Malformed xref entry");
 
             auto letter = m_reader.read();
             if (letter != 'n' && letter != 'f')
-                return {};
+                return error("Malformed xref entry");
 
             // The line ending sequence can be one of the following:
             // SP CR, SP LF, or CR LF
@@ -365,10 +335,10 @@ RefPtr<XRefTable> Parser::parse_xref_table()
                 consume();
                 auto ch = consume();
                 if (ch != '\r' && ch != '\n')
-                    return {};
+                    return error("Malformed xref entry");
             } else {
                 if (!m_reader.matches("\r\n"))
-                    return {};
+                    return error("Malformed xref entry");
                 m_reader.move_by(2);
             }
 
@@ -382,35 +352,33 @@ RefPtr<XRefTable> Parser::parse_xref_table()
     }
 }
 
-RefPtr<DictObject> Parser::parse_file_trailer()
+PDFErrorOr<NonnullRefPtr<DictObject>> Parser::parse_file_trailer()
 {
     if (!m_reader.matches("trailer"))
-        return {};
+        return error("Expected \"trailer\" keyword");
     m_reader.move_by(7);
     consume_whitespace();
-    auto dict = parse_dict();
-    if (!dict)
-        return {};
+    auto dict = TRY(parse_dict());
 
     if (!m_reader.matches("startxref"))
-        return {};
+        return error("Expected \"startxref\"");
     m_reader.move_by(9);
     consume_whitespace();
 
     m_reader.move_until([&](auto) { return matches_eol(); });
     VERIFY(consume_eol());
     if (!m_reader.matches("%%EOF"))
-        return {};
+        return error("Expected \"%%EOF\"");
 
     m_reader.move_by(5);
     consume_whitespace();
     return dict;
 }
 
-Optional<Parser::PageOffsetHintTable> Parser::parse_page_offset_hint_table(ReadonlyBytes hint_stream_bytes)
+PDFErrorOr<Parser::PageOffsetHintTable> Parser::parse_page_offset_hint_table(ReadonlyBytes hint_stream_bytes)
 {
     if (hint_stream_bytes.size() < sizeof(PageOffsetHintTable))
-        return {};
+        return error("Hint stream is too small");
 
     size_t offset = 0;
 
@@ -455,12 +423,10 @@ Optional<Parser::PageOffsetHintTable> Parser::parse_page_offset_hint_table(Reado
     return hint_table;
 }
 
-Optional<Vector<Parser::PageOffsetHintTableEntry>> Parser::parse_all_page_offset_hint_table_entries(PageOffsetHintTable const& hint_table, ReadonlyBytes hint_stream_bytes)
+Vector<Parser::PageOffsetHintTableEntry> Parser::parse_all_page_offset_hint_table_entries(PageOffsetHintTable const& hint_table, ReadonlyBytes hint_stream_bytes)
 {
     InputMemoryStream input_stream(hint_stream_bytes);
     input_stream.seek(sizeof(PageOffsetHintTable));
-    if (input_stream.has_any_error())
-        return {};
 
     InputBitStream bit_stream(input_stream);
 
@@ -576,7 +542,7 @@ String Parser::parse_comment()
     return str;
 }
 
-Value Parser::parse_value()
+PDFErrorOr<Value> Parser::parse_value()
 {
     parse_comment();
 
@@ -602,14 +568,12 @@ Value Parser::parse_value()
         return parse_possible_indirect_value_or_ref();
 
     if (m_reader.matches('/'))
-        return parse_name();
+        return MUST(parse_name());
 
     if (m_reader.matches("<<")) {
-        auto dict = parse_dict();
-        if (!dict)
-            return {};
+        auto dict = TRY(parse_dict());
         if (m_reader.matches("stream"))
-            return parse_stream(dict.release_nonnull());
+            return TRY(parse_stream(dict));
         return dict;
     }
 
@@ -617,21 +581,20 @@ Value Parser::parse_value()
         return parse_string();
 
     if (m_reader.matches('['))
-        return parse_array();
+        return TRY(parse_array());
 
-    dbgln("tried to parse value, but found char {} ({}) at offset {}", m_reader.peek(), static_cast<u8>(m_reader.peek()), m_reader.offset());
-    VERIFY_NOT_REACHED();
+    return error(String::formatted("Unexpected char \"{}\"", m_reader.peek()));
 }
 
-Value Parser::parse_possible_indirect_value_or_ref()
+PDFErrorOr<Value> Parser::parse_possible_indirect_value_or_ref()
 {
-    auto first_number = parse_number();
-    if (!first_number.has<int>() || !matches_number())
+    auto first_number = TRY(parse_number());
+    if (!matches_number())
         return first_number;
 
     m_reader.save();
     auto second_number = parse_number();
-    if (!second_number.has<int>()) {
+    if (second_number.is_error()) {
         m_reader.load();
         return first_number;
     }
@@ -640,28 +603,28 @@ Value Parser::parse_possible_indirect_value_or_ref()
         m_reader.discard();
         consume();
         consume_whitespace();
-        return Value(Reference(first_number.get<int>(), second_number.get<int>()));
+        return Value(Reference(first_number.get<int>(), second_number.value().get<int>()));
     }
 
     if (m_reader.matches("obj")) {
         m_reader.discard();
-        return parse_indirect_value(first_number.get<int>(), second_number.get<int>());
+        return TRY(parse_indirect_value(first_number.get<int>(), second_number.value().get<int>()));
     }
 
     m_reader.load();
     return first_number;
 }
 
-RefPtr<IndirectValue> Parser::parse_indirect_value(int index, int generation)
+PDFErrorOr<NonnullRefPtr<IndirectValue>> Parser::parse_indirect_value(int index, int generation)
 {
     if (!m_reader.matches("obj"))
-        return {};
+        return error("Expected \"obj\" at beginning of indirect value");
     m_reader.move_by(3);
     if (matches_eol())
         consume_eol();
-    auto value = parse_value();
+    auto value = TRY(parse_value());
     if (!m_reader.matches("endobj"))
-        return {};
+        return error("Expected \"endobj\" at end of indirect value");
 
     consume(6);
     consume_whitespace();
@@ -669,21 +632,18 @@ RefPtr<IndirectValue> Parser::parse_indirect_value(int index, int generation)
     return make_object<IndirectValue>(index, generation, value);
 }
 
-RefPtr<IndirectValue> Parser::parse_indirect_value()
+PDFErrorOr<NonnullRefPtr<IndirectValue>> Parser::parse_indirect_value()
 {
-    auto first_number = parse_number();
-    if (!first_number.has<int>())
-        return {};
-    auto second_number = parse_number();
-    if (!second_number.has<int>())
-        return {};
+    auto first_number = TRY(parse_number());
+    auto second_number = TRY(parse_number());
     return parse_indirect_value(first_number.get<int>(), second_number.get<int>());
 }
 
-Value Parser::parse_number()
+PDFErrorOr<Value> Parser::parse_number()
 {
     size_t start_offset = m_reader.offset();
     bool is_float = false;
+    bool consumed_digit = false;
 
     if (m_reader.matches('+') || m_reader.matches('-'))
         consume();
@@ -696,11 +656,15 @@ Value Parser::parse_number()
             consume();
         } else if (isdigit(m_reader.peek())) {
             consume();
+            consumed_digit = true;
         } else {
             break;
         }
     }
 
+    if (!consumed_digit)
+        return error("Invalid number");
+
     consume_whitespace();
 
     auto string = String(m_reader.bytes().slice(start_offset, m_reader.offset() - start_offset));
@@ -712,10 +676,11 @@ Value Parser::parse_number()
     return Value(static_cast<int>(f));
 }
 
-RefPtr<NameObject> Parser::parse_name()
+PDFErrorOr<NonnullRefPtr<NameObject>> Parser::parse_name()
 {
     if (!consume('/'))
-        return {};
+        return error("Expected Name object to start with \"/\"");
+
     StringBuilder builder;
 
     while (true) {
@@ -726,8 +691,7 @@ RefPtr<NameObject> Parser::parse_name()
             int hex_value = 0;
             for (int i = 0; i < 2; i++) {
                 auto ch = consume();
-                if (!isxdigit(ch))
-                    return {};
+                VERIFY(isxdigit(ch));
                 hex_value *= 16;
                 if (ch <= '9') {
                     hex_value += ch - '0';
@@ -747,7 +711,7 @@ RefPtr<NameObject> Parser::parse_name()
     return make_object<NameObject>(builder.to_string());
 }
 
-RefPtr<StringObject> Parser::parse_string()
+NonnullRefPtr<StringObject> Parser::parse_string()
 {
     ScopeGuard guard([&] { consume_whitespace(); });
 
@@ -762,8 +726,7 @@ RefPtr<StringObject> Parser::parse_string()
         is_binary_string = true;
     }
 
-    if (string.is_null())
-        return {};
+    VERIFY(!string.is_null());
 
     if (string.bytes().starts_with(Array<u8, 2> { 0xfe, 0xff })) {
         // The string is encoded in UTF16-BE
@@ -779,8 +742,7 @@ RefPtr<StringObject> Parser::parse_string()
 
 String Parser::parse_literal_string()
 {
-    if (!consume('('))
-        return {};
+    VERIFY(consume('('));
     StringBuilder builder;
     auto opened_parens = 0;
 
@@ -853,16 +815,13 @@ String Parser::parse_literal_string()
         }
     }
 
-    if (opened_parens != 0)
-        return {};
-
     return builder.to_string();
 }
 
 String Parser::parse_hex_string()
 {
-    if (!consume('<'))
-        return {};
+    VERIFY(consume('<'));
+
     StringBuilder builder;
 
     while (true) {
@@ -883,8 +842,7 @@ String Parser::parse_hex_string()
                     return builder.to_string();
                 }
 
-                if (!isxdigit(ch))
-                    return {};
+                VERIFY(isxdigit(ch));
 
                 hex_value *= 16;
                 if (ch <= '9') {
@@ -899,122 +857,101 @@ String Parser::parse_hex_string()
     }
 }
 
-RefPtr<ArrayObject> Parser::parse_array()
+PDFErrorOr<NonnullRefPtr<ArrayObject>> Parser::parse_array()
 {
     if (!consume('['))
-        return {};
+        return error("Expected array to start with \"[\"");
     consume_whitespace();
     Vector<Value> values;
 
-    while (!m_reader.matches(']')) {
-        auto value = parse_value();
-        if (value.has<Empty>())
-            return {};
-        values.append(value);
-    }
+    while (!m_reader.matches(']'))
+        values.append(TRY(parse_value()));
 
-    if (!consume(']'))
-        return {};
+    VERIFY(consume(']'));
     consume_whitespace();
 
     return make_object<ArrayObject>(values);
 }
 
-RefPtr<DictObject> Parser::parse_dict()
+PDFErrorOr<NonnullRefPtr<DictObject>> Parser::parse_dict()
 {
     if (!consume('<') || !consume('<'))
-        return {};
+        return error("Expected dict to start with \"<<\"");
+
     consume_whitespace();
     HashMap<FlyString, Value> map;
 
-    while (true) {
+    while (!m_reader.done()) {
         if (m_reader.matches(">>"))
             break;
-        auto name = parse_name();
-        if (!name)
-            return {};
-        auto value = parse_value();
-        if (value.has<Empty>())
-            return {};
-        map.set(name->name(), value);
+        auto name = TRY(parse_name())->name();
+        auto value = TRY(parse_value());
+        map.set(name, value);
     }
 
     if (!consume('>') || !consume('>'))
-        return {};
+        return error("Expected dict to end with \">>\"");
     consume_whitespace();
 
     return make_object<DictObject>(map);
 }
 
-RefPtr<DictObject> Parser::conditionally_parse_page_tree_node(u32 object_index, bool& ok)
+PDFErrorOr<RefPtr<DictObject>> Parser::conditionally_parse_page_tree_node(u32 object_index)
 {
-    ok = true;
-
     VERIFY(m_xref_table->has_object(object_index));
     auto byte_offset = m_xref_table->byte_offset_for_object(object_index);
 
     m_reader.move_to(byte_offset);
-    parse_number();
-    parse_number();
-    if (!m_reader.matches("obj")) {
-        ok = false;
-        return {};
-    }
+    TRY(parse_number());
+    TRY(parse_number());
+    if (!m_reader.matches("obj"))
+        return error(String::formatted("Invalid page tree offset {}", object_index));
 
     m_reader.move_by(3);
     consume_whitespace();
 
-    if (!consume('<') || !consume('<'))
-        return {};
+    VERIFY(consume('<') && consume('<'));
+
     consume_whitespace();
     HashMap<FlyString, Value> map;
 
     while (true) {
         if (m_reader.matches(">>"))
             break;
-        auto name = parse_name();
-        if (!name) {
-            ok = false;
-            return {};
-        }
-
+        auto name = TRY(parse_name());
         auto name_string = name->name();
         if (!name_string.is_one_of(CommonNames::Type, CommonNames::Parent, CommonNames::Kids, CommonNames::Count)) {
             // This is a page, not a page tree node
-            return {};
-        }
-        auto value = parse_value();
-        if (value.has<Empty>()) {
-            ok = false;
-            return {};
+            return RefPtr<DictObject> {};
         }
+
+        auto value = TRY(parse_value());
         if (name_string == CommonNames::Type) {
             if (!value.has<NonnullRefPtr<Object>>())
-                return {};
+                return RefPtr<DictObject> {};
             auto type_object = value.get<NonnullRefPtr<Object>>();
-            if (!type_object->is_name())
-                return {};
-            auto type_name = object_cast<NameObject>(type_object);
+            if (!type_object->is<NameObject>())
+                return RefPtr<DictObject> {};
+            auto type_name = type_object->cast<NameObject>();
             if (type_name->name() != CommonNames::Pages)
-                return {};
+                return RefPtr<DictObject> {};
         }
         map.set(name->name(), value);
     }
 
-    if (!consume('>') || !consume('>'))
-        return {};
+    VERIFY(consume('>') && consume('>'));
     consume_whitespace();
 
     return make_object<DictObject>(map);
 }
 
-RefPtr<StreamObject> Parser::parse_stream(NonnullRefPtr<DictObject> dict)
+PDFErrorOr<NonnullRefPtr<StreamObject>> Parser::parse_stream(NonnullRefPtr<DictObject> dict)
 {
     if (!m_reader.matches("stream"))
-        return {};
+        return error("Expected stream to start with \"stream\"");
     m_reader.move_by(6);
     if (!consume_eol())
-        return {};
+        return error("Expected \"stream\" to be followed by a newline");
 
     ReadonlyBytes bytes;
 
@@ -1022,7 +959,7 @@ RefPtr<StreamObject> Parser::parse_stream(NonnullRefPtr<DictObject> dict)
     if (maybe_length.has_value() && (!maybe_length->has<Reference>() || m_xref_table)) {
         // The PDF writer has kindly provided us with the direct length of the stream
         m_reader.save();
-        auto length = m_document->resolve_to<int>(maybe_length.value());
+        auto length = TRY(m_document->resolve_to<int>(maybe_length.value()));
         m_reader.load();
         bytes = m_reader.bytes().slice(m_reader.offset(), length);
         m_reader.move_by(length);
@@ -1047,11 +984,11 @@ RefPtr<StreamObject> Parser::parse_stream(NonnullRefPtr<DictObject> dict)
     consume_whitespace();
 
     if (dict->contains(CommonNames::Filter)) {
-        auto filter_type = dict->get_name(m_document, CommonNames::Filter)->name();
+        auto filter_type = MUST(dict->get_name(m_document, CommonNames::Filter))->name();
         auto maybe_bytes = Filter::decode(bytes, filter_type);
         if (maybe_bytes.is_error()) {
             warnln("Failed to decode filter: {}", maybe_bytes.error().string_literal());
-            return {};
+            return error(String::formatted("Failed to decode filter {}", maybe_bytes.error().string_literal()));
         }
         return make_object<EncodedStreamObject>(dict, move(maybe_bytes.value()));
     }
@@ -1059,7 +996,7 @@ RefPtr<StreamObject> Parser::parse_stream(NonnullRefPtr<DictObject> dict)
     return make_object<PlainTextStreamObject>(dict, bytes);
 }
 
-Vector<Command> Parser::parse_graphics_commands()
+PDFErrorOr<Vector<Command>> Parser::parse_graphics_commands()
 {
     Vector<Command> commands;
     Vector<Value> command_args;
@@ -1088,7 +1025,7 @@ Vector<Command> Parser::parse_graphics_commands()
             continue;
         }
 
-        command_args.append(parse_value());
+        command_args.append(TRY(parse_value()));
     }
 
     return commands;
@@ -1161,6 +1098,21 @@ bool Parser::consume(char ch)
     return consume() == ch;
 }
 
+Error Parser::error(
+    String const& message
+#ifdef PDF_DEBUG
+    ,
+    SourceLocation loc
+#endif
+) const
+{
+#ifdef PDF_DEBUG
+    dbgln("\033[31m{} Parser error at offset {}: {}\033[0m", loc, m_reader.offset(), message);
+#endif
+
+    return Error { Error::Type::Parse, message };
+}
+
 }
 
 namespace AK {

+ 34 - 28
Userland/Libraries/LibPDF/Parser.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/NonnullRefPtrVector.h>
+#include <AK/SourceLocation.h>
 #include <AK/WeakPtr.h>
 #include <LibPDF/Command.h>
 #include <LibPDF/Object.h>
@@ -20,12 +21,11 @@ class Document;
 class Parser final : public RefCounted<Parser> {
 public:
     enum class LinearizationResult {
-        Error,
         NotLinearized,
         Linearized,
     };
 
-    static Vector<Command> parse_graphics_commands(ReadonlyBytes);
+    static PDFErrorOr<Vector<Command>> parse_graphics_commands(ReadonlyBytes);
 
     Parser(Badge<Document>, ReadonlyBytes);
 
@@ -33,15 +33,13 @@ public:
     void set_document(WeakPtr<Document> const&);
 
     // Parses the header and initializes the xref table and trailer
-    bool initialize();
+    PDFErrorOr<void> initialize();
 
-    Value parse_object_with_index(u32 index);
+    PDFErrorOr<Value> parse_object_with_index(u32 index);
 
     // Specialized version of parse_dict which aborts early if the dict being parsed
-    // is not a page object. A null RefPtr return indicates that the dict at this index
-    // is not a page tree node, whereas ok == false indicates a malformed PDF file and
-    // should cause an abort of the current operation.
-    RefPtr<DictObject> conditionally_parse_page_tree_node(u32 object_index, bool& ok);
+    // is not a page object
+    PDFErrorOr<RefPtr<DictObject>> conditionally_parse_page_tree_node(u32 object_index);
 
 private:
     struct LinearizationDictionary {
@@ -89,35 +87,35 @@ private:
 
     explicit Parser(ReadonlyBytes);
 
-    bool parse_header();
-    LinearizationResult initialize_linearization_dict();
-    bool initialize_linearized_xref_table();
-    bool initialize_non_linearized_xref_table();
-    bool initialize_hint_tables();
-    Optional<PageOffsetHintTable> parse_page_offset_hint_table(ReadonlyBytes hint_stream_bytes);
-    Optional<Vector<PageOffsetHintTableEntry>> parse_all_page_offset_hint_table_entries(PageOffsetHintTable const&, ReadonlyBytes hint_stream_bytes);
-    RefPtr<XRefTable> parse_xref_table();
-    RefPtr<DictObject> parse_file_trailer();
+    PDFErrorOr<void> parse_header();
+    PDFErrorOr<LinearizationResult> initialize_linearization_dict();
+    PDFErrorOr<void> initialize_linearized_xref_table();
+    PDFErrorOr<void> initialize_non_linearized_xref_table();
+    PDFErrorOr<void> initialize_hint_tables();
+    PDFErrorOr<PageOffsetHintTable> parse_page_offset_hint_table(ReadonlyBytes hint_stream_bytes);
+    Vector<PageOffsetHintTableEntry> parse_all_page_offset_hint_table_entries(PageOffsetHintTable const&, ReadonlyBytes hint_stream_bytes);
+    PDFErrorOr<NonnullRefPtr<XRefTable>> parse_xref_table();
+    PDFErrorOr<NonnullRefPtr<DictObject>> parse_file_trailer();
 
     bool navigate_to_before_eof_marker();
     bool navigate_to_after_startxref();
 
     String parse_comment();
 
-    Value parse_value();
-    Value parse_possible_indirect_value_or_ref();
-    RefPtr<IndirectValue> parse_indirect_value(int index, int generation);
-    RefPtr<IndirectValue> parse_indirect_value();
-    Value parse_number();
-    RefPtr<NameObject> parse_name();
-    RefPtr<StringObject> parse_string();
+    PDFErrorOr<Value> parse_value();
+    PDFErrorOr<Value> parse_possible_indirect_value_or_ref();
+    PDFErrorOr<NonnullRefPtr<IndirectValue>> parse_indirect_value(int index, int generation);
+    PDFErrorOr<NonnullRefPtr<IndirectValue>> parse_indirect_value();
+    PDFErrorOr<Value> parse_number();
+    PDFErrorOr<NonnullRefPtr<NameObject>> parse_name();
+    NonnullRefPtr<StringObject> parse_string();
     String parse_literal_string();
     String parse_hex_string();
-    RefPtr<ArrayObject> parse_array();
-    RefPtr<DictObject> parse_dict();
-    RefPtr<StreamObject> parse_stream(NonnullRefPtr<DictObject> dict);
+    PDFErrorOr<NonnullRefPtr<ArrayObject>> parse_array();
+    PDFErrorOr<NonnullRefPtr<DictObject>> parse_dict();
+    PDFErrorOr<NonnullRefPtr<StreamObject>> parse_stream(NonnullRefPtr<DictObject> dict);
 
-    Vector<Command> parse_graphics_commands();
+    PDFErrorOr<Vector<Command>> parse_graphics_commands();
 
     bool matches_eol() const;
     bool matches_whitespace() const;
@@ -131,6 +129,14 @@ private:
     void consume(int amount);
     bool consume(char);
 
+    Error error(
+        String const& message
+#ifdef PDF_DEBUG
+        ,
+        SourceLocation loc = SourceLocation::current()
+#endif
+    ) const;
+
     Reader m_reader;
     WeakPtr<Document> m_document;
     RefPtr<XRefTable> m_xref_table;

+ 22 - 25
Userland/Libraries/LibPDF/Renderer.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -65,19 +65,18 @@ void Renderer::render()
     // as one stream or multiple?
     ByteBuffer byte_buffer;
 
-    if (m_page.contents->is_array()) {
-        auto contents = object_cast<ArrayObject>(m_page.contents);
+    if (m_page.contents->is<ArrayObject>()) {
+        auto contents = m_page.contents->cast<ArrayObject>();
         for (auto& ref : *contents) {
-            auto bytes = m_document->resolve_to<StreamObject>(ref)->bytes();
+            auto bytes = MUST(m_document->resolve_to<StreamObject>(ref))->bytes();
             byte_buffer.append(bytes.data(), bytes.size());
         }
     } else {
-        VERIFY(m_page.contents->is_stream());
-        auto bytes = object_cast<StreamObject>(m_page.contents)->bytes();
+        auto bytes = m_page.contents->cast<StreamObject>()->bytes();
         byte_buffer.append(bytes.data(), bytes.size());
     }
 
-    auto commands = Parser::parse_graphics_commands(byte_buffer);
+    auto commands = MUST(Parser::parse_graphics_commands(byte_buffer));
 
     for (auto& command : commands)
         handle_command(command);
@@ -147,7 +146,7 @@ RENDERER_HANDLER(set_miter_limit)
 
 RENDERER_HANDLER(set_dash_pattern)
 {
-    auto dash_array = m_document->resolve_to<ArrayObject>(args[0]);
+    auto dash_array = MUST(m_document->resolve_to<ArrayObject>(args[0]));
     Vector<int> pattern;
     for (auto& element : *dash_array)
         pattern.append(element.get<int>());
@@ -160,9 +159,9 @@ RENDERER_TODO(set_flatness_tolerance);
 RENDERER_HANDLER(set_graphics_state_from_dict)
 {
     VERIFY(m_page.resources->contains(CommonNames::ExtGState));
-    auto dict_name = m_document->resolve_to<NameObject>(args[0])->name();
-    auto ext_gstate_dict = m_page.resources->get_dict(m_document, CommonNames::ExtGState);
-    auto target_dict = ext_gstate_dict->get_dict(m_document, dict_name);
+    auto dict_name = MUST(m_document->resolve_to<NameObject>(args[0]))->name();
+    auto ext_gstate_dict = MUST(m_page.resources->get_dict(m_document, CommonNames::ExtGState));
+    auto target_dict = MUST(ext_gstate_dict->get_dict(m_document, dict_name));
     set_graphics_state_from_dict(target_dict);
 }
 
@@ -303,14 +302,14 @@ RENDERER_HANDLER(text_set_leading)
 
 RENDERER_HANDLER(text_set_font)
 {
-    auto target_font_name = m_document->resolve_to<NameObject>(args[0])->name();
-    auto fonts_dictionary = m_page.resources->get_dict(m_document, CommonNames::Font);
-    auto font_dictionary = fonts_dictionary->get_dict(m_document, target_font_name);
+    auto target_font_name = MUST(m_document->resolve_to<NameObject>(args[0]))->name();
+    auto fonts_dictionary = MUST(m_page.resources->get_dict(m_document, CommonNames::Font));
+    auto font_dictionary = MUST(fonts_dictionary->get_dict(m_document, target_font_name));
 
     // FIXME: We do not yet have the standard 14 fonts, as some of them are not open fonts,
     // so we just use LiberationSerif for everything
 
-    auto font_name = font_dictionary->get_name(m_document, CommonNames::BaseFont)->name().to_lowercase();
+    auto font_name = MUST(font_dictionary->get_name(m_document, CommonNames::BaseFont))->name().to_lowercase();
     auto font_view = font_name.view();
     bool is_bold = font_view.contains("bold");
     bool is_italic = font_view.contains("italic");
@@ -380,7 +379,7 @@ RENDERER_HANDLER(text_next_line)
 
 RENDERER_HANDLER(text_show_string)
 {
-    auto text = m_document->resolve_to<StringObject>(args[0])->string();
+    auto text = MUST(m_document->resolve_to<StringObject>(args[0]))->string();
     show_text(text);
 }
 
@@ -394,7 +393,7 @@ RENDERER_TODO(text_next_line_show_string_set_spacing);
 
 RENDERER_HANDLER(text_show_string_array)
 {
-    auto elements = m_document->resolve_to<ArrayObject>(args[0])->elements();
+    auto elements = MUST(m_document->resolve_to<ArrayObject>(args[0]))->elements();
     float next_shift = 0.0f;
 
     for (auto& element : elements) {
@@ -403,9 +402,7 @@ RENDERER_HANDLER(text_show_string_array)
         } else if (element.has<float>()) {
             next_shift = element.get<float>();
         } else {
-            auto& obj = element.get<NonnullRefPtr<Object>>();
-            VERIFY(obj->is_string());
-            auto str = object_cast<StringObject>(obj)->string();
+            auto str = element.get<NonnullRefPtr<Object>>()->cast<StringObject>()->string();
             show_text(str, next_shift);
         }
     }
@@ -523,7 +520,7 @@ void Renderer::set_graphics_state_from_dict(NonnullRefPtr<DictObject> dict)
         handle_set_miter_limit({ dict->get_value(CommonNames::ML) });
 
     if (dict->contains(CommonNames::D))
-        handle_set_dash_pattern(dict->get_array(m_document, CommonNames::D)->elements());
+        handle_set_dash_pattern(MUST(dict->get_array(m_document, CommonNames::D))->elements());
 
     if (dict->contains(CommonNames::FL))
         handle_set_flatness_tolerance({ dict->get_value(CommonNames::FL) });
@@ -569,7 +566,7 @@ void Renderer::show_text(String const& string, float shift)
 
 RefPtr<ColorSpace> Renderer::get_color_space(Value const& value)
 {
-    auto name = object_cast<NameObject>(value.get<NonnullRefPtr<Object>>())->name();
+    auto name = value.get<NonnullRefPtr<Object>>()->cast<NameObject>()->name();
 
     // Simple color spaces with no parameters, which can be specified directly
     if (name == CommonNames::DeviceGray)
@@ -583,12 +580,12 @@ RefPtr<ColorSpace> Renderer::get_color_space(Value const& value)
 
     // The color space is a complex color space with parameters that resides in
     // the resource dictionary
-    auto color_space_resource_dict = m_page.resources->get_dict(m_document, CommonNames::ColorSpace);
+    auto color_space_resource_dict = MUST(m_page.resources->get_dict(m_document, CommonNames::ColorSpace));
     if (!color_space_resource_dict->contains(name))
         TODO();
 
-    auto color_space_array = color_space_resource_dict->get_array(m_document, name);
-    name = color_space_array->get_name_at(m_document, 0)->name();
+    auto color_space_array = MUST(color_space_resource_dict->get_array(m_document, name));
+    name = MUST(color_space_array->get_name_at(m_document, 0))->name();
 
     Vector<Value> parameters;
     parameters.ensure_capacity(color_space_array->size() - 1);

+ 8 - 1
Userland/Libraries/LibPDF/Value.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -28,6 +28,13 @@ public:
             set<NonnullRefPtr<Object>>(*refptr);
     }
 
+    template<IsObject T>
+    Value(NonnullRefPtr<T> const& refptr) requires(!IsSame<Object, T>)
+        : Variant(nullptr)
+    {
+        set<NonnullRefPtr<Object>>(*refptr);
+    }
+
     [[nodiscard]] String to_string(int indent = 0) const;
 
     [[nodiscard]] ALWAYS_INLINE bool has_number() const { return has<int>() || has<float>(); }

+ 4 - 4
Userland/Libraries/LibPDF/XRefTable.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2021-2022, Matthew Olsson <mattco@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -29,7 +29,7 @@ struct XRefSection {
 
 class XRefTable final : public RefCounted<XRefTable> {
 public:
-    bool merge(XRefTable&& other)
+    PDFErrorOr<void> merge(XRefTable&& other)
     {
         auto this_size = m_entries.size();
         auto other_size = other.m_entries.size();
@@ -48,11 +48,11 @@ public:
                 m_entries[i] = other_entry;
             } else if (other_entry.byte_offset != invalid_byte_offset) {
                 // Both xref tables have an entry for the same object index
-                return false;
+                return Error { Error::Type::Parse, "Conflicting xref entry during merge" };
             }
         }
 
-        return true;
+        return {};
     }
 
     void add_section(XRefSection const& section)