httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Kew <n...@webthing.com>
Subject Re: svn commit: r573903 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Date Sun, 09 Sep 2007 14:16:24 GMT
On Sun, 09 Sep 2007 11:51:37 +0200
Ruediger Pluem <rpluem@apache.org> wrote:

> 
> 
> On 09/08/2007 10:29 PM, niq@apache.org wrote:
> > Author: niq
> > Date: Sat Sep  8 13:29:14 2007
> > New Revision: 573903
> > 
> > URL: http://svn.apache.org/viewvc?rev=573903&view=rev
> > Log:
> > mod_proxy: Don't lose bytes when a response line arrives in small
> > chunks. PR 40894
> > 
> > Modified:
> >     httpd/httpd/trunk/CHANGES
> >     httpd/httpd/trunk/modules/proxy/proxy_util.c
> > 
> >
> > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=573903&r1=573902&r2=573903&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) +++
> > httpd/httpd/trunk/modules/proxy/proxy_util.c Sat Sep  8 13:29:14
> > 2007 @@ -994,12 +994,14 @@ len = (bufflen-1)-(pos-buff);
> >                  }
> >                  if (len > 0) {
> > -                    pos = apr_cpystrn(pos, response, len);
> > +                    memcpy(pos, response, len);
> > +                    pos += len;
> 
> I am slightly confused here: IMHO apr_cpystrn does exactly what you
> do here (it terminates pos with \0 and sets the pointer of pos to the
> terminating \0) But it avoids reading over a possible terminating \0
> in response which seems good to me.

  char *buf = apr_pcalloc(p, 20);
  char *pos = apr_cpystrn(buf, "123456", 6);
  pos = apr_cpystrn(buf, "ABCDEF", 6);
  *pos = '\n';
  puts(buf);

12345ABCDE

The bug we're fixing is that the last byte of each string is getting
overwritten, because the length argument to apr_cpystrn includes the
terminating null.  The OP had a slightly different fix (increment len);
I took the view that the NULLs were unnecessary whilst in a
byte-counting loop and modified it as above.

As for running past a \0 (which would imply a malformed input stream),
the caller expects a string, so the first NULL will terminate it
either way.  Also either way the terminating condition is the
APR_ASCII_LF, or an input error.

-- 
Nick Kew

Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/

Mime
View raw message