httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: mod_dav_fs does not check for return value on stream_close
Date Wed, 04 Apr 2012 22:24:30 GMT
On 15 Mar 2012, at 3:56 PM, Brian J. France wrote:

> Could somebody review the patch below for 2.2, 2.4, and trunk?
> 
> A better error message could be sent, but I am more worried about how the return will
effect the code after it.
> 
> I am thinking the file needs to be removed either via a apr_file_remove call or:
> 
>  apr_pool_cleanup_kill(stream->p, stream, tmpfile_cleanup);
> 
> call, but I don't know the code well enough to know which is right and 2.4/trunk adds
even more complexity compared to 2.2.x.
> 
> Thoughts/Comments?
> 
> Thanks,
> 
> Brian
> 
> - I am still getting more details why close is failing, but for some reason it is happening
enough to cause issues. (responding 200, but no file)
> 
> -----
> 
> 2.2:
> 
> Index: modules/dav/fs/repos.c
> ===================================================================
> --- modules/dav/fs/repos.c	(revision 1300964)
> +++ modules/dav/fs/repos.c	(working copy)
> @@ -881,6 +881,10 @@
> {
>     apr_file_close(stream->f);

Would be above not be

status = apr_file_close(stream->f);

> 
> +    if (status != APR_SUCCESS) {
> +	return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a problem closing
the stream");
> +    }
> +
>     if (!commit) {
>         if (apr_file_remove(stream->pathname, stream->p) != APR_SUCCESS) {
>             /* ### use a better description? */
> 
> 
> 2.24 and trunk:
> 
> 
> Index: modules/dav/fs/repos.c
> ===================================================================
> --- modules/dav/fs/repos.c	(revision 1300964)
> +++ modules/dav/fs/repos.c	(working copy)
> @@ -970,6 +970,10 @@
> 
>     apr_file_close(stream->f);

Same with this one.

> 
> +    if (status != APR_SUCCESS) {
> +	return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a problem closing
the stream");
> +    }
> +
>     if (!commit) {
>         if (stream->temppath) {
>             apr_pool_cleanup_run(stream->p, stream, tmpfile_cleanup);
> 
> 

Regards,
Graham
--


Mime
View raw message