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, [email protected] 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, "/");
