From e2823965faa00a115ddc189576336d0bd7278ad0 Mon Sep 17 00:00:00 2001 From: m-holger Date: Thu, 27 Nov 2025 19:05:20 +0000 Subject: [PATCH] Refactor `limits_error` handling for improved clarity and consistency --- libqpdf/QPDFParser.cc | 57 +++++++++++++++++++++++++++++++++------------------------ libqpdf/qpdf/QPDFParser.hh | 1 + qpdf/qtest/qpdf/issue-146.out | 2 +- qpdf/qtest/qpdf/issue-202.out | 4 ++-- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/libqpdf/QPDFParser.cc b/libqpdf/QPDFParser.cc index 60ba7ab..464b0b5 100644 --- a/libqpdf/QPDFParser.cc +++ b/libqpdf/QPDFParser.cc @@ -437,18 +437,16 @@ QPDFParser::parseRemainder(bool content_stream) case QPDFTokenizer::tt_array_open: case QPDFTokenizer::tt_dict_open: if (stack.size() > max_nesting) { - global::Limits::error(); - warn("limits error: ignoring excessively deeply nested data structure"); - return {}; - } else { - b_contents = false; - stack.emplace_back( - input, - (tokenizer.getType() == QPDFTokenizer::tt_array_open) ? st_array - : st_dictionary_key); - frame = &stack.back(); - continue; + limits_error( + "parser-max-nesting", "ignoring excessively deeply nested data structure"); } + b_contents = false; + stack.emplace_back( + input, + (tokenizer.getType() == QPDFTokenizer::tt_array_open) ? st_array + : st_dictionary_key); + frame = &stack.back(); + continue; case QPDFTokenizer::tt_bool: addScalar(tokenizer.getValue() == "true"); @@ -588,10 +586,10 @@ void QPDFParser::addScalar(Args&&... args) { auto limit = Limits::parser_max_container_size(bad_count || sanity_checks); - if (frame->olist.size() > limit || frame->dict.size() > limit) { + if (frame->olist.size() >= limit || frame->dict.size() >= limit) { // Stop adding scalars. We are going to abort when the close token or a bad token is // encountered. - max_bad_count = 0; + max_bad_count = 1; check_too_many_bad_tokens(); // always throws Error() } auto obj = QPDFObject::create(std::forward(args)...); @@ -646,26 +644,29 @@ void QPDFParser::check_too_many_bad_tokens() { auto limit = Limits::parser_max_container_size(bad_count || sanity_checks); - if (frame->olist.size() > limit || frame->dict.size() > limit) { + if (frame->olist.size() >= limit || frame->dict.size() >= limit) { if (bad_count) { - Limits::error(); - warn( - "limits error: encountered errors while parsing an array or dictionary with more " - "than " + - std::to_string(limit) + " elements; giving up on reading object"); - throw Error(); + limits_error( + "parser-max-container-size-damaged", + "encountered errors while parsing an array or dictionary with more than " + + std::to_string(limit) + " elements; giving up on reading object"); } - warn( + limits_error( + "parser-max-container-size", "encountered an array or dictionary with more than " + std::to_string(limit) + - " elements during xref recovery; giving up on reading object"); + " elements during xref recovery; giving up on reading object"); } - if (max_bad_count && --max_bad_count > 0 && good_count > 4) { + if (max_bad_count && --max_bad_count == 0) { + limits_error( + "parser-max-errors", "too many errors during parsing; treating object as null"); + } + if (good_count > 4) { good_count = 0; bad_count = 1; return; } if (++bad_count > 5 || - (frame->state != st_array && QIntC::to_size(max_bad_count) < frame->olist.size())) { + (frame->state != st_array && std::cmp_less(max_bad_count, frame->olist.size()))) { // Give up after 5 errors in close proximity or if the number of missing dictionary keys // exceeds the remaining number of allowable total errors. warn("too many errors; giving up on reading object"); @@ -675,6 +676,14 @@ QPDFParser::check_too_many_bad_tokens() } void +QPDFParser::limits_error(std::string const& limit, std::string const& msg) +{ + Limits::error(); + warn("limits error("s + limit + "): " + msg); + throw Error(); +} + +void QPDFParser::warn(QPDFExc const& e) const { // If parsing on behalf of a QPDF object and want to give a warning, we can warn through the diff --git a/libqpdf/qpdf/QPDFParser.hh b/libqpdf/qpdf/QPDFParser.hh index 0fb7935..e108a20 100644 --- a/libqpdf/qpdf/QPDFParser.hh +++ b/libqpdf/qpdf/QPDFParser.hh @@ -124,6 +124,7 @@ class QPDFParser void check_too_many_bad_tokens(); void warnDuplicateKey(); void fixMissingKeys(); + [[noreturn]] void limits_error(std::string const& limit, std::string const& msg); void warn(qpdf_offset_t offset, std::string const& msg) const; void warn(std::string const& msg) const; void warn(QPDFExc const&) const; diff --git a/qpdf/qtest/qpdf/issue-146.out b/qpdf/qtest/qpdf/issue-146.out index 96dcede..5a2ad11 100644 --- a/qpdf/qtest/qpdf/issue-146.out +++ b/qpdf/qtest/qpdf/issue-146.out @@ -1,7 +1,7 @@ WARNING: issue-146.pdf: file is damaged WARNING: issue-146.pdf: can't find startxref WARNING: issue-146.pdf: Attempting to reconstruct cross-reference table -WARNING: issue-146.pdf (trailer, offset 695): limits error: ignoring excessively deeply nested data structure +WARNING: issue-146.pdf (trailer, offset 695): limits error(parser-max-nesting): ignoring excessively deeply nested data structure WARNING: issue-146.pdf (object 1 0, offset 92): expected endobj WARNING: issue-146.pdf (object 7 0, offset 146): unknown token while reading object; treating as null WARNING: issue-146.pdf (object 7 0, offset 168): expected endobj diff --git a/qpdf/qtest/qpdf/issue-202.out b/qpdf/qtest/qpdf/issue-202.out index e62f736..834743a 100644 --- a/qpdf/qtest/qpdf/issue-202.out +++ b/qpdf/qtest/qpdf/issue-202.out @@ -1,8 +1,8 @@ -WARNING: issue-202.pdf (trailer, offset 55770): limits error: ignoring excessively deeply nested data structure +WARNING: issue-202.pdf (trailer, offset 55770): limits error(parser-max-nesting): ignoring excessively deeply nested data structure WARNING: issue-202.pdf: file is damaged WARNING: issue-202.pdf (offset 54769): expected trailer dictionary WARNING: issue-202.pdf: Attempting to reconstruct cross-reference table -WARNING: issue-202.pdf (trailer, offset 55770): limits error: ignoring excessively deeply nested data structure +WARNING: issue-202.pdf (trailer, offset 55770): limits error(parser-max-nesting): ignoring excessively deeply nested data structure WARNING: issue-202.pdf (object 222 0, offset 50101): dictionary has duplicated key /Creator; last occurrence overrides earlier ones WARNING: issue-202.pdf (object 222 0, offset 50101): dictionary has duplicated key /Producer; last occurrence overrides earlier ones WARNING: issue-202.pdf: unable to find trailer dictionary while recovering damaged file -- libgit2 0.21.4