Re: [PATCH] add tests for tzset(3)

[Jeff Johnston <jjohnstn@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]

I have taken a stab at creating a tzset_r.c that handles errors by setting
timezone to unnamed UTC.

Let me know if there are any issues.

-- Jeff J.

On Fri, Apr 15, 2022 at 6:10 AM jdoubleu <hi@jdoubleu.de> wrote:

> On Thu, Apr 14, 2022 at 12:31 PM Brian Inglis wrote:
> > I am still not hearing from where the requirement originates to set
> > UTC/GMT/etc or do anything other than leave everything as is.
> > Is this glibc behaviour, and why not /etc/localtime or /etc/timezone?
>
> On 4/14/2022 9:23 PM, Jeff Johnston wrote:
> > It is glibc behaviour as I mentioned in my note. The following is also
> > from man tzset
>
> Aside from glibc, the BSD man page[1] also states:
> > If the TZ environment variable [..] cannot be interpreted as a direct
> specification, UTC is used.
> Where "direct specification" is the formatted string.
>
> The POSIX standard does not seem to explicitly state what happens in an
> error case, besides[2]:
> > The interpretation of these fields is unspecified if either field is
> less than three bytes [..],  > more than {TZNAME_MAX} bytes, or if they
> contain characters other
> than those specified.and[3]
> > If TZ is absent from the environment, implementation-defined default
> timezone information shall be used.
>
> [1]: https://man.openbsd.org/tzset
> [2]:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
> [2]:
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/tzset.html#
>
>
> Cheers
> ---
> 🙎🏻‍♂️ jdoubleu
> On 4/14/2022 9:23 PM, Jeff Johnston wrote:
> > On Thu, Apr 14, 2022 at 12:31 PM Brian Inglis <
> > Brian.Inglis@systematicsw.ab.ca> wrote:
> >
> >> I am still not hearing from where the requirement originates to set
> >> UTC/GMT/etc or do anything other than leave everything as is.
> >> Is this glibc behaviour, and why not /etc/localtime or /etc/timezone?
> >>
> >>
> > It is glibc behaviour as I mentioned in my note.  The following is also
> > from man tzset
> >
> >        If the TZ variable does not appear in the environment, the tzname
> > vari‐
> >         able  is  initialized  with  the best approximation of local wall
> > clock
> >         time, as specified by the tzfile(5)-format file localtime found
> in
> >   the
> >         system   timezone   directory   (see  below).   (One  also  often
> >   sees
> >         /etc/localtime used here, a symlink to the right  file  in  the
> >   system
> >         timezone directory.)
> >
> >         If  the  TZ  variable  does  appear in the environment but its
> value
> > is
> >         empty or its value cannot be interpreted using any of the formats
> > spec‐
> >         ified below, Coordinated Universal Time (UTC) is used.
> >
> > Note about if its value is specified and cannot be interpreted using any
> of
> > the formats specified.
> > If there is an error, then that clause would apply.  In glibc's case, it
> is
> > less than 3 chars and invalid
> > chars.  In our case, exceeding the max limit would also apply.
> >
> >  From glibc: tzset.c
> >
> >   /* Clear out old state and reset to unnamed UTC.  */
> >    memset (tz_rules, '\0', sizeof tz_rules);
> >    tz_rules[0].name = tz_rules[1].name = "";
> >
> >    /* Get the standard timezone name.  */
> >    if (parse_tzname (&tz, 0) && parse_offset (&tz, 0))
> >
> > If the parse_tzname fails or parsing the dst name fails, unnamed UTC is
> > used.
> >
> > -- Jeff J.
> >
>
>

[-- Attachment #2: 0001-Modify-tzset_r.c-to-handle-errors.patch --]
[-- Type: text/x-patch, Size: 4523 bytes --]

From 0b5e905696ec12e6408e9330ef4e87ac6a57751d Mon Sep 17 00:00:00 2001
From: Jeff Johnston <jjohnstn@redhat.com>
Date: Wed, 27 Apr 2022 15:27:00 -0400
Subject: [PATCH] Modify tzset_r.c to handle errors

- change __tzset_r so errors end up setting the timezone to
  unnamed UTC
---
 newlib/libc/time/tzset_r.c | 56 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
index 9cb30b1..4d4baff 100644
--- a/newlib/libc/time/tzset_r.c
+++ b/newlib/libc/time/tzset_r.c
@@ -23,6 +23,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   unsigned short hh, mm, ss, m, w, d;
   int sign, n;
   int i, ch;
+  long offset0, offset1;
   __tzinfo_type *tz = __gettzinfo ();
 
   if ((tzenv = _getenv_r (reent_ptr, "TZ")) == NULL)
@@ -44,6 +45,12 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   if (prev_tzenv != NULL)
     strcpy (prev_tzenv, tzenv);
 
+  /* default to unnamed UTC in case of error */
+  _timezone = 0;
+  _daylight = 0;
+  _tzname[0] = "";
+  _tzname[1] = "";
+
   /* ignore implementation-specific format specifier */
   if (*tzenv == ':')
     ++tzenv;  
@@ -85,8 +92,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   if (sscanf (tzenv, "%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n) < 1)
     return;
   
-  tz->__tzrule[0].offset = sign * (ss + SECSPERMIN * mm + SECSPERHOUR * hh);
-  _tzname[0] = __tzname_std;
+  offset0 = sign * (ss + SECSPERMIN * mm + SECSPERHOUR * hh);
   tzenv += n;
 
   /* allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330> */
@@ -95,12 +101,16 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
       ++tzenv;
 
       /* quit if no items, too few or too many chars, or no close quote '>' */
-      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_dst, &n) <= 0
-		|| n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])
+      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_dst, &n) <= 0 && tzenv[0] == '>')
 	{ /* No dst */
-	  _tzname[1] = _tzname[0];
-	  _timezone = tz->__tzrule[0].offset;
-	  _daylight = 0;
+          _tzname[0] = __tzname_std;
+          _tzname[1] = _tzname[0];
+          tz->__tzrule[0].offset = offset0;
+          _timezone = offset0;
+	  return;
+        }
+      else if (n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])
+	{ /* error */
 	  return;
 	}
 
@@ -109,17 +119,20 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   else
     {
       /* allow POSIX unquoted alphabetic tz abbr e.g. MESZ */
-      if (sscanf (tzenv, "%10[A-Za-z]%n", __tzname_dst, &n) <= 0
-				|| n < TZNAME_MIN || TZNAME_MAX < n)
+      if (sscanf (tzenv, "%10[A-Za-z]%n", __tzname_dst, &n) <= 0)
 	{ /* No dst */
-	  _tzname[1] = _tzname[0];
-	  _timezone = tz->__tzrule[0].offset;
-	  _daylight = 0;
+          _tzname[0] = __tzname_std;
+          _tzname[1] = _tzname[0];
+          tz->__tzrule[0].offset = offset0;
+          _timezone = offset0;
+	  return;
+        }
+      else if (n < TZNAME_MIN || TZNAME_MAX < n)
+	{ /* error */
 	  return;
 	}
     }
 
-  _tzname[1] = __tzname_dst;
   tzenv += n;
 
   /* otherwise we have a dst name, look for the offset */
@@ -138,9 +151,9 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   
   n  = 0;
   if (sscanf (tzenv, "%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n) <= 0)
-    tz->__tzrule[1].offset = tz->__tzrule[0].offset - 3600;
+    offset1 = offset0 - 3600;
   else
-    tz->__tzrule[1].offset = sign * (ss + SECSPERMIN * mm + SECSPERHOUR * hh);
+    offset1 = sign * (ss + SECSPERMIN * mm + SECSPERHOUR * hh);
 
   tzenv += n;
 
@@ -211,13 +224,24 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
       n = 0;
       
       if (*tzenv == '/')
-	sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n);
+	if (sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n) <= 0)
+	  {
+	    /* error in time format, restore tz rules to default and return */
+	    struct __tzrule_struct default_tzrule = {'J', 0, 0, 0, 0, (time_t)0, 0L };
+	    tz->__tzrule[0] = default_tzrule;
+	    tz->__tzrule[1] = default_tzrule;
+            return;
+          }
 
       tz->__tzrule[i].s = ss + SECSPERMIN * mm + SECSPERHOUR  * hh;
       
       tzenv += n;
     }
 
+  tz->__tzrule[0].offset = offset0;
+  tz->__tzrule[1].offset = offset1;
+  _tzname[0] = __tzname_std;
+  _tzname[1] = __tzname_dst;
   __tzcalc_limits (tz->__tzyear);
   _timezone = tz->__tzrule[0].offset;  
   _daylight = tz->__tzrule[0].offset != tz->__tzrule[1].offset;
-- 
1.8.3.1