Merge pull request #917 from RcppCore/bugfix/string-embedded-nul

use 'Rf_mkCharLenCE() as appropriate

Author: eddelbuettel

ChangeLog

@@ -1,3 +1,9 @@
+2018-10-25  Kevin Ushey  <[email protected]>
+
+	* inst/include/Rcpp/String.h: Use Rf_mkCharLenCE() as appropriate
+	* inst/unitTests/cpp/String.cpp: Add unit tests
+	* inst/unitTests/runit.String.R: Add unit tests
+
 2018-10-12  Dirk Eddelbuettel  <[email protected]>
 
 	* DESCRIPTION (Date, Version): Roll minor version

inst/NEWS.Rd

@@ -12,6 +12,9 @@
       \item The constructor for \code{NumericMatrix(not_init(n,k))} was
       corrected (Romain in \ghpr{904}, Dirk in \ghpr{905}, and also
       Romain in \ghpr{908} fixing \ghpr{907}).
+      \item Rcpp::String no longer silently drops embedded NUL bytes
+      in strings. Instead, the new Rcpp exception `embedded_nul_in_string`
+      is thrown. (\ghit{916})
     }
     \item Changes in Rcpp Deployment:
     \itemize{

inst/include/Rcpp/String.h

@@ -364,9 +364,22 @@ namespace Rcpp {
         }
 
 
+        inline SEXP get_sexp_impl() const {
+
+            // workaround for h5 package (currently deprecated so updates
+            // to CRAN may not be timely)
+#ifdef __H5Cpp_H
+            return Rf_mkCharCE(buffer.c_str(), enc);
+#else
+            if (buffer.find('\0') != std::string::npos)
+                throw embedded_nul_in_string();
+            return Rf_mkCharLenCE(buffer.c_str(), buffer.size(), enc);
+#endif
+        }
+
         inline SEXP get_sexp() const {
             RCPP_STRING_DEBUG_1("String::get_sexp const (valid = %d) ", valid);
-            return valid ? data : Rf_mkCharCE(buffer.c_str(), enc);
+            return valid ? data : get_sexp_impl();
         }
 
         inline SEXP get_sexp() {
@@ -395,9 +408,11 @@ namespace Rcpp {
             enc = encoding;
 
             if (valid) {
-                data = Rcpp_ReplaceObject(data, Rf_mkCharCE(Rf_translateCharUTF8(data), encoding));
+                // TODO: may longjmp on failure to translate?
+                const char* translated = Rf_translateCharUTF8(data);
+                data = Rcpp_ReplaceObject(data, Rf_mkCharCE(translated, encoding));
             } else {
-                data = Rf_mkCharCE(buffer.c_str(), encoding);
+                data = get_sexp_impl();
                 Rcpp_PreserveObject(data);
                 valid = true;
             }
@@ -469,7 +484,7 @@ namespace Rcpp {
         inline void setData() {
             RCPP_STRING_DEBUG("setData");
             if (!valid) {
-                data = Rf_mkCharCE(buffer.c_str(), enc);
+                data = get_sexp_impl();
                 Rcpp_PreserveObject(data);
                 valid = true;
             }

inst/include/Rcpp/exceptions.h

@@ -208,6 +208,7 @@ namespace Rcpp {
     RCPP_SIMPLE_EXCEPTION_CLASS(no_such_field, "No such field.") // not used internally
     RCPP_SIMPLE_EXCEPTION_CLASS(no_such_function, "No such function.")
     RCPP_SIMPLE_EXCEPTION_CLASS(unevaluated_promise, "Promise not yet evaluated.")
+    RCPP_SIMPLE_EXCEPTION_CLASS(embedded_nul_in_string, "Embedded NUL in string.")
 
     // Promoted
     RCPP_EXCEPTION_CLASS(no_such_slot, "No such slot")

inst/unitTests/cpp/String.cpp

@@ -114,3 +114,9 @@ String test_String_ctor_encoding2() {
     y.set_encoding(CE_UTF8);
     return y;
 }
+
+// [[Rcpp::export]]
+String test_String_embeddedNul() {
+    std::string bad("abc\0abc", 7);
+    return String(bad);
+}

inst/unitTests/runit.String.R

@@ -94,4 +94,8 @@ if (.runThisTest) {
         checkEquals(Encoding(test_String_ctor_encoding2()), "UTF-8")
     }
 
+    test.String.embeddedNul <- function() {
+        checkException(test_String_embeddedNul())
+    }
+
 }