Check that functions work with invalid Unicode strings

[krlmlr]

in particular dbQuoteString():

set.seed(20171108)
x <- rawToChar(as.raw(runif(100) * 255+1))
xx <- gsub("'", "''", x)
xx <- gsub("'", "''", x, fixed = TRUE, useBytes = TRUE)
xx <- gsub("'", "''", x, fixed = TRUE)
#> Error in gsub("'", "''", x, fixed = TRUE): input string 1 is invalid in this locale

(Also, results should always be in UTF-8.)

@gaborcsardi @hadley: I believe this is the reason for revdepcheck failures.

@patperry: Do you have a workaround for this in utf8?

[gaborcsardi]

@krlmlr What kind of failures in particular?

[krlmlr]

revdepcheck was aborting when checking the testthat package with the error shown in the reprex above. (Not a failure in a downstream package, but in revdepcheck.)

[patperry]

utf8::utf8_format and utf8::utf8_encode do the equivalent of

utf8_sanitize <- function(x) {
    ok <- utf8::utf8_valid(x)
    x[ok] <- utf8::as_utf8(x[ok])
    Encoding(x[!ok]) <- "bytes"
    x
}
xx <- gsub("'", "''", utf8_sanitize(x), fixed = TRUE)

I'll put something like this in the next release of utf8. (Thanks for the feature request!)

[patperry]

(utf8_sanitize does what enc2utf8 is supposed to do, but the latter is buggy on Windows. See, e.g. this thread on r-devel: https://stat.ethz.ch/pipermail/r-devel/2017-September/074908.html )

[krlmlr]

If the user somehow managed to produce an invalid UTF-8 string, I don't think it's the job of dbQuoteString() to fix this. Also, I'd rather avoid adding dependencies to the DBI package, but backends are free to do so.

gsub(fixed = TRUE, useBytes = TRUE) does change the encoding of strings with invalid UTF-8, but plain gsub() does not.

[gaborcsardi]

Agreed. If you get an invalid string, then that's probably an error that should be fixed upstream, either by using encoding "bytes", or just creating a valid string.

[krlmlr]

Do you also agree that DBI should pass that verbatim to the database?

[gaborcsardi]

You mean a string with an invalid encoding? Maybe it would make sense to throw an error? As databases can behave differently in this case, I would not rely on them.

[gaborcsardi]

As I understand this is already happening, and you get an error because the gsub fails. Right?

[krlmlr]

We should work with UTF-8 internally. We rely on the Encoding() and on being able to get UTF-8 strings via enc2utf8(). But what if the user passes an invalid UTF-8 string? Do we pass that verbatim to the DB driver? Apparently SQLite is fine, other databases might be not.

[gaborcsardi]

But what if the user passes an invalid UTF-8 string?

I think DBI should error on that. The "bytes" encoding can still be used to pass a string to the DB, as is, right?

[krlmlr]

Not if we're always calling enc2utf8(): here, the "bytes" encoding becomes "UTF-8". I think we need to call enc2utf8() to align with the encoding of the database connection, because (especially on Windows) users might pass strings in the system encoding which is not UTF-8 there.

Encoding(enc2utf8(rawToChar(as.raw(runif(100) * 255 + 1))))
#> [1] "UTF-8"

[krlmlr]

On the other hand:

Encoding(enc2utf8("Encoding<-"(rawToChar(as.raw(runif(100) * 255+1)), "bytes")))
#> [1] "bytes"

So maybe we should support bytes and check UTF-8 sanity?

[gaborcsardi]

Converting to UTF-8 is fine, and the right thing to do imo. As I see the questions are

1. what to do with "bytes" encoding?
2. whether to check UTF-8 validity and error if invalid?
3. what to do with "unknown" encoding?

I would say

1. pass it through (maybe)
2. yes, after enc2utf8 probably.
3. enc2utf8 assumes native encoding for "unknown", AFAICT, so that is fine, but then we need to check if the result is valid.

Looks like iconv(x, "UTF-8", "UTF-8") can be used to check for valid UTF-8.