httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: [PATCH] Fix 1.3.17 crash in util_uri.c, etc.
Date Fri, 16 Feb 2001 20:32:42 GMT

From: "Jeffrey W. Baker" <jwbaker@acm.org>
Sent: Sunday, February 11, 2001 1:55 AM


> Hello,
> 
> The patch I promised on Friday is attached.  I rewrote
> ap_unparse_uri_components and changed ap_default_port_for_scheme to avoid
> the crash bugs in 1.3.17.  My new version of ap_unparse also fixes bugs in
> the previous routine that allowed it to emit bogus URIs.  The new version
> is just as branch-happy as the old version, but takes only one trip
> through ap_pstrcat and also doesn't allow bogus URIs.
> 
> The new code compiles and runs on Linux 2.4.1/glibs 2.2.1 here at home,
> passes 80 tests of my own, and passes modules/test/mod_test_util_uri.c.
> 
> Let me know if there are any problems.
> 
> -jwb
> 


--- src/main/util_uri.c.orig Sat Feb 10 08:53:10 2001
+++ src/main/util_uri.c Sat Feb 10 23:49:32 2001
@@ -87,6 +87,9 @@
 {
     schemes_t *scheme;
 
+    if (scheme_str == NULL)
+        return 0;
+
     for (scheme = schemes; scheme->name != NULL; ++scheme)
         if (strcasecmp(scheme_str, scheme->name) == 0)
             return scheme->default_port;
@@ -167,61 +170,116 @@
 
 /* Unparse a uri_components structure to an URI string.
  * Optionally suppress the password for security reasons.
+ * See also RFC 2396.
  */
 API_EXPORT(char *) ap_unparse_uri_components(pool *p,
                                              const uri_components * uptr,
                                              unsigned flags)
 {
-    char *ret = "";
-
-    /* If suppressing the site part, omit both user name & scheme://hostname */
+    char *parts[16]; /* 16 distinct parts of a URI */
+    int i, j = 0;
+    
+    for (i = 0; i < 16; i++)
+        parts[i] = NULL;
+    /* If suppressing the site part, omit all of scheme://user:pass@host:port */
     if (!(flags & UNP_OMITSITEPART)) {
 
-        /* Construct a "user:password@" string, honoring the passed UNP_ flags: */
-        if (uptr->user || uptr->password)
-            ret = ap_pstrcat(p,
-                             (uptr->user
-                              && !(flags & UNP_OMITUSER)) ? uptr->user : "",
-                             (uptr->password
-                              && !(flags & UNP_OMITPASSWORD)) ? ":" : "",
-                             (uptr->password && !(flags & UNP_OMITPASSWORD))
-                             ? ((flags & UNP_REVEALPASSWORD) ? uptr->
-                                password : "XXXXXXXX")
-                             : "", "@", NULL);
-
-        /* Construct scheme://site string */
-        if (uptr->hostname) {
-            int is_default_port;
-
-            is_default_port =
-                (uptr->port_str == NULL ||
-                 uptr->port == 0 ||
-                 uptr->port == ap_default_port_for_scheme(uptr->scheme));
-
-            ret = ap_pstrcat(p,
-                             uptr->scheme, "://", ret,
-                             uptr->hostname ? uptr->hostname : "",
-                             is_default_port ? "" : ":",
-                             is_default_port ? "" : uptr->port_str, NULL);
+        /* if the user passes in a scheme, we'll assume an absoluteURI */
+        if (uptr->scheme) {
+            parts[j++] = uptr->scheme;
+            parts[j++] = ":";
+        }
+        
+        /* handle the hier_part */
+        if (uptr->user || uptr->password || uptr->hostname) {
+            
+            /* this stuff requires absoluteURI, so we have to add the scheme */
+            if (!uptr->scheme) {
+                uptr->scheme = DEFAULT_URI_SCHEME;
+                
+                parts[j++] = DEFAULT_URI_SCHEME;
+                parts[j++] = ":";
+            }
+            
+            parts[j++] = "//";
+            
+            /* userinfo requires hostport */
+            if (uptr->hostname && (uptr->user || uptr->password)) {
+                if (uptr->user && !(flags & UNP_OMITUSER))
+                    parts[j++] = uptr->user;
+                
+                if (uptr->password && !(flags & UNP_OMITPASSWORD)) {
+                    parts[j++] = ":";
+
+                    if (flags & UNP_REVEALPASSWORD)
+                        parts[j++] = uptr->password;
+                    else
+                        parts[j++] = "XXXXXXXX";
+                }    
+
+                parts[j++] = "@";
+            }                
+            
+            /* If we get here, there must be a hostname. */
+            parts[j++] = uptr->hostname;
+            
+            /* Emit the port.  A small beautification
+             * prevents http://host:80/ and similar visual blight.
+             */
+            if (uptr->port_str &&
+                !(uptr->port   &&
+                  uptr->scheme &&
+                  uptr->port == ap_default_port_for_scheme(uptr->scheme))) {
+
+                parts[j++] = ":";
+                parts[j++] = uptr->port_str;
+            }
         }
     }
-
-    /* Should we suppress all path info? */
+        
     if (!(flags & UNP_OMITPATHINFO)) {
-        /* Append path, query and fragment strings: */
-        ret = ap_pstrcat(p,
-                         ret,
-                         uptr->path ? uptr->path : "",
-                         (uptr->query && !(flags & UNP_OMITQUERY)) ? "?" :
"",
-                         (uptr->query
-                          && !(flags & UNP_OMITQUERY)) ? uptr->query : "",
-                         (uptr->fragment
-                          && !(flags & UNP_OMITQUERY)) ? "#" : NULL,
-                         (uptr->fragment
-                          && !(flags & UNP_OMITQUERY)) ? uptr->
-                         fragment : NULL, NULL);
+        
+        
+        /* We must ensure we don't put out a hier_part and a rel_path */
+        if (j && uptr->path && *uptr->path != '/')
+            parts[j++] = "/";
+        
+        parts[j++] = uptr->path;
+
+        if (!(flags & UNP_OMITQUERY)) {
+            if (uptr->query) {
+                parts[j++] = "?";
+                parts[j++] = uptr->query;
+            }
+            
+            if (uptr->fragment) {
+                parts[j++] = "#";
+                parts[j++] = uptr->fragment;
+            }
+        }
     }
-    return ret;
+
+    /* Ugly, but correct and probably faster than ap_vsnprintf. */
+    return ap_pstrcat(p,
+        parts[0],
+        parts[1],
+        parts[2],
+        parts[3],
+        parts[4],
+        parts[5],
+        parts[6],
+        parts[7],
+        parts[8],
+        parts[9],
+        parts[10],
+        parts[11],
+        parts[12],
+        parts[13],
+        parts[14],
+        parts[15],
+        NULL
+    );
 }
 
 /* The regex version of parse_uri_components has the advantage that it is
--- src/include/util_uri.h.orig Sat Feb 10 21:22:35 2001
+++ src/include/util_uri.h Sat Feb 10 22:32:15 2001
@@ -80,6 +80,8 @@
 #define DEFAULT_SNEWS_PORT 563
 #define DEFAULT_PROSPERO_PORT 1525 /* WARNING: conflict w/Oracle */
 
+#define DEFAULT_URI_SCHEME "http"
+
 /* Flags passed to unparse_uri_components(): */
 #define UNP_OMITSITEPART (1U<<0) /* suppress "scheme://user@site:port" */
 #define UNP_OMITUSER  (1U<<1) /* Just omit user */

========================================

Ignorning my next comment, couldn't the following bit simply be a memset(parts, 0, sizeof(parts));
?

+    for (i = 0; i < 16; i++)
+        parts[i] = NULL;



That point aside, wouldn't the entire patch easily grokked by doing a 

 API_EXPORT(char *) ap_unparse_uri_components(pool *p,
                                              const uri_components * uptr,
                                              unsigned flags)
 {
      uri_components retptr,
      retptr = *uptr;

Fill in the missing components with a constant empty string, balance the elements
as you are already doing, and then merging the list of members rather than
incrementing a string array index through the pieces?

I'm thinking pure legibility here.


Mime
View raw message