httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brad Nicholes" <BNICHO...@novell.com>
Subject Re: [patch] again, mod_userdir
Date Mon, 12 Feb 2001 18:06:27 GMT
Bill,
    The patch seemed to work fine in general on NetWare except for one case.  If the userdir
is defined as vol1:/users/ the resulting directory path is vol1:/users//user which is invalid
on NetWare because of the double /.  If I omit the trailing / on the Userdir definition, then
everything works find.  The same problem occurs if I define Userdir as "http:/www.domain.com/users/"
 .  On lines #312:

            filename = ap_pstrcat(r->pool, userdir, "/", w, NULL);

and #319:

            redirect = ap_pstrcat(r->pool, userdir, "/", w, dname, NULL);

where the filename or redirect is being contructed, it should check for a trailing / first
before blindly concatenating one.



>>> wrowe@rowe-clan.net Monday, February 12, 2001 12:11:01 AM >>>
From: "Greg Stein" <gstein@lyra.org>
Sent: Saturday, February 10, 2001 4:37 PM


> Seems fine, although I don't like returning DECLINED. That feels like
> "silent failure". Given that we could be subtly changing the semantics on
> some people's configurations (e.g. they relied on a broken form working),
> then I think it would be best to log (misconfigured) and error and return
> HTTP_INTERNAL_SERVER_ERROR.
> 
> Even better would be to catch the error in the UserDir config command. In
> that case, right before each "return DECLINED", we could insert a comment
> stating /* shouldn't happen; erroneous form caugh in cmd_userdir */  (or
> whatever the func name is)
> 
> The code below is simpler by virtue of looking for is_absolute before
> looking for ':' to mean a redirect. Good choice.

And good point... here's the updated patch.  Two more folks actually reading
this patch would be good so I can get it committed :-)

Index: modules/standard/mod_userdir.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_userdir.c,v
retrieving revision 1.48
diff -u -r1.48 mod_userdir.c
--- modules/standard/mod_userdir.c 2001/01/25 22:59:36 1.48
+++ modules/standard/mod_userdir.c 2001/02/12 07:10:20
@@ -169,6 +169,14 @@
          * If the first (only?) value isn't one of our keywords, just copy
          * the string to the userdir string.
          */
+        if (!ap_os_is_path_absolute(arg) && !strchr(arg, ':'))
+#if defined(WIN32) || defined(NETWARE)
+            return "UserDir must specify an absolute redirect or absolute "
+                   "file path";
+#else
+            if (strchr(arg, '*'))
+                 return "UserDir cannot specify a both a relative path and '*' substitution"
+#endif
         s_cfg->userdir = ap_pstrdup(cmd->pool, arg);
 #if defined(WIN32) || defined(OS2) || defined(NETWARE)
         /* This is an incomplete path, so we cannot canonicalize it yet.
@@ -207,7 +215,6 @@
     const char *userdirs = s_cfg->userdir;
     const char *w, *dname;
     char *redirect;
-    char *x = NULL;
     struct stat statbuf;
 
     /*
@@ -266,43 +273,62 @@
     while (*userdirs) {
         const char *userdir = ap_getword_conf(r->pool, &userdirs);
         char *filename = NULL;
-#if defined(NETWARE) || defined(HAVE_DRIVE_LETTERS)
         int is_absolute = ap_os_is_path_absolute(userdir);
-#endif  
 
-        if (strchr(userdir, '*'))
-            x = ap_getword(r->pool, &userdir, '*');
-
- if (userdir[0] == '\0' || userdir[0] == '/') {
-            if (x) {
-#if defined(NETWARE) || defined(HAVE_DRIVE_LETTERS)
-                if (strchr(x, ':') && !is_absolute )
-#else /* !(NETWARE || HAVE_DRIVE_LETTERS) */
-                if (strchr(x, ':'))
-#endif
-  {
-                    redirect = ap_pstrcat(r->pool, x, w, userdir, dname, NULL);
-                    ap_table_setn(r->headers_out, "Location", redirect);
-                    return REDIRECT;
-                }
-                else
-                    filename = ap_pstrcat(r->pool, x, w, userdir, NULL);
+        if (strchr(userdir, '*')) {
+            /* token '*' embedded:
+             */
+            char *x = ap_getword(r->pool, &userdir, '*');
+            if (is_absolute) {
+                /* token '*' within absolute path
+                 * serves [UserDir arg-pre*][user][UserDir arg-post*]
+                 * /somepath/ * /somedir + /~smith -> /somepath/smith/somedir
+                 */
+                filename = ap_pstrcat(r->pool, x, w, userdir, NULL);
             }
-            else
-                filename = ap_pstrcat(r->pool, userdir, "/", w, NULL);
+            else if (strchr(x, ':')) {
+                /* token '*' within a redirect path
+                 * serves [UserDir arg-pre*][user][UserDir arg-post*]
+                 * http://server/user/ * + /~smith/foo -> http://server/user/smith/foo

+                 */
+                redirect = ap_pstrcat(r->pool, x, w, userdir, dname, NULL);
+                ap_table_setn(r->headers_out, "Location", redirect);
+                return REDIRECT;
+            }
+            else {
+                /* Not a redirect, not an absolute path, '*' token:
+                 * serves [homedir]/[UserDir arg]
+                 * something/ * /public_html
+                 * Shouldn't happen, we trap for this in set_user_dir
+                 */
+                return DECLINED;
+            }
         }
-#if defined(NETWARE) || defined(HAVE_DRIVE_LETTERS)
-        else if (strchr(userdir, ':') && !is_absolute ) {
-#else /* !(NETWARE || HAVE_DRIVE_LETTERS) */
+        else if (is_absolute) {
+            /* An absolute path, no * token:
+             * serves [UserDir arg]/[user]
+             * /home + /~smith -> /home/smith
+             */
+            filename = ap_pstrcat(r->pool, userdir, "/", w, NULL);
+        }
         else if (strchr(userdir, ':')) {
-#endif
+            /* A redirect, not an absolute path, no * token:
+             * serves [UserDir arg]/[user][dname]
+             * http://server/ + /~smith/foo -> http://server/smith/foo 
+             */
             redirect = ap_pstrcat(r->pool, userdir, "/", w, dname, NULL);
             ap_table_setn(r->headers_out, "Location", redirect);
             return REDIRECT;
         }
         else {
+            /* Not a redirect, not an absolute path, no * token:
+             * serves [homedir]/[UserDir arg]
+             * e.g. /~smith -> /home/smith/public_html
+             */
 #if defined(WIN32) || defined(NETWARE)
-            /* Need to figure out home dirs on NT and NetWare */
+            /* Need to figure out home dirs on NT and NetWare 
+             * Shouldn't happen here, though, we trap for this in set_user_dir
+             */
             return DECLINED;
 #else                           /* WIN32 & NetWare */
             struct passwd *pw;



Mime
View raw message