Bug #32050219 REGEXP_ENGINE::REPLACE DOESN'T RESET ERROR CODE AFTER PROCESSING A RECORD

Regexp_engine::Reset() failed to initialize the status variable
m_error_code.  This could lead to an internal ICU warning generated
for one row "spilling" over to the processing of subsequent rows.

In Regexp_engine::Replace(), test for U_SUCCESS rather than U_ZERO_ERROR,
i.e. ignore warnings for the 'not found' case.

Add unittest to verify that warnings are reset between regexp engine
executions.

Change-Id: Ifdf70c33976228e86c0393d90a88da037ac595db

Author: Tor Didriksen

sql/regexp/regexp_engine.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2017, 2020, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -41,6 +41,7 @@ UBool QueryNotKilled(const void *thd, int32_t) {
 const char *icu_version_string() { return U_ICU_VERSION; }
 
 void Regexp_engine::Reset(const std::u16string &subject) {
+  m_error_code = U_ZERO_ERROR;
   auto usubject = subject.data();
   int length = subject.size();
   uregex_setText(m_re, pointer_cast<const UChar *>(usubject), length,
@@ -82,7 +83,7 @@ const std::u16string &Regexp_engine::Replace(const std::u16string &replacement,
     call to uregex_appendReplacement() leads to ICU trying to free the buffer
     that we own, thus causing a double-delete.
   */
-  if (!found && m_error_code == U_ZERO_ERROR) return m_current_subject;
+  if (!found && U_SUCCESS(m_error_code)) return m_current_subject;
 
   m_replace_buffer.resize(std::min(m_current_subject.size(), HardLimit()));
 

sql/regexp/regexp_engine.h

@@ -1,7 +1,7 @@
 #ifndef SQL_REGEXP_REGEXP_ENGINE_H_
 #define SQL_REGEXP_REGEXP_ENGINE_H_
 
-/* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2017, 2020, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -176,6 +176,9 @@ class Regexp_engine {
   */
   std::pair<int, int> MatchedSubstring();
 
+  bool HasWarning() const {
+    return U_SUCCESS(m_error_code) && m_error_code != U_ZERO_ERROR;
+  }
   bool IsError() const { return U_FAILURE(m_error_code); }
   bool CheckError() const { return check_icu_status(m_error_code); }
 

sql/regexp/regexp_facade.h

@@ -125,6 +125,11 @@ class Regexp_facade {
   /// Delete the "engine" data structure after execution.
   void cleanup() { m_engine = nullptr; }
 
+  /// Did any operation return a warning? For unit testing.
+  bool EngineHasWarning() const {
+    return m_engine != nullptr && m_engine->HasWarning();
+  }
+
  private:
   /**
     Resets the compiled regular expression with a new string.

unittest/gunit/regexp_facade-t.cc

@@ -124,4 +124,21 @@ TEST_F(RegexpFacadeTest, SetPattern) {
   regex.SetPattern(nullptr, 0);
 }
 
+TEST_F(RegexpFacadeTest, Replace) {
+  StringBuffer<STRING_BUFFER_USUAL_SIZE> buf;
+  Item *subject1 = make_fixed_literal(thd(), " aaa");
+  Item *subject2 = make_fixed_literal(thd(), "");
+  Item *replacement = make_fixed_literal(thd(), "aamz");
+  const char *pattern = " +";
+  MockRegexpFacade regex(thd(), pattern);
+
+  regex.Replace(subject1, replacement, 1, 0, &buf);
+  // These arguments to Replace() will set U_STRING_NOT_TERMINATED_WARNING.
+  EXPECT_TRUE(regex.EngineHasWarning());
+
+  regex.Replace(subject2, replacement, 1, 0, &buf);
+  // The previous warning should have been cleared.
+  EXPECT_FALSE(regex.EngineHasWarning());
+}
+
 }  // namespace regexp_facade_unittest