Replace use of apr_file_write() with apr_file_write_full() to prevent

incomplete writes.

Add comments in some places where error handling/logging is missing.

PR: 53131.
Submitted by: Nicolas Viennot <apache viennot biz>, Stefan Fritsch

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1331110 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 97de2c6989534da9549ba388948124cf7d80a4e2..a052bf179a53b5e29ebd87cb32a34435cb96b7d0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) various modules, rotatelogs: Replace use of apr_file_write() with
+     apr_file_write_full() to prevent incomplete writes. PR 53131.
+     [Nicolas Viennot <apache viennot biz>, Stefan Fritsch]
+
   *) core: Fix segfault in logging if r->useragent_addr or c->client_addr is
      unset. [Stefan Fritsch]
 

diff --git a/modules/cache/mod_cache_disk.c b/modules/cache/mod_cache_disk.c
index 0c65c1720ee25f31f2e2e442c43a8082729f4304..55f628fa2cb13baa885d24cc503b891cf00e01ab 100644 (file)
--- a/modules/cache/mod_cache_disk.c
+++ b/modules/cache/mod_cache_disk.c
@@ -739,8 +739,7 @@ static apr_status_t store_array(apr_file_t *fd, apr_array_header_t* arr)
         iov[1].iov_base = CRLF;
         iov[1].iov_len = sizeof(CRLF) - 1;
 
-        rv = apr_file_writev(fd, (const struct iovec *) &iov, 2,
-                             &amt);
+        rv = apr_file_writev_full(fd, (const struct iovec *) &iov, 2, &amt);
         if (rv != APR_SUCCESS) {
             return rv;
         }
@@ -749,8 +748,7 @@ static apr_status_t store_array(apr_file_t *fd, apr_array_header_t* arr)
     iov[0].iov_base = CRLF;
     iov[0].iov_len = sizeof(CRLF) - 1;
 
-    return apr_file_writev(fd, (const struct iovec *) &iov, 1,
-                         &amt);
+    return apr_file_writev_full(fd, (const struct iovec *) &iov, 1, &amt);
 }
 
 static apr_status_t read_table(cache_handle_t *handle, request_rec *r,
@@ -898,8 +896,7 @@ static apr_status_t store_table(apr_file_t *fd, apr_table_t *table)
             iov[3].iov_base = CRLF;
             iov[3].iov_len = sizeof(CRLF) - 1;
 
-            rv = apr_file_writev(fd, (const struct iovec *) &iov, 4,
-                                 &amt);
+            rv = apr_file_writev_full(fd, (const struct iovec *) &iov, 4, &amt);
             if (rv != APR_SUCCESS) {
                 return rv;
             }
@@ -907,8 +904,7 @@ static apr_status_t store_table(apr_file_t *fd, apr_table_t *table)
     }
     iov[0].iov_base = CRLF;
     iov[0].iov_len = sizeof(CRLF) - 1;
-    rv = apr_file_writev(fd, (const struct iovec *) &iov, 1,
-                         &amt);
+    rv = apr_file_writev_full(fd, (const struct iovec *) &iov, 1, &amt);
     return rv;
 }
 
@@ -974,7 +970,7 @@ static apr_status_t write_headers(cache_handle_t *h, request_rec *r)
             }
 
             amt = sizeof(format);
-            rv = apr_file_write(dobj->vary.tempfd, &format, &amt);
+            rv = apr_file_write_full(dobj->vary.tempfd, &format, amt, NULL);
             if (rv != APR_SUCCESS) {
                 ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r, APLOGNO(00722)
                         "could not write to vary file %s",
@@ -985,8 +981,8 @@ static apr_status_t write_headers(cache_handle_t *h, request_rec *r)
             }
 
             amt = sizeof(h->cache_obj->info.expire);
-            rv = apr_file_write(dobj->vary.tempfd, &h->cache_obj->info.expire,
-                    &amt);
+            rv = apr_file_write_full(dobj->vary.tempfd,
+                                     &h->cache_obj->info.expire, amt, NULL);
             if (rv != APR_SUCCESS) {
                 ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r, APLOGNO(00723)
                         "could not write to vary file %s",
@@ -1051,7 +1047,8 @@ static apr_status_t write_headers(cache_handle_t *h, request_rec *r)
     iov[1].iov_base = (void*)dobj->name;
     iov[1].iov_len = disk_info.name_len;
 
-    rv = apr_file_writev(dobj->hdrs.tempfd, (const struct iovec *) &iov, 2, &amt);
+    rv = apr_file_writev_full(dobj->hdrs.tempfd, (const struct iovec *) &iov,
+                              2, &amt);
     if (rv != APR_SUCCESS) {
         ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r, APLOGNO(00726)
                 "could not write info to header file %s",

diff --git a/modules/dav/fs/lock.c b/modules/dav/fs/lock.c
index 331d4e83143c297533df28e08652d2a4cdf20b19..a15b4b91c7a18d789f03d8fd123902190af52d04 100644 (file)
--- a/modules/dav/fs/lock.c
+++ b/modules/dav/fs/lock.c
@@ -845,7 +845,7 @@ static dav_error * dav_fs_save_locknull_list(apr_pool_t *p, const char *dirpath,
     }
 
     amt = pbuf->cur_len;
-    if ((rv = apr_file_write(file, pbuf->buf, &amt)) != APR_SUCCESS
+    if ((rv = apr_file_write_full(file, pbuf->buf, amt, &amt)) != APR_SUCCESS
         || amt != pbuf->cur_len) {
         err = dav_new_error(p, HTTP_INTERNAL_SERVER_ERROR, 0, rv,
                             apr_psprintf(p,

diff --git a/modules/filters/mod_ext_filter.c b/modules/filters/mod_ext_filter.c
index 76ddbc8a0330604b02d920badbdafda2e24ab283..bbe93269f898574768a845af92d556da871568dc 100644 (file)
--- a/modules/filters/mod_ext_filter.c
+++ b/modules/filters/mod_ext_filter.c
@@ -681,9 +681,9 @@ static apr_status_t pass_data_to_filter(ap_filter_t *f, const char *data,
 
     do {
         tmplen = len - bytes_written;
-        rv = apr_file_write(ctx->proc->in,
+        rv = apr_file_write_full(ctx->proc->in,
                        (const char *)data + bytes_written,
-                       &tmplen);
+                       tmplen, &tmplen);
         bytes_written += tmplen;
         if (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv)) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, f->r, APLOGNO(01461)

diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index 7a6540168c9d4824eec5522e06de4689bfe3d93c..5cf7cf5895b6ac3e6a06162386ef56f094d44593 100644 (file)
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -1155,7 +1155,7 @@ static int log_script(request_rec *r, cgid_server_conf * conf, int ret,
             apr_file_puts("%stdout\n", f);
             first = 0;
         }
-        apr_file_write(f, buf, &len);
+        apr_file_write_full(f, buf, len, NULL);
         apr_file_puts("\n", f);
     }
 

diff --git a/modules/loggers/mod_log_config.c b/modules/loggers/mod_log_config.c
index bc225cee769445d06bbfacc29f43f16a4ef774b2..1b3947238c90187c47f63272253a80e635165b7e 100644 (file)
--- a/modules/loggers/mod_log_config.c
+++ b/modules/loggers/mod_log_config.c
@@ -1048,7 +1048,8 @@ static const char *process_item(request_rec *r, request_rec *orig,
 static void flush_log(buffered_log *buf)
 {
     if (buf->outcnt && buf->handle != NULL) {
-        apr_file_write(buf->handle, buf->outbuf, &buf->outcnt);
+        /* XXX: error handling */
+        apr_file_write_full(buf->handle, buf->outbuf, buf->outcnt, NULL);
         buf->outcnt = 0;
     }
 }
@@ -1539,7 +1540,7 @@ static apr_status_t ap_default_log_writer( request_rec *r,
         s += strl[i];
     }
 
-    rv = apr_file_write((apr_file_t*)handle, str, &len);
+    rv = apr_file_write_full((apr_file_t*)handle, str, len, NULL);
 
     return rv;
 }
@@ -1618,7 +1619,7 @@ static apr_status_t ap_buffered_log_writer(request_rec *r,
             s += strl[i];
         }
         w = len;
-        rv = apr_file_write(buf->handle, str, &w);
+        rv = apr_file_write_full(buf->handle, str, w, NULL);
 
     }
     else {

diff --git a/modules/loggers/mod_log_forensic.c b/modules/loggers/mod_log_forensic.c
index 7a333dcfd2f75266c01edf8071ae211a5bce2d84..add3eb94ac64ee5682eecc843512aa05f0c8ea84 100644 (file)
--- a/modules/loggers/mod_log_forensic.c
+++ b/modules/loggers/mod_log_forensic.c
@@ -222,7 +222,7 @@ static int log_before(request_rec *r)
     *h.pos++ = '\n';
 
     n = h.count-1;
-    rv = apr_file_write(cfg->fd, h.log, &n);
+    rv = apr_file_write_full(cfg->fd, h.log, n, &n);
     ap_assert(rv == APR_SUCCESS && n == h.count-1);
 
     apr_table_setn(r->notes, "forensic-id", id);
@@ -246,7 +246,7 @@ static int log_after(request_rec *r)
 
     s = apr_pstrcat(r->pool, "-", id, "\n", NULL);
     l = n = strlen(s);
-    rv = apr_file_write(cfg->fd, s, &n);
+    rv = apr_file_write_full(cfg->fd, s, n, &n);
     ap_assert(rv == APR_SUCCESS && n == l);
 
     return OK;

diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c
index 1d6951343e69d714503e9f91a1e03fb947bfe1c2..314b48a1e51b9b36aff0ed83a484ee4fd13747c3 100644 (file)
--- a/modules/mappers/mod_rewrite.c
+++ b/modules/mappers/mod_rewrite.c
@@ -1401,9 +1401,10 @@ static char *lookup_map_program(request_rec *r, apr_file_t *fpin,
     /* write out the request key */
 #ifdef NO_WRITEV
     nbytes = strlen(key);
-    apr_file_write(fpin, key, &nbytes);
+    /* XXX: error handling */
+    apr_file_write_full(fpin, key, nbytes, NULL);
     nbytes = 1;
-    apr_file_write(fpin, "\n", &nbytes);
+    apr_file_write_full(fpin, "\n", nbytes, NULL);
 #else
     iova[0].iov_base = key;
     iova[0].iov_len = strlen(key);
@@ -1411,7 +1412,8 @@ static char *lookup_map_program(request_rec *r, apr_file_t *fpin,
     iova[1].iov_len = 1;
 
     niov = 2;
-    apr_file_writev(fpin, iova, niov, &nbytes);
+    /* XXX: error handling */
+    apr_file_writev_full(fpin, iova, niov, &nbytes);
 #endif
 
     buf = apr_palloc(r->pool, REWRITE_PRG_MAP_BUF + 1);

diff --git a/modules/slotmem/mod_slotmem_shm.c b/modules/slotmem/mod_slotmem_shm.c
index a9b7ca258cc0766b18837728f7585388d7035f28..fa4dc6795512a929fcd1f5bd800334a24365e0f8 100644 (file)
--- a/modules/slotmem/mod_slotmem_shm.c
+++ b/modules/slotmem/mod_slotmem_shm.c
@@ -166,7 +166,8 @@ static void store_slotmem(ap_slotmem_instance_t *slotmem)
         }
         nbytes = (slotmem->desc.size * slotmem->desc.num) +
                  (slotmem->desc.num * sizeof(char)) + AP_UNSIGNEDINT_OFFSET;
-        apr_file_write(fp, slotmem->persist, &nbytes);
+       /* XXX: Error handling */
+        apr_file_write_full(fp, slotmem->persist, &nbytes, NULL);
         apr_file_close(fp);
     }
 }

diff --git a/support/rotatelogs.c b/support/rotatelogs.c
index 81bd950fc2e28cab2c1622b747d0f0ce1e285318..5ed8befaa7c2679953f1b5c714f09b382b8cdd1c 100644 (file)
--- a/support/rotatelogs.c
+++ b/support/rotatelogs.c
@@ -459,7 +459,7 @@ static void doRotate(rotate_config_t *config, rotate_status_t *status)
             fprintf(stderr, "Error truncating the file %s\n", status->current.name);
             exit(2);
         }
-        if (apr_file_write(status->current.fd, status->errbuf, &nWrite) != APR_SUCCESS) {
+        if (apr_file_write_full(status->current.fd, status->errbuf, nWrite, NULL) != APR_SUCCESS) {
             fprintf(stderr, "Error writing to the file %s\n", status->current.name);
             exit(2);
         }
@@ -687,18 +687,7 @@ int main (int argc, const char * const argv[])
         }
 
         nWrite = nRead;
-        rv = apr_file_write(status.current.fd, buf, &nWrite);
-        if (rv == APR_SUCCESS && nWrite != nRead) {
-            /* buffer partially written, which for rotatelogs means we encountered
-             * an error such as out of space or quota or some other limit reached;
-             * try to write the rest so we get the real error code
-             */
-            apr_size_t nWritten = nWrite;
-
-            nRead  = nRead - nWritten;
-            nWrite = nRead;
-            rv = apr_file_write(status.current.fd, buf + nWritten, &nWrite);
-        }
+        rv = apr_file_write_full(status.current.fd, buf, nWrite, &nWrite);
         if (nWrite != nRead) {
             char strerrbuf[120];
             apr_off_t cur_offset;
@@ -715,7 +704,7 @@ int main (int argc, const char * const argv[])
                          rv, cur_offset, status.nMessCount, strerrbuf);
             nWrite = strlen(status.errbuf);
             apr_file_trunc(status.current.fd, 0);
-            if (apr_file_write(status.current.fd, status.errbuf, &nWrite) != APR_SUCCESS) {
+            if (apr_file_write_full(status.current.fd, status.errbuf, nWrite, NULL) != APR_SUCCESS) {
                 fprintf(stderr, "Error writing to the file %s\n", status.current.name);
                 exit(2);
             }