Re: [PATCH] SEGV and missing Location on redirects

[Randy Terbush <ra...@zyzzyva.com>]

+1

> Two problems were introduced by the two "error responses have wrong headers"
> patches committed a couple weeks ago.  The first is that a NULL table
> causes mod_log_config to SIGSEGV on a call to table_get; the second is
> that Location needs to be added to the output header table for redirects.
> 
> These two fixes, originally noted with different patches from Ed Korthof,
> did not appear in the 5/22 status report.  These are showstopper problems
> introduced in 1.2b11-dev, so we can't make a new release without fixing
> them in some fashion.  The following patch fixes both.
> 
> .....Roy
> 
> Index: alloc.h
> ===================================================================
> RCS file: /export/home/cvs/apache/src/alloc.h,v
> retrieving revision 1.20
> diff -c -r1.20 alloc.h
> *** alloc.h	1997/05/14 19:22:52	1.20
> --- alloc.h	1997/05/27 00:25:09
> ***************
> *** 144,149 ****
> --- 144,150 ----
>   
>   table *make_table (pool *p, int nelts);
>   table *copy_table (pool *p, const table *);     
> + void clear_table (table *);     
>   char *table_get (const table *, const char *);
>   void table_set (table *, const char *name, const char *val);
>   void table_merge (table *, const char *name, const char *more_val);
> Index: alloc.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/alloc.c,v
> retrieving revision 1.27
> diff -c -r1.27 alloc.c
> *** alloc.c	1997/04/24 23:35:18	1.27
> --- alloc.c	1997/05/27 00:25:10
> ***************
> *** 552,557 ****
> --- 552,562 ----
>       return copy_array (p, t);
>   }
>   
> + void clear_table (table *t)
> + {
> +     t->nelts = 0;
> + }
> + 
>   array_header *table_elts (table *t) { return t; }
>   
>   char *table_get (const table *t, const char *key)
> Index: http_protocol.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/http_protocol.c,v
> retrieving revision 1.121
> diff -c -r1.121 http_protocol.c
> *** http_protocol.c	1997/05/14 19:22:53	1.121
> --- http_protocol.c	1997/05/27 00:25:12
> ***************
> *** 1692,1705 ****
>       }
>   
>       if (!r->assbackwards) {
>     
>   	/* For all HTTP/1.x responses for which we generate the message,
>   	 * we need to avoid inheriting the "normal status" header fields
>   	 * that may have been set by the request handler before the
> ! 	 * error or redirect.
>   	 */
>   	r->headers_out = r->err_headers_out;
> ! 	r->err_headers_out = NULL;
>   	r->content_language = NULL;
>   	r->content_languages = NULL;
>   	r->content_encoding = NULL;
> --- 1692,1712 ----
>       }
>   
>       if (!r->assbackwards) {
> + 	table *tmp = r->headers_out;
>     
>   	/* For all HTTP/1.x responses for which we generate the message,
>   	 * we need to avoid inheriting the "normal status" header fields
>   	 * that may have been set by the request handler before the
> ! 	 * error or redirect, except for Location on external redirects.
>   	 */
>   	r->headers_out = r->err_headers_out;
> ! 	r->err_headers_out = tmp;
> ! 	clear_table(r->err_headers_out);
> ! 
> ! 	if (location && *location
> ! 	             && (is_HTTP_REDIRECT(status) || status == HTTP_CREATED))
> ! 	    table_set(r->headers_out, "Location", location);
> ! 
>   	r->content_language = NULL;
>   	r->content_languages = NULL;
>   	r->content_encoding = NULL;