From: "Brian Pane" <[EMAIL PROTECTED]>
Sent: Wednesday, August 15, 2001 12:05 AM
> William A. Rowe, Jr. wrote:
> [...]
>
> >+static void remove_items(apr_pool_t *p, apr_array_header_t *remove,
> >+ apr_hash_t *mappings, apr_size_t member)
> >+{
> >+ attrib_info *suffix = (attrib_info *) remove->elts;
> >+ int i;
> >+ for (i = 0; i < remove->nelts; i++) {
> >+ extension_info *exinfo =
> >+ (extension_info*)apr_hash_get(mappings,
> >+ suffix[i].name,
> >+ APR_HASH_KEY_STRING);
> >+ /* If we find the entry, the given member isn't NULL,
> >+ * and this exinfo wasn't allocated from the current pool,
> >+ * then we must copy before we remove the given member.
> >+ */
> >
> Why is it necessary to make a copy before overwriting the
> field in exinfo with NULL?
The hash members are _not_ copied by the apr_hash functions! This is why the
original patch had broken Apache.
The hash member has 5 pointers. This function nullifies one of them. If you don't
copy first, you are actually wiping out the registered value of the original config.
Worse, (in the overlay case) if you replace one of those pointers due to an .htaccess
override, you have just pointed into the r-> pool, so once this request is released,
the next request would segfault the server.
> >
> >+ if (!base->extension_mappings && !add->extension_mappings)
> >+ new->extension_mappings = NULL;
> >+ else if (base->extension_mappings
> >+ && ((base->pool == p) || !(add->extension_mappings
> >+ || add->handlers_remove
> >+ || add->types_remove
> >+ || add->encodings_remove)))
> >+ new->extension_mappings = base->extension_mappings;
> >+ else {
> >+ new->extension_mappings = apr_hash_make(p);
> >+ if (base->extension_mappings)
> > overlay_extension_mappings(p, base->extension_mappings,
> > new->extension_mappings);
> >+ }
> >
> One minor problem here (and I can't remember if it will be
> a problem in practice): this logic can call overlay_extension_mappings
> with one of its latter two arguments equal to NULL, and
> overlay_extension_mappings
> will dereference NULL without checking.
That's why we test first. new->extension_mappings is guarenteed to be non-null.
Bill
