Re: [bug-report] ztrftime and datetime strftime

[Peter Stephenson <pws@xxxxxxxxxxxxxxxxxxxxxxxx>]

Stephane CHAZELAS wrote:
> [message posted to Peter Stephenson and zsh-workers ML, even if
> it will probably never show up on the ML for a reason I don't
> know of]
> 
> There are several problems with the Src/utils.c ztrftime function:
> 
> It always returns 0.

I can't work out what it was originally supposed to do.  This must be an
age-old oversight.  I've made it do what strftime() does --- hope this
works with strftimes other than Linux.

> When calling libc strftime, if the buffer is too small, the
> error is not reported. buffer overflows are not checked
> (PS1="%D{${(l:10000::%d:)}}" segfaults)

Fixed as a consequence.

> In Src/Modules/datetime.c, it is called as:
> 
> <<
>     bufsize = strlen(argv[0]) * 2;
> 
>     for (x=1;x<4;x++) {
>         buffer = zrealloc(buffer, bufsize * x);
>         size = ztrftime(buffer, bufsize * x, argv[0], t);
>         if (size) x = 4;
>     }
> >>

Hmm.  I don't know what the `size' variable's doing and why x=4 is used
to end the loop, so I've just removed those.

I presume it was just an oversight that buffer wasn't being freed.

> So, it is always called 3 times as ztrftime always returns 0.
> 
> strftime %c $SECS
>
> returns an empty string as %c expansion is more that 12 char
> long.

I made the increase exponential.  It works for my locale, but I haven't
done any calculations to check the range.

> Src/prompt.c is also wrong:
> 
>                     for(t0=80; ; t0*=2) {
>                         addbufspc(t0);
>                         if(ztrftime(bp, t0, tmfmt, tm) != t0)
>                             break;
>                     }

This *really* makes me wonder what the interface was supposed to
be... I've just removed the `!= t0'.

I also discovered that `print -P %D{%}' was causing a pointer overrun in
ztrftime which could cause garbage to be printed.

None of this gives me a warm and fuzzy feeling about the internals.

Please test again...
 
Index: Src/prompt.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/prompt.c,v
retrieving revision 1.11
diff -u -r1.11 prompt.c
--- Src/prompt.c	27 Jan 2003 16:39:25 -0000	1.11
+++ Src/prompt.c	6 Oct 2003 22:37:50 -0000
@@ -500,18 +500,23 @@
 			tmfmt = "%m/%d/%y";
 			break;
 		    case 'D':
-			if (fm[1] == '{') /*}*/ {
+			if (fm[1] == '{' /*}*/) {
 			    for (ss = fm + 2; *ss && *ss != /*{*/ '}'; ss++)
 				if(*ss == '\\' && ss[1])
 				    ss++;
 			    dd = tmfmt = tmbuf = zalloc(ss - fm);
-			    for (ss = fm + 2; *ss && *ss != /*{*/ '}'; ss++) {
+			    for (ss = fm + 2; *ss && *ss != /*{*/ '}';
+				 ss++) {
 				if(*ss == '\\' && ss[1])
 				    ss++;
 				*dd++ = *ss;
 			    }
 			    *dd = 0;
 			    fm = ss - !*ss;
+			    if (!*tmfmt) {
+				free(tmbuf);
+				continue;
+			    }
 			} else
 			    tmfmt = "%y-%m-%d";
 			break;
@@ -523,7 +528,7 @@
 		    tm = localtime(&timet);
 		    for(t0=80; ; t0*=2) {
 			addbufspc(t0);
-			if(ztrftime(bp, t0, tmfmt, tm) != t0)
+			if (ztrftime(bp, t0, tmfmt, tm))
 			    break;
 		    }
 		    bp += strlen(bp);
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.54
diff -u -r1.54 utils.c
--- Src/utils.c	24 Sep 2003 14:55:33 -0000	1.54
+++ Src/utils.c	6 Oct 2003 22:38:07 -0000
@@ -1685,20 +1685,42 @@
     }
 }
 
+/*
+ * Helper for ztrftime.  Called with a pointer to the length left
+ * in the buffer, and a new string length to decrement from that.
+ * Returns 0 if the new length fits, 1 otherwise.  We assume a terminating
+ * NUL and return 1 if that doesn't fit.
+ */
+
+/**/
+static int
+ztrftimebuf(int *bufsizeptr, int decr)
+{
+    if (*bufsizeptr <= decr)
+	return 1;
+    *bufsizeptr -= decr;
+    return 0;
+}
+
+/*
+ * Like the system function, this returns the number of characters
+ * copied, not including the terminating NUL.  This may be zero
+ * if the string didn't fit.
+ */
+
 /**/
 mod_export int
 ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm)
 {
-    int hr12;
+    int hr12, decr;
 #ifndef HAVE_STRFTIME
     static char *astr[] =
     {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
     static char *estr[] =
     {"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul",
      "Aug", "Sep", "Oct", "Nov", "Dec"};
-#else
-    char *origbuf = buf;
 #endif
+    char *origbuf = buf;
     char tmp[3];
 
 
@@ -1707,6 +1729,14 @@
     while (*fmt)
 	if (*fmt == '%') {
 	    fmt++;
+	    /*
+	     * Assume this format will take up at least two
+	     * characters.  Not always true, but if that matters
+	     * we are so close to the edge it's not a big deal.
+	     * Fix up some longer cases specially when we get to them.
+	     */
+	    if (ztrftimebuf(&bufsize, 2))
+		return 0;
 	    switch (*fmt++) {
 	    case 'd':
 		*buf++ = '0' + tm->tm_mday / 10;
@@ -1734,9 +1764,10 @@
 		if (hr12 == 0)
 		    hr12 = 12;
 	        if (hr12 > 9)
-		  *buf++ = '1';
+		    *buf++ = '1';
 		else if (fmt[-1] == 'l')
-		  *buf++ = ' ';
+		    *buf++ = ' ';
+
 		*buf++ = '0' + (hr12 % 10);
 		break;
 	    case 'm':
@@ -1755,11 +1786,20 @@
 		*buf++ = '0' + (tm->tm_year / 10) % 10;
 		*buf++ = '0' + tm->tm_year % 10;
 		break;
+	    case '\0':
+		/* Guard against premature end of string */
+		*buf++ = '%';
+		fmt--;
+		break;
 #ifndef HAVE_STRFTIME
 	    case 'a':
+		if (ztrftimebuf(&bufsize, strlen(astr[tm->tm_wday]) - 2))
+		    return 0;
 		strucpy(&buf, astr[tm->tm_wday]);
 		break;
 	    case 'b':
+		if (ztrftimebuf(&bufsize, strlen(estr[tm->tm_mon]) - 2))
+		    return 0;
 		strucpy(&buf, estr[tm->tm_mon]);
 		break;
 	    case 'p':
@@ -1772,17 +1812,27 @@
 		    *buf++ = fmt[-1];
 #else
 	    default:
+		/*
+		 * Remember we've already allowed for two characters
+		 * in the accounting in bufsize (but nowhere else).
+		 */
 		*buf = '\0';
 		tmp[1] = fmt[-1];
-		strftime(buf, bufsize - strlen(origbuf), tmp, tm);
-		buf += strlen(buf);
+		if (!strftime(buf, bufsize + 2, tmp, tm))
+		    return 0;
+		decr = strlen(buf);
+		buf += decr;
+		bufsize -= decr - 2;
 #endif
 		break;
 	    }
-	} else
+	} else {
+	    if (ztrftimebuf(&bufsize, 1))
+		return 0;
 	    *buf++ = *fmt++;
+	}
     *buf = '\0';
-    return 0;
+    return buf - origbuf;
 }
 
 /**/
Index: Src/Modules/datetime.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/datetime.c,v
retrieving revision 1.4
diff -u -r1.4 datetime.c
--- Src/Modules/datetime.c	28 Sep 2003 16:27:00 -0000	1.4
+++ Src/Modules/datetime.c	6 Oct 2003 22:38:07 -0000
@@ -35,10 +35,9 @@
 bin_strftime(char *nam, char **argv, Options ops, int func)
 {
     int bufsize, x;
-    char *endptr = NULL, *buffer = NULL;
+    char *endptr = NULL, *buffer;
     time_t secs;
     struct tm *t;
-    int size;
 
     secs = (time_t)strtoul(argv[1], &endptr, 10);
     if (secs == ULONG_MAX) {
@@ -51,15 +50,17 @@
 
     t = localtime(&secs);
     bufsize = strlen(argv[0]) * 2;
+    buffer = zalloc(bufsize);
 
-    for (x=1;x<4;x++) {
-	buffer = zrealloc(buffer, bufsize * x);
-        size = ztrftime(buffer, bufsize * x, argv[0], t);
-	if (size) x = 4;
+    for (x=0; x < 4; x++) {
+        if (ztrftime(buffer, bufsize, argv[0], t))
+	    break;
+	buffer = zrealloc(buffer, bufsize *= 2);
     }
 
     printf("%s\n", buffer);
-    
+    zfree(buffer, bufsize);
+
     return 0;
 }
 
-- 
Peter Stephenson <pws@xxxxxxxxxxxxxxxxxxxxxxxx>
Work: pws@xxxxxxx
Web: http://www.pwstephenson.fsnet.co.uk