Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Date: 14 Jan 2002 13:43:24 -0000
Message-ID: <20020114134324.41338.qmail@icarus.apache.org>
From: gstein@apache.org
To: httpd-2.0-cvs@apache.org
Subject: cvs commit: httpd-2.0/modules/dav/main mod_dav.c mod_dav.h

gstein      02/01/14 05:43:24

  Modified:    modules/dav/fs repos.c
               modules/dav/main mod_dav.c mod_dav.h
  Log:
  Revamp how mod_dav asks its provider to generate a GET response.
  
  * eliminate the get_pathname and free_file vtable functions. add the
      deliver() function to have the provider deliver the content
      straight into a filter [stack].
  
  * eliminate readable streams -- they are now obsolete. this gets rid
      of the read_stream vtable function and DAV_MODE_READ*
  
  * implement a deliver() function for the FS provider. this simply
      constructs a bucket and EOS and shoves them into the filter. note
      that this is debug code only, so the "large file" issue handled by
      the core's default handler doesn't count here.
  
  * allow the provider to handle GET for any resource type and for
      collections. this moves the checks into the provider.
  
  Revision  Changes    Path
  1.56      +57 -33    httpd-2.0/modules/dav/fs/repos.c
  
  Index: repos.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/dav/fs/repos.c,v
  retrieving revision 1.55
  retrieving revision 1.56
  diff -u -r1.55 -r1.56
  --- repos.c	16 May 2001 17:24:12 -0000	1.55
  +++ repos.c	14 Jan 2002 13:43:24 -0000	1.56
  @@ -75,7 +75,6 @@
   
   /* to assist in debugging mod_dav's GET handling */
   #define DEBUG_GET_HANDLER       0
  -#define DEBUG_PATHNAME_STYLE    0
   
   #define DAV_FS_COPY_BLOCKSIZE	16384	/* copy 16k at a time */
   
  @@ -804,8 +803,6 @@
       apr_int32_t flags;
   
       switch (mode) {
  -    case DAV_MODE_READ:
  -    case DAV_MODE_READ_SEEKABLE:
       default:
   	flags = APR_READ | APR_BINARY;
   	break;
  @@ -850,18 +847,6 @@
       return NULL;
   }
   
  -static dav_error * dav_fs_read_stream(dav_stream *stream,
  -				      void *buf, apr_size_t *bufsize)
  -{
  -    if (apr_file_read(stream->f, buf, bufsize) != APR_SUCCESS) {
  -	/* ### use something besides 500? */
  -	return dav_new_error(stream->p, HTTP_INTERNAL_SERVER_ERROR, 0,
  -			     "An error occurred while reading from a "
  -			     "resource.");
  -    }
  -    return NULL;
  -}
  -
   static dav_error * dav_fs_write_stream(dav_stream *stream,
   				       const void *buf, apr_size_t bufsize)
   {
  @@ -895,11 +880,16 @@
       return NULL;
   }
   
  +
  +#if DEBUG_GET_HANDLER
  +
  +/* only define set_headers() and deliver() for debug purposes */
  +
  +
   static dav_error * dav_fs_set_headers(request_rec *r,
   				      const dav_resource *resource)
   {
       /* ### this function isn't really used since we have a get_pathname */
  -#if DEBUG_GET_HANDLER
       if (!resource->exists)
   	return NULL;
   
  @@ -919,25 +909,60 @@
       /* ### how to set the content type? */
       /* ### until this is resolved, the Content-Type header is busted */
   
  -#endif
  -
       return NULL;
   }
   
  -#if DEBUG_PATHNAME_STYLE
  -static const char * dav_fs_get_pathname(
  -    const dav_resource *resource,
  -    void **free_handle_p)
  +static dav_error * dav_fs_deliver(const dav_resource *resource,
  +                                  ap_filter_t *output)
   {
  -    return resource->info->pathname;
  -}
  -#endif
  +    apr_pool_t *pool = resource->pool;
  +    apr_bucket_brigade *bb;
  +    apr_file_t *fd;
  +    apr_status_t status;
  +    apr_bucket *bkt;
   
  -static void dav_fs_free_file(void *free_handle)
  -{
  -    /* nothing to free ... */
  +    /* Check resource type */
  +    if (resource->type != DAV_RESOURCE_TYPE_REGULAR
  +        && resource->type != DAV_RESOURCE_TYPE_VERSION
  +        && resource->type != DAV_RESOURCE_TYPE_WORKING) {
  +        return dav_new_error(pool, HTTP_CONFLICT, 0,
  +                             "Cannot GET this type of resource.");
  +    }
  +    if (resource->collection) {
  +        return dav_new_error(pool, HTTP_CONFLICT, 0,
  +                             "There is no default response to GET for a "
  +                             "collection.");
  +    }
  +
  +    if ((status = apr_file_open(&fd, resource->info->pathname,
  +                                APR_READ | APR_BINARY, 0,
  +                                pool)) != APR_SUCCESS) {
  +        return dav_new_error(pool, HTTP_FORBIDDEN, 0,
  +                             "File permissions deny server access.");
  +    }
  +
  +    bb = apr_brigade_create(pool);
  +
  +    /* ### this does not handle large files. but this is test code anyway */
  +    bkt = apr_bucket_file_create(fd, 0,
  +                                 (apr_size_t)resource->info->finfo.size,
  +                                 pool);
  +    APR_BRIGADE_INSERT_TAIL(bb, bkt);
  +
  +    bkt = apr_bucket_eos_create();
  +    APR_BRIGADE_INSERT_TAIL(bb, bkt);
  +
  +    if ((status = ap_pass_brigade(output, bb)) != APR_SUCCESS) {
  +        return dav_new_error(pool, HTTP_FORBIDDEN, 0,
  +                             "Could not write contents to filter.");
  +    }
  +
  +    return NULL;
   }
   
  +#endif /* DEBUG_GET_HANDLER */
  +
  +
   static dav_error * dav_fs_create_collection(dav_resource *resource)
   {
       dav_resource_private *ctx = resource->info;
  @@ -1714,16 +1739,15 @@
       dav_fs_is_parent_resource,
       dav_fs_open_stream,
       dav_fs_close_stream,
  -    dav_fs_read_stream,
       dav_fs_write_stream,
       dav_fs_seek_stream,
  +#if DEBUG_GET_HANDLER
       dav_fs_set_headers,
  -#if DEBUG_PATHNAME_STYLE
  -    dav_fs_get_pathname,
  +    dav_fs_deliver,
   #else
  -    0,
  +    NULL,
  +    NULL,
   #endif
  -    dav_fs_free_file,
       dav_fs_create_collection,
       dav_fs_copy_resource,
       dav_fs_move_resource,
  
  
  
  1.67      +16 -111   httpd-2.0/modules/dav/main/mod_dav.c
  
  Index: mod_dav.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.c,v
  retrieving revision 1.66
  retrieving revision 1.67
  diff -u -r1.66 -r1.67
  --- mod_dav.c	7 Jan 2002 22:36:15 -0000	1.66
  +++ mod_dav.c	14 Jan 2002 13:43:24 -0000	1.67
  @@ -756,7 +756,6 @@
   static int dav_method_get(request_rec *r)
   {
       dav_resource *resource;
  -    int result;
       dav_error *err;
   
       /* This method should only be called when the resource is not
  @@ -772,122 +771,28 @@
           return HTTP_NOT_FOUND;
       }
   
  -    /* Check resource type */
  -    if (resource->type != DAV_RESOURCE_TYPE_REGULAR &&
  -        resource->type != DAV_RESOURCE_TYPE_VERSION &&
  -        resource->type != DAV_RESOURCE_TYPE_WORKING)
  -    {
  -        return dav_error_response(r, HTTP_CONFLICT,
  -                                  "Cannot GET this type of resource.");
  -    }
  -
  -    /* Cannot handle GET of a collection from a repository */
  -    if (resource->collection) {
  -	return dav_error_response(r, HTTP_CONFLICT, 
  -                                  "No default response to GET for a "
  -                                  "collection.");
  +    /* set up the HTTP headers for the response */
  +    if ((err = (*resource->hooks->set_headers)(r, resource)) != NULL) {
  +        err = dav_push_error(r->pool, err->status, 0,
  +                             "Unable to set up HTTP headers.",
  +                             err);
  +        return dav_handle_err(r, err, NULL);
       }
   
  -    /*
  -    ** We can use two different approaches for a GET.
  -    **
  -    ** 1) get_pathname will return a pathname to a file which should be
  -    **    sent to the client. If the repository provides this, then we
  -    **    use it.
  -    **
  -    **    This is the best alternative since it allows us to do a sub-
  -    **    request on the file, which gives the Apache framework a chance
  -    **    to deal with negotiation, MIME types, or whatever.
  -    **
  -    ** 2) open_stream and read_stream.
  -    */
  -    if (resource->hooks->get_pathname != NULL) {
  -	const char *pathname;
  -	void *fhandle;
  -	request_rec *new_req;
  -	
  -	/* Ask repository for copy of file */
  -	pathname = (*resource->hooks->get_pathname)(resource, &fhandle);
  -	if (pathname == NULL) {
  -	    return HTTP_NOT_FOUND;
  -	}
  -
  -	/* Create a sub-request with the new filename 
  -         * The new_req filename is canonicalized by ap_sub_req_lookup_file()
  -         */
  -	new_req = ap_sub_req_lookup_file(pathname, r, NULL);
  -	if (new_req == NULL) {
  -	    (*resource->hooks->free_file)(fhandle);
  -	    return HTTP_INTERNAL_SERVER_ERROR;
  -	}
  -
  -	/* This may be a HEAD request */
  -	new_req->header_only = r->header_only;
  -
  -	/* ### this enables header generation */
  -	new_req->assbackwards = 0;
  -
  -	/* Run the sub-request */
  -	result = ap_run_sub_req(new_req);
  -	ap_destroy_sub_req(new_req);
  -
  -	/* Free resources */
  -	(*resource->hooks->free_file)(fhandle);
  -
  -	return result;
  +    if (r->header_only) {
  +        return DONE;
       }
  -    else {
  -	dav_stream *stream;
  -	void *buffer;
  -
  -	/* set up the HTTP headers for the response */
  -	if ((err = (*resource->hooks->set_headers)(r, resource)) != NULL) {
  -	    err = dav_push_error(r->pool, err->status, 0,
  -				 "Unable to set up HTTP headers.",
  -				 err);
  -	    return dav_handle_err(r, err, NULL);
  -	}
  -
  -        if (r->header_only) {
  -            return DONE;
  -        }
  -
  -	if ((err = (*resource->hooks->open_stream)(resource, DAV_MODE_READ,
  -                                                   &stream)) != NULL) {
  -	    /* ### assuming FORBIDDEN is probably not quite right... */
  -	    err = dav_push_error(r->pool, HTTP_FORBIDDEN, 0,
  -				 apr_psprintf(r->pool,
  -					     "Unable to GET contents for %s.",
  -					     ap_escape_html(r->pool, r->uri)),
  -				 err);
  -	    return dav_handle_err(r, err, NULL);
  -	}
   
  -	buffer = apr_palloc(r->pool, DAV_READ_BLOCKSIZE);
  -	while (1) {
  -	    apr_size_t amt = DAV_READ_BLOCKSIZE;
  -
  -	    if ((err = (*resource->hooks->read_stream)(stream, buffer,
  -                                                       &amt)) != NULL) {
  -		break;
  -	    }
  -	    if (amt == 0) {
  -		/* no more content */
  -		break;
  -	    }
  -	    if (ap_rwrite(buffer, amt, r) < 0) {
  -		/* ### what to do with this error? */
  -		break;
  -	    }
  -	}
  -
  -	if (err != NULL)
  -	    return dav_handle_err(r, err, NULL);
  -
  -	return DONE;
  +    /* okay... time to deliver the content */
  +    if ((err = (*resource->hooks->deliver)(resource,
  +                                           r->output_filters)) != NULL) {
  +        err = dav_push_error(r->pool, err->status, 0,
  +                             "Unable to deliver content.",
  +                             err);
  +        return dav_handle_err(r, err, NULL);
       }
   
  -    /* NOTREACHED */
  +    return DONE;
   }
   
   /* validate resource on POST, then pass it off to the default handler */
  
  
  
  1.54      +23 -35    httpd-2.0/modules/dav/main/mod_dav.h
  
  Index: mod_dav.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.h,v
  retrieving revision 1.53
  retrieving revision 1.54
  diff -u -r1.53 -r1.54
  --- mod_dav.h	18 Sep 2001 04:09:19 -0000	1.53
  +++ mod_dav.h	14 Jan 2002 13:43:24 -0000	1.54
  @@ -1672,13 +1672,15 @@
   **
   ** Note that the structure is opaque -- it is private to the repository
   ** that created the stream in the repository's "open" function.
  +**
  +** ### THIS STUFF IS GOING AWAY ... GET/read requests are handled by
  +** ### having the provider jam stuff straight into the filter stack.
  +** ### this is only left for handling PUT/write requests.
   */
   
   typedef struct dav_stream dav_stream;
   
   typedef enum {
  -    DAV_MODE_READ,		/* open for reading */
  -    DAV_MODE_READ_SEEKABLE,	/* open for random access reading */
       DAV_MODE_WRITE_TRUNC,	/* truncate and open for writing */
       DAV_MODE_WRITE_SEEKABLE	/* open for writing; random access */
   } dav_stream_mode;
  @@ -1789,19 +1791,6 @@
       dav_error * (*close_stream)(dav_stream *stream, int commit);
   
       /*
  -    ** Read data from the stream.
  -    **
  -    ** The size of the buffer is passed in *bufsize, and the amount read
  -    ** is returned in *bufsize.
  -    **
  -    ** *bufsize should be set to zero when the end of file is reached.
  -    ** As a corollary, this function should always read at least one byte
  -    ** on each call, until the EOF condition is met.
  -    */
  -    dav_error * (*read_stream)(dav_stream *stream,
  -			       void *buf, apr_size_t *bufsize);
  -
  -    /*
       ** Write data to the stream.
       **
       ** All of the bytes must be written, or an error should be returned.
  @@ -1824,30 +1813,29 @@
       ** is used to provide the repository with a way to set the headers
       ** in the response.
       **
  -    ** It may be NULL if get_pathname is provided.
  +    ** This function may be called without a following deliver(), to
  +    ** handle a HEAD request.
  +    **
  +    ** This may be NULL if handle_get is FALSE.
       */
       dav_error * (*set_headers)(request_rec *r,
   			       const dav_resource *resource);
   
  -    /* Get a pathname for the file represented by the resource descriptor.
  -     * A provider may need to create a temporary copy of the file, if it is
  -     * not directly accessible in a filesystem. free_handle_p will be set by
  -     * the provider to point to information needed to clean up any temporary
  -     * storage used.
  -     *
  -     * Returns NULL if the file could not be made accessible.
  -     */
  -    const char * (*get_pathname)(
  -        const dav_resource *resource,
  -        void **free_handle_p
  -    );
  -
  -    /* Free any temporary storage associated with a file made accessible by
  -     * get_pathname().
  -     */
  -    void (*free_file)(
  -        void *free_handle
  -    );
  +    /*
  +    ** The provider should deliver the resource into the specified filter.
  +    ** Basically, this is the response to the GET method.
  +    **
  +    ** Note that this is called for all resources, including collections.
  +    ** The provider should determine what has content to deliver or not.
  +    **
  +    ** set_headers will be called prior to this function, allowing the
  +    ** provider to set the appropriate response headers.
  +    **
  +    ** This may be NULL if handle_get is FALSE.
  +    ** ### maybe toss handle_get and just use this function as the marker
  +    */
  +    dav_error * (*deliver)(const dav_resource *resource,
  +                           ap_filter_t *output);
   
       /* Create a collection resource. The resource must not already exist.
        *