Merge r1788929 from trunk:

Fix two issues with apr_file_trunc() for buffered files:

 - The Win32 implementation incorrectly flushes the buffered writes _after_
   truncating a file.  Such files will have unexpected data written after
   the position at which they should've been truncated. PR 51017.
   (Under Unix, this issue has been fixed in r1044440)

 - Both Win32 and Unix implementations incorrectly keep the data read into
   a buffer after the file is truncated.  Thus, reading from a file after
   apr_file_trunc() can return invalid data from the previous file offset.

* file_io/win32/seek.c
  (apr_file_trunc): Flush the write buffer or discard the read buffer
   before truncating.  Propely update the internal file offset (filePtr)
   and the eof_hit marker.

* file_io/unix/seek.c
  (apr_file_trunc): Discard the read buffer before truncating.

* test/testfile.c
  (test_file_trunc): Extend the checks.  Factor out part of this test...
  (test_file_trunc_buffered_write): ...into this new test.
  (test_file_trunc_buffered_write2, test_file_trunc_buffered_read): New tests.
  (testfile): Run the new tests.

Patch by: Evgeny Kotkov <evgeny.kotkov {at} visualsvn.com>

Submitted by: ivan
Reviewed by: jim


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

4 files changed, 293 insertions, 4 deletions

diff --git a/CHANGES b/CHANGES
index ef2c854c9..91d050e88 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,13 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 1.6.3
 
+  *) apr_file_trunc: Truncating a buffered file could add unexpected
+     data after the truncate position. PR 51017.
+     [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
 
+  *) apr_file_trunc: Fix an issue where reading from a buffered file
+     after truncate could return stale data from the buffer.
+     [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
 
 Changes for APR 1.6.2
 
diff --git a/file_io/unix/seek.c b/file_io/unix/seek.c
index 3f5aa00e9..2e973377b 100644
--- a/file_io/unix/seek.c
+++ b/file_io/unix/seek.c
@@ -117,6 +117,13 @@ apr_status_t apr_file_trunc(apr_file_t *fp, apr_off_t offset)
             /* Reset buffer positions for write mode */
             fp->bufpos = fp->direction = fp->dataRead = 0;
         }
+        else if (fp->direction == 0) {
+            /* Discard the read buffer, as we are about to reposition
+             * ourselves to the end of file.
+             */
+            fp->bufpos = 0;
+            fp->dataRead = 0;
+        }
         file_unlock(fp);
         if (rc) {
             return rc;
diff --git a/file_io/win32/seek.c b/file_io/win32/seek.c
index b412fd4cb..afe6edb00 100644
--- a/file_io/win32/seek.c
+++ b/file_io/win32/seek.c
@@ -161,17 +161,43 @@ APR_DECLARE(apr_status_t) apr_file_trunc(apr_file_t *thefile, apr_off_t offset)
     LONG  offhi = (LONG)(offset >> 32);
     DWORD rc;
 
+    if (thefile->buffered) {
+        if (thefile->direction == 1) {
+            /* Figure out what needs to be flushed.  Don't flush the part
+             * of the write buffer that will get truncated anyway.
+             */
+            if (offset < thefile->filePtr) {
+                thefile->bufpos = 0;
+            }
+            else if (offset < thefile->filePtr + (apr_off_t)thefile->bufpos) {
+                thefile->bufpos = offset - thefile->filePtr;
+            }
+
+            if (thefile->bufpos != 0) {
+                rv = apr_file_flush(thefile);
+                if (rv != APR_SUCCESS)
+                    return rv;
+            }
+        }
+        else if (thefile->direction == 0) {
+            /* Discard the read buffer, as we are about to reposition
+             * ourselves to the end of file.
+             */
+            thefile->bufpos = 0;
+            thefile->dataRead = 0;
+        }
+    }
+
     rc = SetFilePointer(thefile->filehand, offlo, &offhi, FILE_BEGIN);
     if (rc == 0xFFFFFFFF)
         if ((rv = apr_get_os_error()) != APR_SUCCESS)
             return rv;
+    thefile->filePtr = offset;
+    /* Don't report EOF until the next read. */
+    thefile->eof_hit = 0;
 
     if (!SetEndOfFile(thefile->filehand))
         return apr_get_os_error();
 
-    if (thefile->buffered) {
-        return setptr(thefile, offset);
-    }
-
     return APR_SUCCESS;
 }
diff --git a/test/testfile.c b/test/testfile.c
index 7b5800661..c59b3ae8b 100644
--- a/test/testfile.c
+++ b/test/testfile.c
@@ -811,6 +811,253 @@ static void test_truncate(abts_case *tc, void *data)
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 }
 
+static void test_file_trunc(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtruncate.dat";
+    const char *s;
+    apr_size_t nbytes;
+    apr_finfo_t finfo;
+
+    apr_file_remove(fname, p);
+
+    /* Test unbuffered */
+    rv = apr_file_open(&f, fname,
+                        APR_FOPEN_CREATE | APR_FOPEN_READ |
+                        APR_FOPEN_WRITE,
+                        APR_FPROT_UREAD | APR_FPROT_UWRITE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    s = "some data";
+    nbytes = strlen(s);
+    rv = apr_file_write(f, s, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_SIZE_EQUAL(tc, strlen(s), nbytes);
+    rv = apr_file_trunc(f, 4);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_close(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_ASSERT(tc, "File size mismatch, expected 4", finfo.size == 4);
+
+    rv = apr_file_remove(fname, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    /* Test buffered */
+    rv = apr_file_open(&f, fname,
+                        APR_FOPEN_CREATE | APR_FOPEN_READ |
+                        APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                        APR_FPROT_UREAD | APR_FPROT_UWRITE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    nbytes = strlen(s);
+    rv = apr_file_write(f, s, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_SIZE_EQUAL(tc, strlen(s), nbytes);
+    rv = apr_file_trunc(f, 4);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_close(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_ASSERT(tc, "File size mismatch, expected 4", finfo.size == 4);
+
+    rv = apr_file_remove(fname, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+}
+
+static void test_file_trunc(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtruncate.dat";
+    const char *s;
+    apr_size_t nbytes;
+    apr_finfo_t finfo;
+    char c;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                        APR_FOPEN_CREATE | APR_FOPEN_READ |
+                        APR_FOPEN_WRITE,
+                        APR_FPROT_UREAD | APR_FPROT_UWRITE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    s = "some data";
+    nbytes = strlen(s);
+    rv = apr_file_write(f, s, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_SIZE_EQUAL(tc, strlen(s), nbytes);
+    rv = apr_file_trunc(f, 4);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    /* Test apr_file_info_get(). */
+    rv = apr_file_info_get(&finfo, APR_FINFO_SIZE, f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_INT_EQUAL(tc, 4, (int)finfo.size);
+    /* EOF is not reported until the next read. */
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_getc(&c, f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+
+    rv = apr_file_close(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_INT_EQUAL(tc, 4, (int)finfo.size);
+
+    rv = apr_file_remove(fname, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+}
+
+static void test_file_trunc_buffered_write(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtruncate_buffered_write.dat";
+    const char *s;
+    apr_size_t nbytes;
+    apr_finfo_t finfo;
+    char c;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                        APR_FOPEN_CREATE | APR_FOPEN_READ |
+                        APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                        APR_FPROT_UREAD | APR_FPROT_UWRITE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+    s = "some data";
+    nbytes = strlen(s);
+    rv = apr_file_write(f, s, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_SIZE_EQUAL(tc, strlen(s), nbytes);
+    rv = apr_file_trunc(f, 4);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    /* Test apr_file_info_get(). */
+    rv = apr_file_info_get(&finfo, APR_FINFO_SIZE, f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_INT_EQUAL(tc, 4, (int)finfo.size);
+    /* EOF is not reported until the next read. */
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_getc(&c, f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+
+    rv = apr_file_close(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_INT_EQUAL(tc, 4, (int)finfo.size);
+
+    rv = apr_file_remove(fname, p);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+}
+
+static void test_file_trunc_buffered_write2(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtruncate_buffered_write2.dat";
+    apr_finfo_t finfo;
+    char c;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_READ |
+                       APR_FOPEN_WRITE | APR_FOPEN_BUFFERED,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file", rv);
+
+    rv = apr_file_puts("abc", f);
+    APR_ASSERT_SUCCESS(tc, "write first string", rv);
+    rv = apr_file_flush(f);
+    APR_ASSERT_SUCCESS(tc, "flush", rv);
+    rv = apr_file_puts("def", f);
+    APR_ASSERT_SUCCESS(tc, "write second string", rv);
+    /* Truncate behind the write buffer. */
+    rv = apr_file_trunc(f, 2);
+    APR_ASSERT_SUCCESS(tc, "truncate the file", rv);
+    /* Test apr_file_info_get(). */
+    rv = apr_file_info_get(&finfo, APR_FINFO_SIZE, f);
+    APR_ASSERT_SUCCESS(tc, "get file info", rv);
+    ABTS_INT_EQUAL(tc, 2, (int)finfo.size);
+    /* EOF is not reported until the next read. */
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_getc(&c, f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+
+    apr_file_close(f);
+
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    APR_ASSERT_SUCCESS(tc, "stat file", rv);
+    ABTS_INT_EQUAL(tc, 2, (int)finfo.size);
+
+    apr_file_remove(fname, p);
+}
+
+static void test_file_trunc_buffered_read(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testtruncate_buffered_read.dat";
+    apr_finfo_t finfo;
+    char c;
+
+    apr_file_remove(fname, p);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_CREATE | APR_FOPEN_READ |
+                       APR_FOPEN_WRITE, APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open test file", rv);
+
+    rv = apr_file_puts("abc", f);
+    APR_ASSERT_SUCCESS(tc, "write test data", rv);
+    apr_file_close(f);
+
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_READ | APR_FOPEN_WRITE |
+                       APR_FOPEN_BUFFERED, APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "re-open test file", rv);
+
+    /* Read to fill in the buffer. */
+    rv = apr_file_getc(&c, f);
+    APR_ASSERT_SUCCESS(tc, "read char", rv);
+    /* Truncate the file. */
+    rv = apr_file_trunc(f, 1);
+    APR_ASSERT_SUCCESS(tc, "truncate the file", rv);
+    /* Test apr_file_info_get(). */
+    rv = apr_file_info_get(&finfo, APR_FINFO_SIZE, f);
+    APR_ASSERT_SUCCESS(tc, "get file info", rv);
+    ABTS_INT_EQUAL(tc, 1, (int)finfo.size);
+    /* EOF is not reported until the next read. */
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    rv = apr_file_getc(&c, f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+    rv = apr_file_eof(f);
+    ABTS_INT_EQUAL(tc, APR_EOF, rv);
+
+    apr_file_close(f);
+
+    rv = apr_stat(&finfo, fname, APR_FINFO_SIZE, p);
+    APR_ASSERT_SUCCESS(tc, "stat file", rv);
+    ABTS_INT_EQUAL(tc, 1, (int)finfo.size);
+
+    apr_file_remove(fname, p);
+}
+
 static void test_bigfprintf(abts_case *tc, void *data)
 {
     apr_file_t *f;
@@ -1006,6 +1253,9 @@ abts_suite *testfile(abts_suite *suite)
     abts_run_test(suite, test_bigread, NULL);
     abts_run_test(suite, test_mod_neg, NULL);
     abts_run_test(suite, test_truncate, NULL);
+    abts_run_test(suite, test_file_trunc_buffered_write, NULL);
+    abts_run_test(suite, test_file_trunc_buffered_write2, NULL);
+    abts_run_test(suite, test_file_trunc_buffered_read, NULL);
     abts_run_test(suite, test_bigfprintf, NULL);
     abts_run_test(suite, test_fail_write_flush, NULL);
     abts_run_test(suite, test_fail_read_flush, NULL);