diff options
| -rw-r--r-- | ChangeLog | 5 | ||||
| -rw-r--r-- | qpdf/qpdf.cc | 72 | ||||
| -rw-r--r-- | qpdf/qpdf.testcov | 1 | ||||
| -rw-r--r-- | qpdf/qtest/qpdf.test | 17 |
4 files changed, 71 insertions, 24 deletions
@@ -1,3 +1,8 @@ +2021-11-02 Jay Berkenbilt <ejb@ql.org> + + * Improve error reporting when someone forgets the -- after + --pages. Fixes #555. + 2021-05-12 Jay Berkenbilt <ejb@ql.org> * Bug fix: ensure we don't overflow any string bounds while diff --git a/qpdf/qpdf.cc b/qpdf/qpdf.cc index 7e450cf8..2ed03cff 100644 --- a/qpdf/qpdf.cc +++ b/qpdf/qpdf.cc @@ -738,6 +738,21 @@ static void parse_object_id(std::string const& objspec, } } +static bool file_exists(char const* filename) +{ + try + { + fclose(QUtil::safe_fopen(filename, "rb")); + return true; + } + catch (std::runtime_error&) + { + // can't open the file + } + return false; +} + + // This is not a general-purpose argument parser. It is tightly // crafted to work with qpdf. qpdf's command-line syntax is very // complex because of its long history, and it doesn't really follow @@ -2080,6 +2095,10 @@ void ArgParser::argPages() { ++cur_arg; + if (! o.page_specs.empty()) + { + usage("the --pages may only be specified one time"); + } o.page_specs = parsePagesOptions(); if (o.page_specs.empty()) { @@ -2937,19 +2956,15 @@ ArgParser::handleArgFileArguments() char* argfile = 0; if ((strlen(argv[i]) > 1) && (argv[i][0] == '@')) { - try + argfile = 1 + argv[i]; + if (strcmp(argfile, "-") != 0) { - argfile = 1 + argv[i]; - if (strcmp(argfile, "-") != 0) + if (! file_exists(argfile)) { - fclose(QUtil::safe_fopen(argfile, "rb")); + // The file's not there; treating as regular option + argfile = nullptr; } } - catch (std::runtime_error&) - { - // The file's not there; treating as regular option - argfile = 0; - } } if (argfile) { @@ -3220,6 +3235,16 @@ ArgParser::parseNumrange(char const* range, int max, bool throw_error) std::vector<PageSpec> ArgParser::parsePagesOptions() { + auto check_unclosed = [this](char const* arg, int n) { + if ((strlen(arg) > 0) && (arg[0] == '-')) + { + // A common error is to forget to close --pages with --, + // so catch this as special case + QTC::TC("qpdf", "check unclosed --pages", n); + usage("the --pages option must be terminated with -- by itself"); + } + }; + std::vector<PageSpec> result; while (1) { @@ -3234,6 +3259,10 @@ ArgParser::parsePagesOptions() char const* file = argv[cur_arg++]; char const* password = 0; char const* range = argv[cur_arg++]; + if (! file_exists(file)) + { + check_unclosed(file, 0); + } if (strncmp(range, "--password=", 11) == 0) { // Oh, that's the password, not the range @@ -3263,23 +3292,20 @@ ArgParser::parsePagesOptions() catch (std::runtime_error& e1) { // The range is invalid. Let's see if it's a file. - try + range_omitted = true; + if (strcmp(range, ".") == 0) { - if (strcmp(range, ".") == 0) - { - // "." means the input file. - QTC::TC("qpdf", "qpdf pages range omitted with ."); - } - else - { - fclose(QUtil::safe_fopen(range, "rb")); - QTC::TC("qpdf", "qpdf pages range omitted in middle"); - // Yup, it's a file. - } - range_omitted = true; + // "." means the input file. + QTC::TC("qpdf", "qpdf pages range omitted with ."); } - catch (std::runtime_error&) + else if (file_exists(range)) + { + QTC::TC("qpdf", "qpdf pages range omitted in middle"); + // Yup, it's a file. + } + else { + check_unclosed(range, 1); // Give the range error usage(e1.what()); } diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 9a15ae69..331547cd 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -594,3 +594,4 @@ qpdf copy fields non-first from orig 0 QPDF resolve duplicated page in insert 0 QPDFWriter preserve object streams 1 QPDFWriter exclude from object stream 0 +check unclosed --pages 1 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 6350c045..5c58c5a3 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -139,7 +139,7 @@ foreach my $c (@completion_tests) show_ntests(); # ---------- $td->notify("--- Argument Parsing ---"); -$n_tests += 6; +$n_tests += 9; $td->runtest("required argument", {$td->COMMAND => "qpdf --password minimal.pdf"}, @@ -171,6 +171,21 @@ $td->runtest("extra overlay filename", {$td->REGEXP => ".*overlay file already specified.*", $td->EXIT_STATUS => 2}, $td->NORMALIZE_NEWLINES); +$td->runtest("multiple pages options", + {$td->COMMAND => "qpdf --pages . -- --pages . --"}, + {$td->REGEXP => ".*--pages may only be specified one time.*", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("bad numeric range detects unclosed --pages", + {$td->COMMAND => "qpdf --pages . --pages . --"}, + {$td->REGEXP => ".*--pages option must be terminated with --.*", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("bad file detected as unclosed --pages", + {$td->COMMAND => "qpdf --pages . 1 --xyz out"}, + {$td->REGEXP => ".*--pages option must be terminated with --.*", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); show_ntests(); # ---------- |