httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian J. France" <br...@brianfrance.com>
Subject mod_dav_fs does not check for return value on stream_close
Date Thu, 15 Mar 2012 13:56:40 GMT
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);
 
+    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);
 
+    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);



Mime
View raw message