lag.xts segfault when 'k' is character

[joshuaulrich]

Yes, you shouldn't do this... but it's not nice to bring down the entire R session if someone accidentally enters a typo. ;)

xts:::lag.xts(xts::.xts(1:5,1:5), 'ka-boom')

[ghost]

Hi Josh,
Thank you for reporting this bug.

--- lag.xts.R   2016-01-20 21:21:03.000000000 +0100
+++ lag.xts.fix.R   2016-09-06 18:54:45.000000000 +0200
@@ -23,7 +23,9 @@
   x <- try.xts(x, error=FALSE)

   if(!is.xts(x)) x <- as.matrix(x)
-  
+  if(!is.numeric(k))
+    stop("'k' must be a numeric type")
+
   xx <-sapply(k, 
          function(k) {
            apply(x, 2, 
@@ -46,6 +48,8 @@
   x <- try.xts(x, error=FALSE)

   if(!is.xts(x)) x <- as.matrix(x)
+  if(!is.numeric(k))
+    stop("'k' must be a numeric type")

   xx <-sapply(k, 
          function(k) {
@@ -67,6 +71,8 @@


 lag.xts <- function(x, k=1, na.pad=TRUE, ...) {
+  if(!is.numeric(k))
+    stop("'k' must be a numeric type")
   zooCompat <- getOption('xts.compat.zoo.lag')
   if(is.logical(zooCompat) && zooCompat) {
     k <- -k
@@ -81,6 +87,8 @@
 }

 lagts.xts <- function(x, k=1, na.pad=TRUE, ...) {
+  if(!is.numeric(k))
+    stop("'k' must be a numeric type")
   if(length(k) > 1) {
     if(is.null(names(k)))
       names(k) <- paste("lag",k,sep="")
@@ -91,12 +99,16 @@

 diff.xts <- function(x, lag=1, differences=1, arithmetic=TRUE, log=FALSE, na.pad=TRUE, ...)
 {
-  if(is.logical(x))
-    x <- .xts(matrix(as.integer(x),ncol=NCOL(x)), .index(x))
-
+  if(!is.numeric(lag))
+    stop("'lag' must be a numeric type")
+  if(!is.numeric(differences))
+    stop("'differences' must be a numeric type")
   if(lag < 1 || differences < 1)
     stop("'diff.xts' defined only for positive lag and differences arguments")

+  if(is.logical(x))
+    x <- .xts(matrix(as.integer(x),ncol=NCOL(x)), .index(x))
+
   zooCompat <- getOption('xts.compat.zoo.lag')
   if(is.logical(zooCompat) && zooCompat) {
     # this has to negated to satisfy the test in lag.xts... oh my

Untested, but should be ok.

Best regards,
Daniel Cegielka

[joshuaulrich]

@silentbits Thanks for the patch, but I decided to move the checks to C, so we can't accidentally pass an invalid value to zoo_lag() and cause a crash in the future.

[ghost]

1e15602

I think, that this problem should also be solved in C code. It looks like a quick fix.

[joshuaulrich]

I'm not sure what you're referring to. Can you please elaborate?

[ghost]

"merge was segfaulting when called with 3 or more zero-width object"

(merge.xts) segfault refers to merge.c code, but the args check is in R (patch 1e15602) before .External() / .Call() call, which means that this bug is still in merge.c. This is similar to lag.xts segfault when 'k' is character (checks in R vs checks in C).

[joshuaulrich]

Ah, I understand now. Thanks! I opened a new issue to keep track of your suggestion.