Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Wed, 14 Feb 2001 11:28:39 +0100
From: Martin Kraemer <Martin.Kraemer@Fujitsu-Siemens.com>
To: new-httpd@apache.org
Subject: Re: [PATCH] Fix 1.3.17 crash in util_uri.c, etc.
Message-ID: <20010214112839.A34417@deejai2.mch.fsc.net>
References: <Pine.LNX.4.21.0102102350060.6815-200000@desktop>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <Pine.LNX.4.21.0102102350060.6815-200000@desktop>;
 from jwbaker@acm.org on Sat, Feb 10, 2001 at 11:55:51PM -0800

On Sat, Feb 10, 2001 at 11:55:51PM -0800, Jeffrey W. Baker wrote:
> 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

It looks good, although in the line
   29	                uptr->scheme = DEFAULT_URI_SCHEME;
you assign to a "const" structure, and some compilers dislike that.

Also, the "fragment" stuff should IMHO be deleted, because fragments
are a "client-only" interpretation, they should not go on the wire
(or be handled in the server).

   Martin


Here's the function again (easier to read than the context diff):
    1	/* Unparse a uri_components structure to an URI string.
    2	 * Optionally suppress the password for security reasons.
    3	 * See also RFC 2396.
    4	 */
    5	API_EXPORT(char *) ap_unparse_uri_components(pool *p,
    6	                                             const uri_components * uptr,
    7	                                             unsigned flags)
    8	{
    9	    char *parts[16]; /* 16 distinct parts of a URI */
   10	    int i, j = 0;
   11	    
   12	    for (i = 0; i < 16; i++)
   13	        parts[i] = NULL;
   14	        
   15	    /* If suppressing the site part, omit all of scheme://user:pass@host:port */
   16	    if (!(flags & UNP_OMITSITEPART)) {
   17	
   18	        /* if the user passes in a scheme, we'll assume an absoluteURI */
   19	        if (uptr->scheme) {
   20	            parts[j++] = uptr->scheme;
   21	            parts[j++] = ":";
   22	        }
   23	        
   24	        /* handle the hier_part */
   25	        if (uptr->user || uptr->password || uptr->hostname) {
   26	            
   27	            /* this stuff requires absoluteURI, so we have to add the scheme */
   28	            if (!uptr->scheme) {
   29	                uptr->scheme = DEFAULT_URI_SCHEME;
   30	                
   31	                parts[j++] = DEFAULT_URI_SCHEME;
   32	                parts[j++] = ":";
   33	            }
   34	            
   35	            parts[j++] = "//";
   36	            
   37	            /* userinfo requires hostport */
   38	            if (uptr->hostname && (uptr->user || uptr->password)) {
   39	                if (uptr->user && !(flags & UNP_OMITUSER))
   40	                    parts[j++] = uptr->user;
   41	                
   42	                if (uptr->password && !(flags & UNP_OMITPASSWORD)) {
   43	                    parts[j++] = ":";
   44	
   45	                    if (flags & UNP_REVEALPASSWORD)
   46	                        parts[j++] = uptr->password;
   47	                    else
   48	                        parts[j++] = "XXXXXXXX";
   49	                }    
   50	
   51	                parts[j++] = "@";
   52	            }                
   53	            
   54	            /* If we get here, there must be a hostname. */
   55	            parts[j++] = uptr->hostname;
   56	            
   57	            /* Emit the port.  A small beautification
   58	             * prevents http://host:80/ and similar visual blight.
   59	             */
   60	            if (uptr->port_str &&
   61	                !(uptr->port   &&
   62	                  uptr->scheme &&
   63	                  uptr->port == ap_default_port_for_scheme(uptr->scheme))) {
   64	
   65	                parts[j++] = ":";
   66	                parts[j++] = uptr->port_str;
   67	            }
   68	        }
   69	    }
   70	        
   71	    if (!(flags & UNP_OMITPATHINFO)) {
   72	        
   73	        
   74	        /* We must ensure we don't put out a hier_part and a rel_path */
   75	        if (j && uptr->path && *uptr->path != '/')
   76	            parts[j++] = "/";
   77	        
   78	        parts[j++] = uptr->path;
   79	
   80	        if (!(flags & UNP_OMITQUERY)) {
   81	            if (uptr->query) {
   82	                parts[j++] = "?";
   83	                parts[j++] = uptr->query;
   84	            }
   85	            
   86	            if (uptr->fragment) {
   87	                parts[j++] = "#";
   88	                parts[j++] = uptr->fragment;
   89	            }
   90	        }
   91	    }
   92	
   93	    /* Ugly, but correct and probably faster than ap_vsnprintf. */
   94	    return ap_pstrcat(p,
   95	        parts[0],
   96	        parts[1],
   97	        parts[2],
   98	        parts[3],
   99	        parts[4],
  100	        parts[5],
  101	        parts[6],
  102	        parts[7],
  103	        parts[8],
  104	        parts[9],
  105	        parts[10],
  106	        parts[11],
  107	        parts[12],
  108	        parts[13],
  109	        parts[14],
  110	        parts[15],
  111	        NULL
  112	    );
  113	}
-- 
<Martin.Kraemer@Fujitsu-Siemens.com>    |       Fujitsu Siemens
       <martin@apache.org>              |   81730  Munich,  Germany