You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rob Hartill <ro...@imdb.com> on 1996/11/04 11:46:00 UTC

Re: fixes for Apache 1.1.1 bugs when computing time zones for logs

Hi, Paul.

thanks for the information and patch. It is being forwarded to the developers
list for consideration.

cheers,
rob

Paul Eggert wrote:
>
>Email address: eggert@twinsun.com
>1-line summary: fixes for Apache 1.1.1 bugs when computing time zones for logs
>OS: Solaris 2.5.1
>Apache version: 1.1.1
>Extra modules used: none
>Symptoms:
>
>* When the time zone is not an integral multiple of 1 hour,
>the logs report bogus UTC offsets.  E.g. if the time zone
>is +0530 the reported UTC offset is +051800.
>
>* If !defined(HAS_GMTOFF) && !defined(NO_TIMEZONE), when the
>daylight-saving offset is not exactly 1 hour, the logs compute the UTC
>offset incorrectly when daylight saving is in effect.
>
>* If !defined(HAS_GMTOFF) && defined(NO_TIMEZONE), the code invokes
>mktime twice, once on the output of localtime and once on the output
>of gmtime, and subtracts the results.  This method is incorrect when
>the time is near a daylight-saving shift boundary.  For example,
>suppose we are in US Pacific time and the current time is 1996-10-27
>00:00:00 local time, just before a daylight saving time shift.  The
>correct UTC offset is -0700, but in this case get_gmtoff incorrectly
>computes an offset of -0800, because mktime(gmtime(...)) applies
>mktime to `1996-10-27 07:00:00' (actually a UTC time, not a local
>time) and the UTC offset as of 1996-10-27 07:00:00 local time is -0800.
>
>
>Here is a patch.  The key idea of the patch, which you'll find at the
>end, is an idea stolen from INN 1.5b1: assume that we are no more than
>24 hours away from UTC, which is a reasonable assumption (among other
>things, Posix requires support only for -2400 through +2400 time
>zones).
>
>1996-11-03  Paul Eggert  <eg...@twinsun.com>
>
>	* httpd.h, util.c (get_gmtoff): Now returns minutes as int,
>	not seconds as long.
>	* util.c (get_gmtoff): When !defined(HAS_GMTOFF) &&
>	defined(NO_TIMEZONE), fix bug when computing UTC offset near
>	DST shift boundary.
>	* util.c (get_gmtoff), mod_cookies.c (cookie_log_transaction),
>	mod_log_common.c (common_log_transaction), mod_log_config.c
>	(log_request_time): Don't assume UTC offset is a multiple of 1 hour.
>	* conf.h (timezone, NO_TIMEZONE): Remove; no longer needed.
>
>===================================================================
>RCS file: conf.h,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** conf.h	1996/06/29 20:02:46	1.1.1.0
>--- conf.h	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 282,288 ****
>  #undef HAS_GMTOFF
>  #undef NO_KILLPG
>  #undef NO_SETSID
>- #define timezone	_bky_timezone
>  
>  #elif defined(__FreeBSD__) || defined(__bsdi__)
>  #define HAS_GMTOFF
>--- 282,287 ----
>***************
>*** 301,307 ****
>  #elif defined(LYNXOS)
>  #undef NO_KILLPG
>  #undef NO_SETSID
>- #define NO_TIMEZONE
>  #define NEED_STRCASECMP
>  #define NEED_STRNCASECMP
>  #define NEED_INITGROUPS
>--- 300,305 ----
>===================================================================
>RCS file: httpd.h,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** httpd.h	1996/07/08 19:01:19	1.1.1.0
>--- httpd.h	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 507,513 ****
>  /* Time */
>  extern const char month_snames[12][4];
>  
>! struct tm *get_gmtoff(long *tz);
>  char *get_time();
>  char *ht_time (pool *p, time_t t, char *fmt, int gmt);     
>  char *gm_timestr_822(pool *p, time_t t);
>--- 507,513 ----
>  /* Time */
>  extern const char month_snames[12][4];
>  
>! struct tm *get_gmtoff(int *tz);
>  char *get_time();
>  char *ht_time (pool *p, time_t t, char *fmt, int gmt);     
>  char *gm_timestr_822(pool *p, time_t t);
>===================================================================
>RCS file: mod_cookies.c,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** mod_cookies.c	1996/07/04 13:04:22	1.1.1.0
>--- mod_cookies.c	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 236,242 ****
>      cookie_log_state *cls = get_module_config (orig->server->module_config,
>                             &cookies_module);
>      char *str;
>!     long timz;
>      struct tm *t;
>      char tstr[MAX_STRING_LEN],sign;
>      request_rec *r;
>--- 236,242 ----
>      cookie_log_state *cls = get_module_config (orig->server->module_config,
>                             &cookies_module);
>      char *str;
>!     int timz;
>      struct tm *t;
>      char tstr[MAX_STRING_LEN],sign;
>      request_rec *r;
>***************
>*** 265,275 ****
>  
>      strftime(tstr,MAX_STRING_LEN,"\" [%d/%b/%Y:%H:%M:%S ",t);
>      if (r->status != -1)
>! 	sprintf(&tstr[strlen(tstr)], "%c%02ld%02ld] %d\n", sign, timz/3600,
>! 		timz%3600, r->status);
>      else
>! 	sprintf(&tstr[strlen(tstr)], "%c%02ld%02ld] -\n", sign, timz/3600,
>! 		timz%3600);
>  
>      str = pstrcat(orig->pool, cookiebuf, " \"", orig->the_request, tstr, NULL);
>      
>--- 265,275 ----
>  
>      strftime(tstr,MAX_STRING_LEN,"\" [%d/%b/%Y:%H:%M:%S ",t);
>      if (r->status != -1)
>! 	sprintf(&tstr[strlen(tstr)], "%c%.2d%.2d] %d\n", sign, timz/60,
>! 		timz%60, r->status);
>      else
>! 	sprintf(&tstr[strlen(tstr)], "%c%.2d%.2d] -\n", sign, timz/60,
>! 		timz%60);
>  
>      str = pstrcat(orig->pool, cookiebuf, " \"", orig->the_request, tstr, NULL);
>      
>===================================================================
>RCS file: mod_log_common.c,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** mod_log_common.c	1996/06/17 20:17:03	1.1.1.0
>--- mod_log_common.c	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 153,159 ****
>  					       &common_log_module);
>    
>      char *str;
>!     long timz;
>      struct tm *t;
>      const char *rem_logname;
>      char tstr[MAX_STRING_LEN], status[MAX_STRING_LEN], sign;
>--- 153,159 ----
>  					       &common_log_module);
>    
>      char *str;
>!     int timz;
>      struct tm *t;
>      const char *rem_logname;
>      char tstr[MAX_STRING_LEN], status[MAX_STRING_LEN], sign;
>***************
>*** 175,183 ****
>      if(timz < 0) 
>          timz = -timz;
>  
>!     sprintf(tstr, " [%.2d/%s/%d:%.2d:%.2d:%.2d %c%02ld%02ld] \"", t->tm_mday,
>  	    month_snames[t->tm_mon], t->tm_year + 1900, t->tm_hour, t->tm_min,
>! 	    t->tm_sec, sign, timz/3600, timz%3600);
>  
>      if (r->status != -1) sprintf(status,"%d ", r->status);
>      else                 strcpy(status, "- ");
>--- 175,183 ----
>      if(timz < 0) 
>          timz = -timz;
>  
>!     sprintf(tstr, " [%.2d/%s/%d:%.2d:%.2d:%.2d %c%.2d%.2d] \"", t->tm_mday,
>  	    month_snames[t->tm_mon], t->tm_year + 1900, t->tm_hour, t->tm_min,
>! 	    t->tm_sec, sign, timz/60, timz%60);
>  
>      if (r->status != -1) sprintf(status,"%d ", r->status);
>      else                 strcpy(status, "- ");
>===================================================================
>RCS file: mod_log_config.c,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** mod_log_config.c	1996/06/27 01:23:26	1.1.1.0
>--- mod_log_config.c	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 217,224 ****
>  
>      strftime(tstr,MAX_STRING_LEN,"[%d/%b/%Y:%H:%M:%S ",t);
>  
>!     sprintf (tstr + strlen(tstr), "%c%02ld%02ld]",
>! 	     sign, timz/3600, timz%3600);
>  
>      return pstrdup (r->pool, tstr);
>  }
>--- 217,224 ----
>  
>      strftime(tstr,MAX_STRING_LEN,"[%d/%b/%Y:%H:%M:%S ",t);
>  
>!     sprintf (tstr + strlen(tstr), "%c%.2d%.2d]",
>! 	     sign, timz/60, timz%60);
>  
>      return pstrdup (r->pool, tstr);
>  }
>===================================================================
>RCS file: util.c,v
>retrieving revision 1.1.1.0
>retrieving revision 1.1.1.1
>diff -c -r1.1.1.0 -r1.1.1.1
>*** util.c	1996/06/26 10:46:37	1.1.1.0
>--- util.c	1996/11/04 01:42:28	1.1.1.1
>***************
>*** 142,167 ****
>  }
>  
>  /* What a pain in the ass. */
>! struct tm *get_gmtoff(long *tz) {
>!     time_t tt;
>!     struct tm *t;
>  
>-     tt = time(NULL);
>-     t = localtime(&tt);
>  #if defined(HAS_GMTOFF)
>!     *tz = t->tm_gmtoff;
>! #elif !defined(NO_TIMEZONE)
>!     *tz = - timezone;
>!     if(t->tm_isdst)
>!         *tz += 3600;
>  #else
>    {
>!     static struct tm loc_t;
>! 
>!     loc_t = *t;   /* save it */
>!     t = gmtime(&tt);
>!     *tz = mktime(&loc_t) - mktime(t);
>!     t = &loc_t; /* return pointer to saved time */
>    }
>  #endif
>      return t;
>--- 142,164 ----
>  }
>  
>  /* What a pain in the ass. */
>! struct tm *get_gmtoff(int *tz) {
>!     time_t tt = time(NULL);
>! #if ! defined(HAS_GMTOFF)
>!     struct tm gmt = *gmtime(&tt);
>! #endif
>!     struct tm *t = localtime(&tt);
>  
>  #if defined(HAS_GMTOFF)
>!     *tz = (int) (t->tm_gmtoff / 60);
>  #else
>    {
>!     /* Assume we are never more than 24 hours away. */
>!     int days = t->tm_yday - gmt.tm_yday;
>!     int hours = ((days < -1 ? 24 : 1 < days ? -24 : days * 24)
>! 		 + t->tm_hour - gmt.tm_hour);
>!     int minutes = hours * 60 + t->tm_min - gmt.tm_min;
>!     *tz = minutes;
>    }
>  #endif
>      return t;
>


-- 
Rob Hartill.       Internet Movie Database Ltd.    http://www.imdb.com/  

Re: fixes for Apache 1.1.1 bugs when computing time zones for logs

Posted by Brian Behlendorf <br...@organic.com>.
I've worked this patch against the current tree, and sent Paul the patched
source to try out.  If it comes back positive I think we should commit it.
Thoughts?

	Brian

> Paul Eggert wrote:
> >
> >Email address: eggert@twinsun.com
> >1-line summary: fixes for Apache 1.1.1 bugs when computing time zones for logs
> >OS: Solaris 2.5.1
> >Apache version: 1.1.1
> >Extra modules used: none
> >Symptoms:
> >
> >* When the time zone is not an integral multiple of 1 hour,
> >the logs report bogus UTC offsets.  E.g. if the time zone
> >is +0530 the reported UTC offset is +051800.
> >
> >* If !defined(HAS_GMTOFF) && !defined(NO_TIMEZONE), when the
> >daylight-saving offset is not exactly 1 hour, the logs compute the UTC
> >offset incorrectly when daylight saving is in effect.
> >
> >* If !defined(HAS_GMTOFF) && defined(NO_TIMEZONE), the code invokes
> >mktime twice, once on the output of localtime and once on the output
> >of gmtime, and subtracts the results.  This method is incorrect when
> >the time is near a daylight-saving shift boundary.  For example,
> >suppose we are in US Pacific time and the current time is 1996-10-27
> >00:00:00 local time, just before a daylight saving time shift.  The
> >correct UTC offset is -0700, but in this case get_gmtoff incorrectly
> >computes an offset of -0800, because mktime(gmtime(...)) applies
> >mktime to `1996-10-27 07:00:00' (actually a UTC time, not a local
> >time) and the UTC offset as of 1996-10-27 07:00:00 local time is -0800.
> >
> >
> >Here is a patch.  The key idea of the patch, which you'll find at the
> >end, is an idea stolen from INN 1.5b1: assume that we are no more than
> >24 hours away from UTC, which is a reasonable assumption (among other
> >things, Posix requires support only for -2400 through +2400 time
> >zones).

--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
brian@organic.com  www.apache.org  hyperreal.com  http://www.organic.com/JOBS