On Thu, Oct 17, 2013 at 6:19 PM, Jim Jagielski <[email protected]> wrote:

> Need to look, but at 1st blush it looks like an
> 
> off-by-1 error
> there.
>

When source length >= dlen, apr_cpystrn() ensures dst[0:dlen - 1] ==
src[0:dlen - 1], hence off-by-1 is useless.

Regards.



> On Oct 17, 2013, at 11:33 AM, Yann Ylavic <[email protected]> wrote:
>
> >
> > Maybe ap_proxy_strncpy() could aso have no "slow" path with this change :
> >
> > Index: modules/proxy/proxy_util.c
> > ===================================================================
> > --- modules/proxy/proxy_util.c    (revision 1533118)
> > +++ modules/proxy/proxy_util.c    (working copy)
> > @@ -90,7 +90,6 @@ APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(proxy, PROXY,
> >  PROXY_DECLARE(apr_status_t) ap_proxy_strncpy(char *dst, const char *src,
> >                                               apr_size_t dlen)
> >  {
> > -    char *thenil;
> >      apr_size_t thelen;
> >
> >      /* special case: really  apr_cpystrn should handle src==NULL*/
> > @@ -98,11 +97,8 @@ PROXY_DECLARE(apr_status_t) ap_proxy_strncpy(char
> >          *dst = '\0';
> >          return APR_SUCCESS;
> >      }
> > -    thenil = apr_cpystrn(dst, src, dlen);
> > -    thelen = thenil - dst;
> > -    /* Assume the typical case is smaller copying into bigger
> > -       so we have a fast return */
> > -    if ((thelen < dlen-1) || ((strlen(src)) == thelen)) {
> > +    thelen = apr_cpystrn(dst, src, dlen) - dst;
> > +    if (thelen < dlen || !src[dlen]) {
> >          return APR_SUCCESS;
> >      }
> >      /* XXX: APR_ENOSPACE would be better */
> > [EOS]
> >
> > Regards,
> > Yann.
> >
>
>

Reply via email to