From 7414abb01674b7431e4929b30cb8851224120bb6 Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Wed, 22 Jun 2022 14:05:52 +1200 Subject: [PATCH] scriptindex: Fix weird error cases In four cases while handling input data (two cases of bad hex data fed to `hextobin`, an input data line without a `=`, and `load` failing to load the specified file) we'd emit a diagnostic that was labelled as an "error" but really it was handled as a warning as we kept reading input and the "error" didn't affect the exit status. It doesn't seem to really make sense to continue in any of these cases so we now exit with non-zero status right away. (cherry picked from commit 5a6098d4cc37e172753d4fd2f2f0637f7e8f3009) --- xapian-applications/omega/docs/scriptindex.rst | 14 +++++---- xapian-applications/omega/omegatest | 18 ++++------- xapian-applications/omega/scriptindex.cc | 41 ++++++++++++++------------ 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/xapian-applications/omega/docs/scriptindex.rst b/xapian-applications/omega/docs/scriptindex.rst index 3989836e2..9ed9dd9b5 100644 --- a/xapian-applications/omega/docs/scriptindex.rst +++ b/xapian-applications/omega/docs/scriptindex.rst @@ -91,11 +91,13 @@ hash[=LENGTH] hextobin converts pairs of hex digits to binary byte values (providing a way to specify arbitrary binary strings e.g. for use in a document value - slot). The input should have an even length and be composed entirely - of hex digits (if it isn't, an error is reported and the value is - unchanged). + slot). The input must have an even length and be composed entirely + of hex digits (if it isn't, an error is reported). - ``hextobin`` was added in Omega 1.4.6. + ``hextobin`` was added in Omega 1.4.6. Prior to Omega 1.4.20, the + "error" on a bad value was really handled like a warning - it didn't + cause Omega to exit with non-zero status, instead the value was + passed on unchanged. index[=PREFIX] split text into words and index (with prefix PREFIX if specified). @@ -110,7 +112,9 @@ load and then sets the current text to the contents. If the current text is empty, a warning is issued (since Xapian 1.4.10). If the file can't be loaded (not found, wrong permissions, etc) then an error is issued and - the current text is set to empty. + scriptindex exits (prior to Omega 1.4.20 this "error" was really handled + as a warning - scriptindex continued with the current text set to empty, + and the final exit status wasn't affected). If the next action is ``truncate``, then scriptindex is smart enough to know it only needs to load the start of a large file. diff --git a/xapian-applications/omega/omegatest b/xapian-applications/omega/omegatest index 32115da15..d9179c6fb 100755 --- a/xapian-applications/omega/omegatest +++ b/xapian-applications/omega/omegatest @@ -1021,18 +1021,12 @@ printf '$list{$map{$split{$cgi{DOCIDS}},$value{0,$_}},|}' > "$TEST_TEMPLATE" testcase '|A|Test|Kill' DOCIDS='1 2 3 4' # Feature test error cases for scriptindex `hextobin` action. -rm -rf "$TEST_DB" -echo hex=7g |\ - $SCRIPTINDEX "$TEST_DB" "$TEST_INDEXSCRIPT" 2>&1 > /dev/null |\ - grep -q ":1: error: hextobin: input must be all hex digits" ||\ - { echo "scriptindex hextobin didn't give error for bad hex digit";\ - failed=`expr $failed + 1`; } -echo hex=404 |\ - $SCRIPTINDEX "$TEST_DB" "$TEST_INDEXSCRIPT" 2>&1 > /dev/null |\ - grep -q ":1: error: hextobin: input must have even length" ||\ - { echo "scriptindex hextobin didn't give error for odd length hex string";\ - failed=`expr $failed + 1`; } -testcase '7g|404' DOCIDS='1 2' +test_scriptindex_error 'bad hex digit' \ + ":1: error: hextobin: input must be all hex digits" \ + 'hex=7g' +test_scriptindex_error 'bad hex length' \ + ":1: error: hextobin: input must have even length" \ + 'hex=404' # Feature test for scriptindex `spell` action. printf '%s\n' 's : spell index' 'n : index' > "$TEST_INDEXSCRIPT" diff --git a/xapian-applications/omega/scriptindex.cc b/xapian-applications/omega/scriptindex.cc index 9c9595dd8..528f6e630 100644 --- a/xapian-applications/omega/scriptindex.cc +++ b/xapian-applications/omega/scriptindex.cc @@ -564,6 +564,11 @@ bad_hex_digit: i - s.begin()); cerr << "Unexpected character '" << *i << "' after closing quote" << endl; + do { + ++i; + } while (i != s.end() && *i != ',' && !C_isspace(*i)); + if (*i != ',') break; + ++i; } } else if (max_args > 1) { // Unquoted argument, split on comma. @@ -972,24 +977,23 @@ run_actions(vector::const_iterator action_it, report_location(DIAG_ERROR, fname, line_no); cerr << "hextobin: input must have even length" << endl; - } else { - string output; - output.reserve(len / 2); - for (size_t j = 0; j < len; j += 2) { - char a = value[j]; - char b = value[j + 1]; - if (!C_isxdigit(a) || !C_isxdigit(b)) { - report_location(DIAG_ERROR, fname, line_no); - cerr << "hextobin: input must be all hex " - "digits" << endl; - goto badhex; - } - char r = (hex_digit(a) << 4) | hex_digit(b); - output.push_back(r); + exit(1); + } + + string output; + output.reserve(len / 2); + for (size_t j = 0; j < len; j += 2) { + char a = value[j]; + char b = value[j + 1]; + if (!C_isxdigit(a) || !C_isxdigit(b)) { + report_location(DIAG_ERROR, fname, line_no); + cerr << "hextobin: input must be all hex digits\n"; + exit(1); } - value = std::move(output); + char r = (hex_digit(a) << 4) | hex_digit(b); + output.push_back(r); } -badhex: + value = std::move(output); break; } case Action::LOWER: @@ -1023,8 +1027,7 @@ badhex: report_location(DIAG_ERROR, fname, line_no); cerr << "Couldn't load file '" << filename << "': " << strerror(errno) << endl; - value.resize(0); - break; + exit(1); } if (!truncated) break; } @@ -1359,7 +1362,7 @@ index_file(const char *fname, istream &stream, if (eq == string::npos && !line.empty()) { report_location(DIAG_ERROR, fname, line_no, line.size()); cerr << "expected = somewhere in this line" << endl; - // FIXME: die or what? + exit(1); } string field(line, 0, eq); string value(line, eq + 1, string::npos); -- 2.11.4.GIT