On Sun, Jun 29, 2014 at 10:51 PM, Jeff Trawick <[email protected]> wrote:
> Hi Stefan,
>
> Are you able to update test/testfile.c to include testcases like those you
> provided in your post?
>
Hi Jeff,
It took me a while to get back to it, but here it finally is.
Please find attached the (unchange) patch for trunk()
plus the testfile.c extensions. The updated and corrected
log message is at the beginning of the patch file.
-- Stefan^2.
[[[
Prevent stale buffer data and bogus file pointers to be returned
after apr_file_flush() on Unix.
* file_io/unix/seek.c
(apr_file_trunc): Limit the data buffer to what will remain
after trunc(). If the trunc() itself should
fail, having shortended buffer contents
does not effect the result of any APR call.
* test/testfile.c
(test_truncate): Provide regression tests that demonstate / check
for the issues found on UNIX.
Patch by: stefan2
]]]
Index: file_io/unix/seek.c
===================================================================
--- file_io/unix/seek.c (revision 1618575)
+++ file_io/unix/seek.c (working copy)
@@ -117,6 +117,24 @@ apr_status_t apr_file_trunc(apr_file_t *
/* Reset buffer positions for write mode */
fp->bufpos = fp->direction = fp->dataRead = 0;
}
+
+ /* Limit the buffered data to what will remain of the actual file.
+ * That prevents stale data from being read and wrong file locations
+ * being reported. */
+ if (fp->filePtr >= offset) {
+ /* The entire buffer will lie beyond EOF now. Put it at EOF. */
+ fp->dataRead = fp->bufpos = 0;
+ fp->filePtr = offset;
+ }
+ else if (fp->filePtr + fp->dataRead > offset) {
+ /* Only part of the buffer will be beyond EOF.
+ * Truncate it and adjust the read pointer if necessary. */
+ fp->dataRead = offset - fp->filePtr;
+ if (fp->dataRead < fp->bufpos) {
+ fp->bufpos = fp->dataRead;
+ }
+ }
+
file_unlock(fp);
if (rc) {
return rc;
Index: test/testfile.c
===================================================================
--- test/testfile.c (revision 1618575)
+++ test/testfile.c (working copy)
@@ -777,8 +777,11 @@ static void test_truncate(abts_case *tc,
apr_file_t *f;
const char *fname = "data/testtruncate.dat";
const char *s;
+ apr_off_t offset;
apr_size_t nbytes;
apr_finfo_t finfo;
+ const char teststring = "Hello World";
+ char *buffer = apr_pcalloc(p, sizeof(teststring));
apr_file_remove(fname, p);
@@ -806,6 +809,81 @@ static void test_truncate(abts_case *tc,
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
ABTS_ASSERT(tc, "File size mismatch, expected 0 (empty)", finfo.size == 0);
+ /* Regression test: File read buffer did not get truncated. */
+
+ /* Setup. */
+ rv = apr_file_open(&f, fname,
+ APR_FOPEN_READ | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED |
+ APR_FOPEN_TRUNCATE, APR_UREAD | APR_UWRITE, p);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ nbytes = sizeof(teststring);
+ rv = apr_file_write(f, "Hello World", &nbytes);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ rv = apr_file_flush(f);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ offset = 0;
+ rv = apr_file_seek(f, APR_SET, &offset);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ rv = apr_file_read(f, buffer, &nbytes);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ rv = apr_file_trunc(f, 5);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ /* Test. Attempting to read data should return nothing but it used to
+ return stale data. */
+ nbytes = 6;
+ buffer[0] = 0;
+ rv = apr_file_read(f, buffer, &nbytes);
+ ABTS_ASSERT(tc, "Read stale buffered data beyond EOF",
+ nbytes == 0 && buffer[0] == 0);
+ ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_EOF(rv));
+
+ rv = apr_file_close(f);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ /* Regression test: File read buffer state had been inconsistent. */
+
+ /* Setup. */
+ rv = apr_file_open(&f, fname,
+ APR_FOPEN_READ | APR_FOPEN_WRITE | APR_FOPEN_BUFFERED |
+ APR_FOPEN_TRUNCATE, APR_UREAD | APR_UWRITE, p);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ nbytes = sizeof(teststring);
+ rv = apr_file_write(f, "Hello World", &nbytes);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ rv = apr_file_flush(f);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ offset = 0;
+ rv = apr_file_seek(f, APR_SET, &offset);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ rv = apr_file_read(f, buffer, &nbytes);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ABTS_INT_EQUAL(tc, sizeof(teststring), nbytes);
+ rv = apr_file_trunc(f, 5);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ /* Test. Superficially, seek() seems to work ... */
+ offset = 0;
+ rv = apr_file_seek(f, APR_CUR, &offset);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ABTS_ASSERT(tc, "File pointer is not at EOF", offset == 5);
+
+ /* ... but the internal buffer state is inconsistent making it fail
+ further down the road. */
+ nbytes = 1;
+ rv = apr_file_write(f, "X", &nbytes);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ offset = 0;
+ rv = apr_file_seek(f, APR_CUR, &offset);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ABTS_ASSERT(tc, "Buffered file pointer is off", offset == 6);
+
+ rv = apr_file_close(f);
+ ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+
+ /* Cleanup */
rv = apr_file_remove(fname, p);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
}
