httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Basant.Kukr...@Sun.COM
Subject Re: Solaris sed based apache filtering module (mod_sed)
Date Sun, 29 Jun 2008 20:01:37 GMT

Solaris ucb sed code works fine when each line has new line character. If new
line character is missing from the end of the file then behaviour was not
friendly. The reason is that sed is a true "line" editor.

$ cat nocrlf.inp
<HTML>
abcd
</HTML>   ---> "no new line here"
$ /usr/ucb/sed -e 's/a/g/g' nocrlf.inp
<HTML>
gbcd

gnu sed behaves properly in above situation.

This is the fix for the regression caused by my previous checkin. If last byte
of the file is not a new line, then last byte was not printing in to the
output.
Steffen <info@apachelounge.com> reported this issue to me.

diff is attached.

Regards,
Basant.

diff -r 2866cd8c83a2 sed1.c
--- a/sed1.c    Sat Jun 28 21:00:41 2008 -0700
+++ b/sed1.c    Sun Jun 29 12:51:49 2008 -0700
@@ -432,11 +432,10 @@ apr_status_t sed_finalize_eval(sed_eval_
             eval->lspend--;
         } else {
             /* Code can probably reach here when last character in output
-             * buffer is not null terminated
+             * buffer is not a newline.
              */
             /* Assure space for NULL */
             append_to_linebuf(eval, "");
-            eval->lspend--;
         }

         *eval->lspend = '\0';


On Sat, Jun 28, 2008 at 09:08:50PM -0700, Basant.Kukreja@Sun.COM wrote:
> mod_sed constant length buffer sizes fix :
> 
> Three buffers linebuf, genbuf and holdbuf are now reallocated dynamically.
> Sed code has 3 buffers :
> linebuf : Store a line.
> holdbuf : store the intemediate hold data (sed h/H commands)
> genbuf : Used in substitution intermediate results.
> 
> Original sed has the limitation on the fixed size buffers.
> In this fix all these 3 buffers are adjusted dynamically. Initial buffer size
> of these buffers is 1024 bytes. When these buffers needs reallocation, their
> new buffer size is aligned to 4KB boundary.
> 
> List of affected files :
> sed1.c libsed.h
> 
> Code has been updated and can be obtained by mercurial as :
> $ hg clone ssh://anon@hg.opensolaris.org/hg/webstack/mod_sed
> 
> It should soon be visible at :
> http://src.opensolaris.org/source/xref/webstack/mod_sed/
> 
> Diff is attached.
> 
> Regards,
> Basant.
> 
> 
> -------------------------------------------------------------
> diff -r 32f5eb1dc14f libsed.h
> --- a/libsed.h  Wed Apr 30 11:52:57 2008 -0700
> +++ b/libsed.h  Sat Jun 28 20:35:06 2008 -0700
> @@ -121,17 +121,14 @@
>  
>      unsigned       lsize;
>      char           *linebuf;
> -    char           *lbend;
>      char           *lspend;
>  
>      unsigned       hsize;
>      char           *holdbuf;
> -    char           *hbend;
>      char           *hspend;
>  
>      unsigned       gsize;
>      char           *genbuf;
> -    char           *gbend;
>      char           *lcomend;
>  
>      apr_file_t    *fcode[NWFILES];
> diff -r 32f5eb1dc14f sed1.c
> --- a/sed1.c    Wed Apr 30 11:52:57 2008 -0700
> +++ b/sed1.c    Sat Jun 28 20:35:06 2008 -0700
> @@ -86,6 +86,151 @@
>      }
>  }
>  
> +#define INIT_BUF_SIZE 1024
> +
> +/*
> + * grow_buffer
> + */
> +static void grow_buffer(apr_pool_t *pool, char **buffer,
> +                        char **spend, unsigned int *cursize,
> +                        unsigned int newsize)
> +{
> +    char* newbuffer = NULL;
> +    int spendsize = 0;
> +    if (*cursize >= newsize)
> +        return;
> +    /* Align it to 4 KB boundary */
> +    newsize = (newsize  + ((1 << 12) - 1)) & ~((1 << 12) -1);
> +    newbuffer = apr_pcalloc(pool, newsize);
> +    if (*spend && *buffer && (*cursize > 0)) {
> +        spendsize = *spend - *buffer;
> +    }
> +    if ((*cursize > 0) && *buffer) {
> +        memcpy(newbuffer, *buffer, *cursize);
> +    }
> +    *buffer = newbuffer;
> +    *cursize = newsize;
> +    if (spend != buffer) {
> +        *spend = *buffer + spendsize;
> +    }
> +}
> +
> +/*
> + * grow_line_buffer
> + */
> +static void grow_line_buffer(sed_eval_t *eval, int newsize)
> +{
> +    grow_buffer(eval->pool, &eval->linebuf, &eval->lspend,
> +                &eval->lsize, newsize);
> +}
> +
> +/*
> + * grow_hold_buffer
> + */
> +static void grow_hold_buffer(sed_eval_t *eval, int newsize)
> +{
> +    grow_buffer(eval->pool, &eval->holdbuf, &eval->hspend,
> +                &eval->hsize, newsize);
> +}
> +
> +/*
> + * grow_gen_buffer
> + */
> +static void grow_gen_buffer(sed_eval_t *eval, int newsize,
> +                            char **gspend)
> +{
> +    if (gspend == NULL) {
> +        gspend = &eval->genbuf;
> +    }
> +    grow_buffer(eval->pool, &eval->genbuf, gspend,
> +                &eval->gsize, newsize);
> +    eval->lcomend = &eval->genbuf[71];
> +}
> +
> +/*
> + * appendmem_to_linebuf
> + */
> +static void appendmem_to_linebuf(sed_eval_t *eval, const char* sz, int len)
> +{
> +    int reqsize = (eval->lspend - eval->linebuf) + len;
> +    if (eval->lsize < reqsize) {
> +        grow_line_buffer(eval, reqsize);
> +    }
> +    memcpy(eval->lspend, sz, len);
> +    eval->lspend += len;
> +}
> +
> +/*
> + * append_to_linebuf
> + */
> +static void append_to_linebuf(sed_eval_t *eval, const char* sz)
> +{
> +    int len = strlen(sz);
> +    /* Copy string including null character */
> +    appendmem_to_linebuf(eval, sz, len + 1);
> +    --eval->lspend; /* lspend will now point to NULL character */
> +}
> +
> +/*
> + * copy_to_linebuf
> + */
> +static void copy_to_linebuf(sed_eval_t *eval, const char* sz)
> +{
> +    eval->lspend = eval->linebuf;
> +    append_to_linebuf(eval, sz);
> +}
> +
> +/*
> + * append_to_holdbuf
> + */
> +static void append_to_holdbuf(sed_eval_t *eval, const char* sz)
> +{
> +    int len = strlen(sz);
> +    int reqsize = (eval->hspend - eval->holdbuf) + len + 1;
> +    if (eval->hsize <= reqsize) {
> +        grow_hold_buffer(eval, reqsize);
> +    }
> +    strcpy(eval->hspend, sz);
> +    /* hspend will now point to NULL character */
> +    eval->hspend += len;
> +}
> +
> +/*
> + * copy_to_holdbuf
> + */
> +static void copy_to_holdbuf(sed_eval_t *eval, const char* sz)
> +{
> +    eval->hspend = eval->holdbuf;
> +    append_to_holdbuf(eval, sz);
> +}
> +
> +/*
> + * append_to_genbuf
> + */
> +static void append_to_genbuf(sed_eval_t *eval, const char* sz, char **gspend)
> +{
> +    int len = strlen(sz);
> +    int reqsize = (*gspend - eval->genbuf) + len + 1;
> +    if (eval->gsize < reqsize) {
> +        grow_gen_buffer(eval, reqsize, gspend);
> +    }
> +    strcpy(*gspend, sz);
> +    /* *gspend will now point to NULL character */
> +    *gspend += len;
> +}
> +
> +/*
> + * copy_to_genbuf
> + */
> +static void copy_to_genbuf(sed_eval_t *eval, const char* sz)
> +{
> +    int len = strlen(sz);
> +    int reqsize = len + 1;
> +    if (eval->gsize < reqsize) {
> +        grow_gen_buffer(eval, reqsize, NULL);
> +    }
> +}
> +
>  /*
>   * sed_init_eval
>   */
> @@ -113,16 +258,16 @@
>      eval->fout = NULL;
>  
>      if (eval->linebuf == NULL) {
> -        eval->linebuf = apr_palloc(eval->pool, LBSIZE + 1);
> -        eval->lbend = eval->linebuf + LBSIZE;
> +        eval->lsize = INIT_BUF_SIZE;
> +        eval->linebuf = apr_pcalloc(eval->pool, eval->lsize);
>      }
>      if (eval->holdbuf == NULL) {
> -        eval->holdbuf = apr_palloc(eval->pool, LBSIZE + 1);
> -        eval->hbend = eval->holdbuf + LBSIZE;
> +        eval->hsize = INIT_BUF_SIZE;
> +        eval->holdbuf = apr_pcalloc(eval->pool, eval->hsize);
>      }
>      if (eval->genbuf == NULL) {
> -        eval->genbuf = apr_palloc(eval->pool, LBSIZE + 1);
> -        eval->gbend = eval->genbuf + LBSIZE;
> +        eval->gsize = INIT_BUF_SIZE;
> +        eval->genbuf = apr_pcalloc(eval->pool, eval->gsize);
>      }
>      eval->lspend = eval->linebuf;
>      eval->hspend = eval->holdbuf;
> @@ -243,18 +388,13 @@
>              eval->lreadyflag = 1;
>              break;
>          }
> -
> -        while ((eval->linebuf + LBSIZE) <= eval->lspend + llen + 1) {
> -            /* XXX : grow buffer */
> -            return APR_EGENERAL;
> -        }
> -
> -        memcpy(eval->lspend, buf, llen);
> -        eval->lspend += llen;
> +        
> +        appendmem_to_linebuf(eval, buf, llen + 1);
> +        --eval->lspend;
> +        /* replace new line character with NULL */
> +        *eval->lspend = '\0';
>          buf += (llen + 1);
>          bufsz -= (llen + 1);
> -
> -        *eval->lspend = '\0';
>          rv = execute(eval);
>          if (rv != 0)
>              return APR_EGENERAL;
> @@ -264,12 +404,7 @@
>  
>      /* Save the leftovers for later */
>      if (bufsz) {
> -        if ((eval->linebuf + LBSIZE) <= eval->lspend + bufsz) {
> -            /* XXX : grow buffer */
> -            return APR_EGENERAL;
> -        }
> -        memcpy(eval->lspend, buf, bufsz);
> -        eval->lspend += bufsz;
> +        appendmem_to_linebuf(eval, buf, bufsz);
>      }
>  
>      return APR_SUCCESS;
> @@ -296,10 +431,12 @@
>              eval->lreadyflag = 0;
>              eval->lspend--;
>          } else {
> -            if ((eval->linebuf + LBSIZE) <= eval->lspend) {
> -                /* XXX : grow buffer */
> -                return APR_EGENERAL;
> -            }
> +            /* Code can probably reach here when last character in output
> +             * buffer is not null terminated 
> +             */
> +            /* Assure space for NULL */
> +            append_to_linebuf(eval, "");
> +            eval->lspend--;
>          }
>  
>          *eval->lspend = '\0';
> @@ -493,8 +630,7 @@
>      lp = eval->linebuf;
>      sp = eval->genbuf;
>      rp = rhsbuf;
> -    while (lp < step_vars->loc1)
> -        *sp++ = *lp++;
> +    sp = place(eval, sp, lp, step_vars->loc1);
>      while(c = *rp++) {
>          if (c == '&') {
>              sp = place(eval, sp, step_vars->loc1, step_vars->loc2);
> @@ -513,27 +649,15 @@
>                  *sp++ = c;
>            } else
>              *sp++ = c;
> -        if (sp == &eval->genbuf[LBSIZE+1]) {
> -            eval_errf(eval, SEDERR_OLTL, eval->lnum);
> -            *--sp = '\0';
> -            rv = APR_EGENERAL;
> -            goto out;
> +        if (sp >= eval->genbuf + eval->gsize) {
> +            /* expand genbuf and set the sp appropriately */
> +            grow_gen_buffer(eval, eval->gsize + 1024, &sp);
>          }
>      }
>      lp = step_vars->loc2;
>      step_vars->loc2 = sp - eval->genbuf + eval->linebuf;
> -    while(*sp++ = *lp++)
> -        if (sp == &eval->genbuf[LBSIZE+1]) {
> -            eval_errf(eval, SEDERR_OLTL, eval->lnum);
> -            *--sp = '\0';
> -            rv = APR_EGENERAL;
> -            break;
> -        }
> -out:
> -    lp = eval->linebuf;
> -    sp = eval->genbuf;
> -    while (*lp++ = *sp++);
> -    eval->lspend = lp-1;
> +    append_to_genbuf(eval, lp, &sp);
> +    copy_to_linebuf(eval, eval->genbuf);
>      return rv;
>  }
>  
> @@ -542,17 +666,15 @@
>   */
>  static char *place(sed_eval_t *eval, char *asp, char *al1, char *al2)
>  {
> -    char *sp, *l1, *l2;
> +    char *sp = asp;
> +    int n = al2 - al1;
> +    int reqsize = (sp - eval->genbuf) + n + 1;
>  
> -    sp = asp;
> -    l1 = al1;
> -    l2 = al2;
> -    while (l1 < l2) {
> -        *sp++ = *l1++;
> -        if (sp == &eval->genbuf[LBSIZE+1])
> -            break;
> +    if (eval->gsize < reqsize) {
> +        grow_gen_buffer(eval, reqsize, &sp);
>      }
> -    return(sp);
> +    memcpy(sp, al1, n);
> +    return sp + n;
>  }
>  
>  /*
> @@ -600,9 +722,7 @@
>              }
>  
>              p1++;
> -            p2 = eval->linebuf;
> -            while(*p2++ = *p1++);
> -            eval->lspend = p2-1;
> +            copy_to_linebuf(eval, p1);
>              eval->jflag++;
>              break;
>  
> @@ -612,45 +732,21 @@
>              break;
>  
>          case GCOM:
> -            p1 = eval->linebuf;
> -            p2 = eval->holdbuf;
> -            while(*p1++ = *p2++);
> -            eval->lspend = p1-1;
> +            copy_to_linebuf(eval, eval->holdbuf);
>              break;
>  
>          case CGCOM:
> -            *eval->lspend++ = '\n';
> -            p1 = eval->lspend;
> -            p2 = eval->holdbuf;
> -            do {
> -                if (p1 == &eval->linebuf[LBSIZE+1]) {
> -                    eval_errf(eval, SEDERR_OLTL, eval->lnum);
> -                    *--p1 = '\0';
> -                    return APR_EGENERAL;
> -                }
> -            } while(*p1++ = *p2++);
> -            eval->lspend = p1-1;
> +            append_to_linebuf(eval, "\n");
> +            append_to_linebuf(eval, eval->holdbuf);
>              break;
>  
>          case HCOM:
> -            p1 = eval->holdbuf;
> -            p2 = eval->linebuf;
> -            while(*p1++ = *p2++);
> -            eval->hspend = p1-1;
> +            copy_to_holdbuf(eval, eval->linebuf);
>              break;
>  
>          case CHCOM:
> -            *eval->hspend++ = '\n';
> -            p1 = eval->hspend;
> -            p2 = eval->linebuf;
> -            do {
> -                if (p1 == &eval->holdbuf[LBSIZE+1]) {
> -                    eval_errf(eval, SEDERR_HSOVERFLOW);
> -                    *--p1 = '\0';
> -                    return APR_EGENERAL;
> -                }
> -            } while(*p1++ = *p2++);
> -            eval->hspend = p1-1;
> +            append_to_holdbuf(eval, "\n");
> +            append_to_holdbuf(eval, eval->linebuf);
>              break;
>  
>          case ICOM:
> @@ -744,7 +840,7 @@
>          case CNCOM:
>              if(eval->aptr > eval->abuf)
>                  arout(eval);
> -            *eval->lspend++ = '\n';
> +            append_to_linebuf(eval, "\n");
>              eval->pending = ipc->next;
>              break;
>  
> @@ -804,17 +900,9 @@
>                                  eval->linebuf);
>              break;
>          case XCOM:
> -            p1 = eval->linebuf;
> -            p2 = eval->genbuf;
> -            while(*p2++ = *p1++);
> -            p1 = eval->holdbuf;
> -            p2 = eval->linebuf;
> -            while(*p2++ = *p1++);
> -            eval->lspend = p2 - 1;
> -            p1 = eval->genbuf;
> -            p2 = eval->holdbuf;
> -            while(*p2++ = *p1++);
> -            eval->hspend = p2 - 1;
> +            copy_to_genbuf(eval, eval->linebuf);
> +            copy_to_linebuf(eval, eval->holdbuf);
> +            copy_to_holdbuf(eval, eval->genbuf);
>              break;
>  
>          case YCOM: 

Mime
View raw message