httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ed Korthof ...@organic.com>
Subject Re: Agenda for 1.2b7
Date Thu, 06 Feb 1997 07:28:26 GMT
On Wed, 5 Feb 1997, Marc Slemko wrote:

>   * mod_include is slow.  Ed posted a patch to remove feof and
>     ferror from GET_CHAR.  Can we do more?

Yes; someone (I've forgotten who) mentioned buffering output.  I never
saw a patch, so I wrote my own.  It is below; I've tested it extensively,
and it appears to work fine on a live server.  I can't really quantify
how much of an advantage it is, however... haven't had a chance to
do a real profile.

>   * 64-bit issues; general cleanup, ap_snprintf("%d", (int)-1) giving
>     wrong behavior on Alpha boxes.

I don't enough about ap_snprintf to feel to comfortable working w/ it, but
I can easily patch things so we won't get all the warnings about bad casts
(from int to void *).  I don't think this is causing problems, but it's a
bit disturbing, especially when the linker says that everything is linked,
but might have run-time errors. 

>   * Satisfy Any can be changed if .htaccess exists
>         If you give Satisfy Any in access.conf for a particular directory,
>         and have a .htaccess in that directory, Satisfy mode reverts
>         to Satisfy All even if the .htaccess has _no_ authentication
>         directives.

I've posted a patch for this.  It has some problems, but will correctly
determine if a Satisfy directive has been found in a .htaccess file,
and if so, will have that value override.  It recieved one +1.

>   * new header_parse API hook is called too often
>        Status: RobH posted patch, had second thoughts.  He
>        suggests that mod_browser be optimised by detecting if it has been
>        called already and returning early if it has.

I'd be interested in working on this, and in creating a linked-list
of existing modules after reading the configuration file.  I've been
reading the docs on mod_dld so as to try to avoid messing it up.

>   * accept errors EPROTO and ECONNABORTED should not be logged
>        Status: no patch, ditto above, but will require ifdefs

I'll put together a patch for this -- it's not hard if all we
have to ifdef on is 'defined(EPROTO)' and 'defined(ECONNABORTED)'.
If there's more complexity to it than that, someone else ought to
take a look at it... I'm not that familiar with sockets.

> Planning/design items:
> 
>     * Should we change the default timeout of 1200?
> 	Status: discussion ongoing, +1 concept Jim, Randy, Marc

I would also like to add a timeout for reading the request; but
some people consider that a feature addition.  If so, I'll work on
other stuff now and do that later as a patch.

> Contrib stuff / future:
> 
>     * Chris Adams <cadams@ro.com> patch to mod_log_config to add %m
>       and %c.
> 
>     * "Large groups cause authentication errors" on FreeBSD
>       [salari@cs.ubc.ca]; problem looks to be MAX_STRING_LEN buffer
>       in groups_for_user.  
> 
>   * mod_log_config patch for conditional logging
> 	Status: contrib, not in server
> 
>     * Jim has patch for time taken to handle a request in status module

Also, I have a patch for limiting connections by remote IP.  It's
much better developed than the original one I posted here; it has
separate timeouts for requests in the Read status and all requests,
and it returns a 503 error.  Unfortunately, it requires some violation
of modularity (I declare die as an external function in
http_protocol.c; OTOH, it'd allow us to abort with an approrpriate
error code in other conditions (specifically, a 414)).

Anyway, it's all contained in 
#if defined(LIMIT_CONNECTIONS_BY_IP)'s, so it could be added w/o
messing with anything unless the person compiling wants to use
this feature. 

The one change I'd like to make is to get remote addresses in 16 bit
numbers, and do numeric comparisons rather than strcmp's (I minimize
those, but w/ 200+ servers running...). 

     -- Ed Korthof        |  Web Server Engineer --
     -- ed@organic.com    |  Organic Online, Inc --
     -- (415) 278-5676    |  Fax: (415) 284-6891 --

The patch mentioned above (for buffering mod_include output).  Most
of the changes are just because the buffering required me to
remove the 'return' statement from the GET_CHAR macro so I
could flush the buffer first.

*** mod_include.c.orig	Fri Jan 31 09:45:28 1997
--- mod_include.c	Mon Feb  3 01:12:34 1997
***************
*** 116,124 ****
  #define GET_CHAR(f,c,r,p) \
   { \
     int i = getc(f); \
!    if(feof(f) || ferror(f) || (i == -1)) { \
          pfclose(p,f); \
-         return r; \
     } \
     c = (char)i; \
   }
--- 116,126 ----
  #define GET_CHAR(f,c,r,p) \
   { \
     int i = getc(f); \
!    if (i == EOF) { /* either EOF or error--needs error handling if latter */ \
!         if (ferror(f)) \
!             fprintf(stderr, \
!                     "encountered error in GET_CHAR macro, mod_include.\n"); \
          pfclose(p,f); \
     } \
     c = (char)i; \
   }
***************
*** 129,153 ****
   * matter much, but this is an inner loop...
   */
  
  int find_string(FILE *in,char *str, request_rec *r, int printing) {
!     int x,l=strlen(str),p;
      char c;
  
      p=0;
      while(1) {
          GET_CHAR(in,c,1,r->pool);
          if(c == str[p]) {
!             if((++p) == l)
                  return 0;
          }
          else {
              if (printing) {
!                 for(x=0;x<p;x++) {
!                     rputc(str[x],r);
                  }
!                 rputc(c,r);
              }
-             p=0;
          }
      }
  }
--- 131,185 ----
   * matter much, but this is an inner loop...
   */
  
+ #ifndef LONG_STRING_LEN
+ #define LONG_STRING_LEN 4096
+ #endif
+ 
+ #define PUSH_BUFFER(b,l,r,t) \
+ { \
+     t = rwrite(b,l,r); \
+     if (t == l) { \
+         b[0]='\0'; \
+         l = 0; \
+     } \
+ }
+ 
  int find_string(FILE *in,char *str, request_rec *r, int printing) {
!     int x,l=strlen(str),p,count_in_buffer=0,test;
!     char buffer[LONG_STRING_LEN];
      char c;
  
      p=0;
      while(1) {
          GET_CHAR(in,c,1,r->pool);
+         if (c == EOF)
+         {
+             PUSH_BUFFER(buffer,count_in_buffer,r,test);
+             return 1;
+         }
          if(c == str[p]) {
!             if((++p) == l) {
!                 if (printing && count_in_buffer) {
!                     PUSH_BUFFER(buffer,count_in_buffer,r,test);
!                 } else {
!                     printf ("not printing: %d, %d\n",printing,count_in_buffer);
!                 }
                  return 0;
+             }
          }
          else {
              if (printing) {
!                 if ((count_in_buffer + 1 + p) >= LONG_STRING_LEN)
!                 {                      /* make sure not to overflow */
!                     PUSH_BUFFER(buffer,count_in_buffer,r,test);
                  }
!                 if (p) {
!                     strncpy (buffer+count_in_buffer,str,p);
!                     count_in_buffer+=p;
!                     p=0;
!                 }
!                 buffer[count_in_buffer++] = c;
              }
          }
      }
  }
***************
*** 250,264 ****
      n = 0;
  
      do { /* skip whitespace */
! 	GET_CHAR(in,c,NULL,p);
      } while (isspace(c));
  
      /* tags can't start with - */
      if(c == '-') {
!         GET_CHAR(in,c,NULL,p);
          if(c == '-') {
              do {
! 		GET_CHAR(in,c,NULL,p);
  	    } while (isspace(c));
              if(c == '>') {
                  strncpy(tag,"done", tagbuf_len-1);
--- 282,296 ----
      n = 0;
  
      do { /* skip whitespace */
! 	GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
      } while (isspace(c));
  
      /* tags can't start with - */
      if(c == '-') {
!         GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
          if(c == '-') {
              do {
! 		GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
  	    } while (isspace(c));
              if(c == '>') {
                  strncpy(tag,"done", tagbuf_len-1);
***************
*** 277,289 ****
          }
  	if(c == '=' || isspace(c)) break;
  	*(t++) = tolower(c);
!         GET_CHAR(in,c,NULL,p);
      }
  
      *t++ = '\0';
      tag_val = t;
  
!     while (isspace(c)) GET_CHAR(in, c, NULL,p); /* space before = */
      if (c != '=') {
          ungetc(c, in);
          return NULL;
--- 309,321 ----
          }
  	if(c == '=' || isspace(c)) break;
  	*(t++) = tolower(c);
!         GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
      }
  
      *t++ = '\0';
      tag_val = t;
  
!     while (isspace(c)) GET_CHAR(in, c, NULL,p); if (c == EOF) return NULL; /* space before
= */
      if (c != '=') {
          ungetc(c, in);
          return NULL;
***************
*** 290,296 ****
      }
  
      do {
! 	GET_CHAR(in,c,NULL,p);  /* space after = */
      } while (isspace(c));
  
      /* we should allow a 'name' as a value */
--- 322,328 ----
      }
  
      do {
! 	GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;  /* space after = */
      } while (isspace(c));
  
      /* we should allow a 'name' as a value */
***************
*** 298,304 ****
      if (c != '"' && c != '\'') return NULL;
      term = c;
      while(1) {
! 	GET_CHAR(in,c,NULL,p);
  	if(++n == tagbuf_len) {
  	    t[tagbuf_len - 1] = '\0';
  	    return NULL;
--- 330,336 ----
      if (c != '"' && c != '\'') return NULL;
      term = c;
      while(1) {
! 	GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
  	if(++n == tagbuf_len) {
  	    t[tagbuf_len - 1] = '\0';
  	    return NULL;
***************
*** 306,312 ****
  /* Want to accept \" as a valid character within a string. */
  	if (c == '\\') {
  	    *(t++) = c; /* Add backslash */
! 	    GET_CHAR(in,c,NULL,p);
  	    if (c == term) /* Only if */
  		*(--t) = c; /* Replace backslash ONLY for terminator */
  	} else if (c == term) break;
--- 338,344 ----
  /* Want to accept \" as a valid character within a string. */
  	if (c == '\\') {
  	    *(t++) = c; /* Add backslash */
! 	    GET_CHAR(in,c,NULL,p); if (c == EOF) return NULL;
  	    if (c == term) /* Only if */
  		*(--t) = c; /* Replace backslash ONLY for terminator */
  	} else if (c == term) break;
***************
*** 323,329 ****
  
      /* skip initial whitespace */
      while(1) {
!         GET_CHAR(in,c,1,p);
          if(!isspace(c))
              break;
      }
--- 355,361 ----
  
      /* skip initial whitespace */
      while(1) {
!         GET_CHAR(in,c,1,p); if (c == EOF) return 1;
          if(!isspace(c))
              break;
      }
***************
*** 330,336 ****
      /* now get directive */
      while(1) {
          *d++ = tolower(c);
!         GET_CHAR(in,c,1,p);
          if(isspace(c))
              break;
      }
--- 362,368 ----
      /* now get directive */
      while(1) {
          *d++ = tolower(c);
!         GET_CHAR(in,c,1,p); if (c == EOF) return 1;
          if(isspace(c))
              break;
      }


Mime
View raw message