httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christophe JAILLET <christophe.jail...@wanadoo.fr>
Subject Re: svn commit: r1384924 - in /httpd/httpd/trunk: include/httpd.h server/request.c
Date Tue, 22 Apr 2014 07:47:14 GMT
Hi,

while looking at candidates for backport to synch 2.4.x and trunk, I 
came across this old comment update.

The first part of the comment, against 'ap_os_escape_path', is, IMO, wrong.
We are not guaranteed that, if partial is *not* set, that there will be 
one byte of additional space after the NUL.

If partial *is* set, then we skip the :
     *d++ = '.';
     *d++ = '/';
and extra bytes will be available after the NUL.

But if it is *not* set, I think that there may be (unlikely) cases where 
not space is available at the end.

So, either the comment should be updated or 1 extra byte should be 
allocated, to be safe. In this later case, the comment should also be 
updated to state that in *all* cases, one extra byte is available.
+1 for allocating an extra byte.


Moreover, 'ap_os_escape_path' could be tweaked as in r1485723.


Best regards,

CJ



Le 14/09/2012 23:06, sf@apache.org a écrit :
> Author: sf
> Date: Fri Sep 14 21:06:05 2012
> New Revision: 1384924
>
> URL: http://svn.apache.org/viewvc?rev=1384924&view=rev
> Log:
> ap_sub_req_lookup_dirent() depends on the over-allocation done by
> ap_make_full_path and ap_escape_uri, so let's document it so that it is not
> accidentally removed.
>
> Modified:
>      httpd/httpd/trunk/include/httpd.h
>      httpd/httpd/trunk/server/request.c
>
> Modified: httpd/httpd/trunk/include/httpd.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/httpd.h?rev=1384924&r1=1384923&r2=1384924&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/httpd.h (original)
> +++ httpd/httpd/trunk/include/httpd.h Fri Sep 14 21:06:05 2012
> @@ -1579,7 +1579,9 @@ AP_DECLARE(char *) ap_escape_path_segmen
>    * @param p The pool to allocate from
>    * @param path The path to convert
>    * @param partial if set, assume that the path will be appended to something
> - *        with a '/' in it (and thus does not prefix "./")
> + *        with a '/' in it (and thus does not prefix "./").
> + *        If not set, there will be one byte of additional space after the
> + *        NUL, to allow the caller to append a '/'.
>    * @return The converted URL
>    */
>   AP_DECLARE(char *) ap_os_escape_path(apr_pool_t *p, const char *path, int partial);
> @@ -1692,7 +1694,8 @@ AP_DECLARE(char *) ap_make_dirstr_parent
>    * @param a The pool to allocate from
>    * @param dir The directory name
>    * @param f The filename
> - * @return A copy of the full path
> + * @return A copy of the full path, with one byte of extra space after the NUL
> + *         to allow the caller to add a trailing '/'.
>    * @note Never consider using this function if you are dealing with filesystem
>    * names that need to remain canonical, unless you are merging an apr_dir_read
>    * path and returned filename.  Otherwise, the result is not canonical.
>
> Modified: httpd/httpd/trunk/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1384924&r1=1384923&r2=1384924&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/request.c (original)
> +++ httpd/httpd/trunk/server/request.c Fri Sep 14 21:06:05 2012
> @@ -2160,7 +2160,7 @@ AP_DECLARE(request_rec *) ap_sub_req_loo
>       }
>   
>       if (rnew->finfo.filetype == APR_DIR) {
> -        /* ap_make_full_path overallocated the buffers
> +        /* ap_make_full_path and ap_escape_uri overallocated the buffers
>            * by one character to help us out here.
>            */
>           strcat(rnew->filename, "/");

Mime
View raw message