You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2026/06/16 17:30:21 UTC

(impala) branch master updated: IMPALA-13870: Add operator<< for StringVal to prevent unsafe ptr usage

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new f7449b264 IMPALA-13870: Add operator<< for StringVal to prevent unsafe ptr usage
f7449b264 is described below

commit f7449b26477c6c7654ad9b7d03710332200f2a33
Author: Arnab Karmakar <ar...@gmail.com>
AuthorDate: Tue Jun 16 00:02:32 2026 -0700

    IMPALA-13870: Add operator<< for StringVal to prevent unsafe ptr usage
    
    StringVal buffers are not null-terminated, but manual string
    construction from ptr/len is error-prone. This change adds a
    length-safe stream operator that handles NULL values and
    non-terminated buffers correctly.
    
    The operator uses ostream::write(ptr, len) to ensure only len bytes
    are printed, never relying on null termination.
    
    Testing:
    - Added unit tests including special cases for NULL, empty,
      non-terminated, and embedded-null cases
    
    Change-Id: I2dd226d69fee428b5b38bdbf84dc73de4293c81b
    Reviewed-on: http://gerrit.cloudera.org:8080/24474
    Reviewed-by: Peter Rozsa <pr...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Michael Smith <mi...@cloudera.com>
---
 be/src/udf/udf-test.cc | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 be/src/udf/udf.h       |  8 ++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/be/src/udf/udf-test.cc b/be/src/udf/udf-test.cc
index fabf11055..c50598132 100644
--- a/be/src/udf/udf-test.cc
+++ b/be/src/udf/udf-test.cc
@@ -16,6 +16,7 @@
 // under the License.
 
 #include <iostream>
+#include <sstream>
 #include <boost/date_time/posix_time/posix_time.hpp>
 
 #include "common/logging.h"
@@ -40,7 +41,7 @@ DoubleVal ZeroUdf(FunctionContext* context) {
 }
 
 StringVal LogUdf(FunctionContext* context, const StringVal& arg1) {
-  cerr << (arg1.is_null ? "NULL" : string((char*)arg1.ptr, arg1.len)) << endl;
+  cerr << arg1 << endl;
   return arg1;
 }
 
@@ -398,4 +399,47 @@ TEST(UdfTest, TestStringValCopyFrom) {
   CheckStringValCopyFromFailsWithLength(len64);
 }
 
+TEST(UdfTest, TestStringValStreamOperator) {
+  std::ostringstream oss;
+
+  StringVal null_val = StringVal::null();
+  oss << null_val;
+  EXPECT_EQ("NULL", oss.str());
+  oss.str("");
+  oss.clear();
+
+  StringVal empty_val("");
+  oss << empty_val;
+  EXPECT_EQ("", oss.str());
+  oss.str("");
+  oss.clear();
+
+  StringVal hello_val("hello");
+  oss << hello_val;
+  EXPECT_EQ("hello", oss.str());
+  oss.str("");
+  oss.clear();
+
+  // Buffer is not null-terminated; only 'len' bytes should be printed.
+  const char buffer[] = {'a', 'b', 'c', 'X', 'Y', 'Z'};
+  StringVal non_null_terminated_val(
+      const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(buffer)), 3);
+  oss << non_null_terminated_val;
+  EXPECT_EQ("abc", oss.str());
+  oss.str("");
+  oss.clear();
+
+  // Embedded null bytes should be preserved in the output.
+  const char embedded_null[] = {'a', '\0', 'b'};
+  StringVal embedded_null_val(
+      const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(embedded_null)), 3);
+  oss << embedded_null_val;
+  EXPECT_EQ(std::string(embedded_null, 3), oss.str());
+  oss.str("");
+  oss.clear();
+
+  EXPECT_EQ("NULL", DebugString(null_val));
+  EXPECT_EQ("hello", DebugString(hello_val));
+}
+
 IMPALA_TEST_MAIN();
diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h
index a8a867544..89241a1e4 100644
--- a/be/src/udf/udf.h
+++ b/be/src/udf/udf.h
@@ -24,6 +24,7 @@
 
 #include <assert.h>
 #include <boost/cstdint.hpp>
+#include <ostream>
 #include <string.h>
 
 // Only use noexcept if the compiler supports C++11 (some system compilers may not
@@ -722,6 +723,13 @@ struct StringVal : public AnyVal {
       StringVal* res);
 };
 
+/// Prints the string contents using 'len' bytes from 'ptr'. The buffer is not required
+/// to be null-terminated. NULL strings are printed as "NULL".
+inline std::ostream& operator<<(std::ostream& os, const StringVal& val) {
+  if (val.is_null) return os << "NULL";
+  return os.write(reinterpret_cast<const char*>(val.ptr), val.len);
+}
+
 struct DecimalVal : public impala_udf::AnyVal {
   /// Decimal data is stored as an unscaled integer value. For example, the decimal 1.00
   /// (precision 3, scale 2) is stored as 100. The byte size necessary to store the