Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use 'Rf_mkCharLenCE() as appropriate #917

Merged
merged 2 commits into from
Nov 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
2018-10-25 Kevin Ushey <kevinushey@gmail.com>

* 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 <edd@debian.org>

* DESCRIPTION (Date, Version): Roll minor version
Expand Down
3 changes: 3 additions & 0 deletions inst/NEWS.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
23 changes: 19 additions & 4 deletions inst/include/Rcpp/String.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions inst/include/Rcpp/exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions inst/unitTests/cpp/String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
4 changes: 4 additions & 0 deletions inst/unitTests/runit.String.R
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,8 @@ if (.runThisTest) {
checkEquals(Encoding(test_String_ctor_encoding2()), "UTF-8")
}

test.String.embeddedNul <- function() {
checkException(test_String_embeddedNul())
}

}