Merge r234013, r239221 from trunk:

* file_io/unix/readwrite.c (apr_file_write): Catch apr_file_flush()
failure for buffered files.
(apr_file_read): Handle the apr_file_flush() return value when
flushing buffered writes.

* test/testfile.c (test_fail_write_flush, test_fail_read_flush): Add
test cases.

Submitted by: Erik Huelsmann <ehuels gmail.com>, jorton


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/0.9.x@267192 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 86 insertions, 2 deletions

diff --git a/CHANGES b/CHANGES
index 77a5a7ce0..de68e09c5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,11 @@
 Changes with APR 0.9.7
 
+  *) Fix apr_file_read() to catch write failures when flushing pending
+     writes for a buffered file.  [Joe Orton]
+
+  *) Fix apr_file_write() infinite loop on write failure for buffered
+     files.  [Erik Huelsmann <ehuels gmail.com>]
+
   *) Fix error handling where apr_uid_* and apr_gid_* could segfault
      or return APR_SUCCESS in failure cases.  PR 34053.  [Joe Orton,
      Paul Querna]
diff --git a/file_io/unix/readwrite.c b/file_io/unix/readwrite.c
index 5e163ec7a..2b9deacf9 100644
--- a/file_io/unix/readwrite.c
+++ b/file_io/unix/readwrite.c
@@ -50,7 +50,15 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t *thefile, void *buf, apr_size
 #endif
 
         if (thefile->direction == 1) {
-            apr_file_flush(thefile);
+            rv = apr_file_flush(thefile);
+            if (rv) {
+#if APR_HAS_THREADS
+                if (thefile->thlock) {
+                    apr_thread_mutex_unlock(thefile->thlock);
+                }
+#endif
+                return rv;
+            }
             thefile->bufpos = 0;
             thefile->direction = 0;
             thefile->dataRead = 0;
@@ -173,7 +181,7 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, a
         rv = 0;
         while (rv == 0 && size > 0) {
             if (thefile->bufpos == APR_FILE_BUFSIZE)   /* write buffer is full*/
-                apr_file_flush(thefile);
+                rv = apr_file_flush(thefile);
 
             blocksize = size > APR_FILE_BUFSIZE - thefile->bufpos ? 
                         APR_FILE_BUFSIZE - thefile->bufpos : size;
diff --git a/test/testfile.c b/test/testfile.c
index 7b097351e..7871fc3d6 100644
--- a/test/testfile.c
+++ b/test/testfile.c
@@ -528,6 +528,74 @@ static void test_truncate(CuTest *tc)
     CuAssertIntEquals(tc, APR_SUCCESS, rv);
 }
 
+static void test_fail_write_flush(CuTest *tc)
+{
+    apr_file_t *f;
+    const char *fname = "data/testflush.dat";
+    apr_status_t rv;
+    char buf[APR_BUFFERSIZE];
+    int n;
+
+    apr_file_remove(fname, p);
+
+    apr_assert_success(tc, "open test file",
+                       apr_file_open(&f, fname,
+                                     APR_CREATE|APR_READ|APR_BUFFERED,
+                                     APR_UREAD|APR_UWRITE, p));
+
+    memset(buf, 'A', sizeof buf);
+
+    /* Try three writes.  One of these should fail when it exceeds the
+     * internal buffer and actually tries to write to the file, which
+     * was opened read-only and hence should be unwritable. */
+    for (n = 0, rv = APR_SUCCESS; n < 4 && rv == APR_SUCCESS; n++) {
+        apr_size_t bytes = sizeof buf;
+        rv = apr_file_write(f, buf, &bytes);
+    }
+
+    CuAssert(tc, "failed to write to read-only buffered fd",
+             rv != APR_SUCCESS);
+
+    apr_file_close(f);
+    apr_file_remove(fname, p);
+}
+
+static void test_fail_read_flush(CuTest *tc)
+{
+    apr_file_t *f;
+    const char *fname = "data/testflush.dat";
+    apr_status_t rv;
+    char buf[2];
+
+    apr_file_remove(fname, p);
+
+    apr_assert_success(tc, "open test file",
+                       apr_file_open(&f, fname,
+                                     APR_CREATE|APR_READ|APR_BUFFERED,
+                                     APR_UREAD|APR_UWRITE, p));
+
+    /* this write should be buffered. */
+    apr_assert_success(tc, "buffered write should succeed",
+                       apr_file_puts("hello", f));
+
+    /* Now, trying a read should fail since the write must be flushed,
+     * and should fail with something other than EOF since the file is
+     * opened read-only. */
+    rv = apr_file_read_full(f, buf, 2, NULL);
+
+    CuAssert(tc, "read should flush buffered write and fail",
+             rv != APR_SUCCESS && rv != APR_EOF);
+
+    /* Likewise for gets */
+    rv = apr_file_gets(buf, 2, f);
+
+    CuAssert(tc, "gets should flush buffered write and fail",
+             rv != APR_SUCCESS && rv != APR_EOF);
+
+    apr_file_close(f);
+    apr_file_remove(fname, p);
+}
+
 CuSuite *testfile(void)
 {
     CuSuite *suite = CuSuiteNew("File I/O");
@@ -553,6 +621,8 @@ CuSuite *testfile(void)
     SUITE_ADD_TEST(suite, test_bigread);
     SUITE_ADD_TEST(suite, test_mod_neg);
     SUITE_ADD_TEST(suite, test_truncate);
+    SUITE_ADD_TEST(suite, test_fail_write_flush);
+    SUITE_ADD_TEST(suite, test_fail_read_flush);
 
     return suite;
 }