Added error checking for APR File I/O routines on Unix. This needs to be

ported to the rest of the platforms.
Submitted by:	Jon Travis <jtravis@covalent.net>
Reviewed by:	Ryan Bloom


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@59767 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 160 insertions, 12 deletions

diff --git a/file_io/unix/dir.c b/file_io/unix/dir.c
index 38e778220..ad90506f6 100644
--- a/file_io/unix/dir.c
+++ b/file_io/unix/dir.c
@@ -75,6 +75,11 @@ static ap_status_t dir_cleanup(void *thedir)
  */                        
 ap_status_t ap_opendir(struct dir_t **new, const char *dirname, ap_context_t *cont)
 {
+    if (new == NULL)
+        return APR_EBADARG;
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     (*new) = (struct dir_t *)ap_palloc(cont, sizeof(struct dir_t));
 
     (*new)->cntxt = cont;
@@ -101,6 +106,9 @@ ap_status_t ap_closedir(struct dir_t *thedir)
 {
     ap_status_t rv;
 
+    if (thedir == NULL)
+        return APR_EBADARG;
+
     if ((rv = dir_cleanup(thedir)) == APR_SUCCESS) {
         ap_kill_cleanup(thedir->cntxt, thedir, dir_cleanup);
         return APR_SUCCESS;
@@ -119,6 +127,14 @@ ap_status_t ap_readdir(struct dir_t *thedir)
 #if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) \
     && !defined(READDIR_IS_THREAD_SAFE)
     ap_status_t ret;
+#endif
+
+    if (thedir == NULL)
+        return APR_EBADARG;
+
+#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) \
+    && !defined(READDIR_IS_THREAD_SAFE)
+
     ret = readdir_r(thedir->dirstruct, thedir->entry, &thedir->entry);
     /* Avoid the Linux problem where at end-of-directory thedir->entry
      * is set to NULL, but ret = APR_SUCCESS.
@@ -145,6 +161,9 @@ ap_status_t ap_readdir(struct dir_t *thedir)
  */                        
 ap_status_t ap_rewinddir(struct dir_t *thedir)
 {
+    if (thedir == NULL)
+        return APR_EBADARG;
+
     rewinddir(thedir->dirstruct);
     return APR_SUCCESS;
 }
@@ -160,6 +179,10 @@ ap_status_t ap_rewinddir(struct dir_t *thedir)
 ap_status_t ap_make_dir(const char *path, ap_fileperms_t perm, ap_context_t *cont)
 {
     mode_t mode = get_fileperms(perm);
+
+    if (cont == NULL) 
+        return APR_ENOCONT;
+
     if (mkdir(path, mode) == 0) {
         return APR_SUCCESS;
     }
@@ -176,6 +199,9 @@ ap_status_t ap_make_dir(const char *path, ap_fileperms_t perm, ap_context_t *con
  */                        
 ap_status_t ap_remove_dir(const char *path, ap_context_t *cont)
 {
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     if (rmdir(path) == 0) {
         return APR_SUCCESS;
     }
@@ -195,6 +221,9 @@ ap_status_t ap_dir_entry_size(ap_ssize_t *size, struct dir_t *thedir)
     struct stat filestat;
     char *fname = NULL;    
 
+    if (size == NULL || thedir == NULL)
+        return APR_EBADARG;
+
     if (thedir->entry == NULL) {
         *size = -1;
         return APR_ENOFILE;
@@ -211,16 +240,19 @@ ap_status_t ap_dir_entry_size(ap_ssize_t *size, struct dir_t *thedir)
 }
 
 /* ***APRDOC********************************************************
- * ap_status_t ap_dir_entry_mtime(time_t *mtime, ap_dir_t *thedir)
+ * ap_status_t ap_dir_entry_mtime(ap_time_t *mtime, ap_dir_t *thedir)
  *    Get the last modified time of the current directory entry. 
  * arg 1) the last modified time of the directory entry. 
  * arg 2) the currently open directory.
  */                        
-ap_status_t ap_dir_entry_mtime(time_t *mtime, struct dir_t *thedir)
+ap_status_t ap_dir_entry_mtime(ap_time_t *mtime, struct dir_t *thedir)
 {
     struct stat filestat;
     char *fname = NULL;
 
+    if (mtime == NULL || thedir == NULL)
+        return APR_EBADARG;
+
     if (thedir->entry == NULL) {
         *mtime = -1;
         return APR_ENOFILE;
@@ -233,7 +265,7 @@ ap_status_t ap_dir_entry_mtime(time_t *mtime, struct dir_t *thedir)
         return APR_ENOSTAT;
     }
     
-    *mtime = filestat.st_mtime;
+    ap_ansi_time_to_ap_time(mtime, filestat.st_mtime);
     return APR_SUCCESS;
 }
  
@@ -248,6 +280,9 @@ ap_status_t ap_dir_entry_ftype(ap_filetype_e *type, struct dir_t *thedir)
     struct stat filestat;
     char *fname = NULL;
 
+    if (type == NULL || thedir == NULL)
+        return APR_EBADARG;
+
     if (thedir->entry == NULL) {
         *type = APR_REG;
         return APR_ENOFILE;
@@ -287,10 +322,13 @@ ap_status_t ap_dir_entry_ftype(ap_filetype_e *type, struct dir_t *thedir)
  */                        
 ap_status_t ap_get_dir_filename(char **new, struct dir_t *thedir)
 {
+    if (new == NULL)
+        return APR_EBADARG;
+
     /* Detect End-Of-File */
     if (thedir == NULL || thedir->entry == NULL) {
-       *new = NULL;
-       return APR_ENOENT;
+        *new = NULL;
+        return APR_ENOENT;
     }
     (*new) = ap_pstrdup(thedir->cntxt, thedir->entry->d_name);
     return APR_SUCCESS;
diff --git a/file_io/unix/fileacc.c b/file_io/unix/fileacc.c
index de2ab39bd..7fd081f2d 100644
--- a/file_io/unix/fileacc.c
+++ b/file_io/unix/fileacc.c
@@ -64,6 +64,9 @@
  */                     
 ap_status_t ap_get_filename(char **new, struct file_t *thefile)
 {
+    if(new == NULL)
+        return APR_EBADARG;
+
     if (thefile != NULL) {
         *new = ap_pstrdup(thefile->cntxt, thefile->fname);
         return APR_SUCCESS;
@@ -111,11 +114,14 @@ mode_t get_fileperms(ap_fileperms_t mode)
  */                     
 ap_status_t ap_get_filedata(void **data, char *key, struct file_t *file)
 {    
+    if(data == NULL || key == NULL)
+        return APR_EBADARG;
+
     if (file != NULL) {
         return ap_get_userdata(data, key, file->cntxt);
     }
     else {
-        data = NULL;
+        *data = NULL;
         return APR_ENOFILE;
     }
 }
@@ -132,11 +138,13 @@ ap_status_t ap_get_filedata(void **data, char *key, struct file_t *file)
 ap_status_t ap_set_filedata(struct file_t *file, void *data, char *key,
                             ap_status_t (*cleanup) (void *))
 {    
+    if(data == NULL || key == NULL)
+        return APR_EBADARG;
+
     if (file != NULL) {
         return ap_set_userdata(data, key, cleanup, file->cntxt);
     }
     else {
-        data = NULL;
         return APR_ENOFILE;
     }
 }
diff --git a/file_io/unix/filedup.c b/file_io/unix/filedup.c
index 1eee401eb..a8745c661 100644
--- a/file_io/unix/filedup.c
+++ b/file_io/unix/filedup.c
@@ -59,12 +59,16 @@
  *    duplicate the specified file descriptor.
  * arg 1) The structure to duplicate into. 
  * arg 2) The file to duplicate.
+ * NOTE: *arg1 must point to a valid ap_file_t, or point to NULL
  */         
 ap_status_t ap_dupfile(struct file_t **new_file, struct file_t *old_file)
 {
     char *buf_oflags;
     int have_file = 0;
 
+    if (new_file == NULL || old_file == NULL)
+        return APR_EBADARG;
+
     if ((*new_file) == NULL) {
         (*new_file) = (struct file_t *)ap_pcalloc(old_file->cntxt,
                                    sizeof(struct file_t));
@@ -92,6 +96,8 @@ ap_status_t ap_dupfile(struct file_t **new_file, struct file_t *old_file)
         }
         (*new_file)->filehand = freopen(old_file->fname, buf_oflags, 
                                         old_file->filehand); 
+	if ((*new_file)->filehand == NULL)
+	    return errno;
     }
     else {
         if (have_file) {
diff --git a/file_io/unix/filestat.c b/file_io/unix/filestat.c
index 10c75efb2..0835c5f6f 100644
--- a/file_io/unix/filestat.c
+++ b/file_io/unix/filestat.c
@@ -89,8 +89,16 @@ static ap_filetype_e filetype_from_mode(int mode)
 ap_status_t ap_getfileinfo(ap_finfo_t *finfo, struct file_t *thefile)
 {
     struct stat info;
-/* XXX: this should be the equivalent of fstat() in unix, using stat() here is wrong */
-    int rv = stat(thefile->fname, &info);
+    int rv;
+
+    if (finfo == NULL || thefile == NULL)
+        return APR_EBADARG;
+
+    if (thefile->filehand == NULL) {
+        rv = fstat(thefile->filedes, &info);
+    } else {
+        rv = stat(thefile->fname, &info);
+    }
 
     if (rv == 0) {
         finfo->protection = info.st_mode;
@@ -120,9 +128,15 @@ ap_status_t ap_getfileinfo(ap_finfo_t *finfo, struct file_t *thefile)
 ap_status_t ap_stat(ap_finfo_t *finfo, const char *fname, ap_context_t *cont)
 {
     struct stat info;
-    int rv = stat(fname, &info);
+    int rv;
 
-    if (rv == 0) {
+    if(finfo == NULL || fname == NULL)
+        return APR_EBADARG;
+
+    if(cont == NULL)
+        return APR_ENOCONT;
+
+    if ((rv = stat(fname, &info)) == 0) {
         finfo->protection = info.st_mode;
         finfo->filetype = filetype_from_mode(info.st_mode);
         finfo->user = info.st_uid;
diff --git a/file_io/unix/open.c b/file_io/unix/open.c
index 43b84fcd1..d165e2c48 100644
--- a/file_io/unix/open.c
+++ b/file_io/unix/open.c
@@ -59,6 +59,7 @@ ap_status_t file_cleanup(void *thefile)
 {
     struct file_t *file = thefile;
     int rv;
+
     if (file->buffered) {
         rv = fclose(file->filehand);
     }
@@ -96,13 +97,20 @@ ap_status_t file_cleanup(void *thefile)
  * arg 4) Access permissions for file.
  * arg 5) The context to use.
  * NOTE:  If mode is APR_OS_DEFAULT, the system open command will be 
- *        called without any mode parameters.
+ *        called without any mode parameters.  *arg1 must point to a valid
+ *        file_t, or NULL (in which case it will be allocated)
  */
 ap_status_t ap_open(struct file_t **new, const char *fname, ap_int32_t flag,  ap_fileperms_t perm, ap_context_t *cont)
 {
     int oflags = 0;
     char *buf_oflags;
 
+    if (new == NULL)
+        return APR_EBADARG;
+
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     if ((*new) == NULL) {
         (*new) = (struct file_t *)ap_palloc(cont, sizeof(struct file_t));
     }
@@ -192,6 +200,9 @@ ap_status_t ap_close(struct file_t *file)
 {
     ap_status_t rv;
   
+    if (file == NULL)
+        return APR_EBADARG;
+
     if ((rv = file_cleanup(file)) == APR_SUCCESS) {
         ap_kill_cleanup(file->cntxt, file, file_cleanup);
         return APR_SUCCESS;
@@ -209,6 +220,9 @@ ap_status_t ap_close(struct file_t *file)
  */
 ap_status_t ap_remove_file(char *path, ap_context_t *cont)
 {
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     if (unlink(path) == 0) {
         return APR_SUCCESS;
     }
@@ -227,6 +241,9 @@ ap_status_t ap_remove_file(char *path, ap_context_t *cont)
  */
 ap_status_t ap_get_os_file(ap_os_file_t *thefile, struct file_t *file)
 {
+    if (thefile == NULL)
+        return APR_EBADARG;
+
     if (file == NULL) {
         return APR_ENOFILE;
     }
@@ -254,6 +271,13 @@ ap_status_t ap_put_os_file(struct file_t **file, ap_os_file_t *thefile,
                            ap_context_t *cont)
 {
     int *dafile = thefile;
+    
+    if (file == NULL || thefile == NULL)
+        return APR_EBADARG;
+
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     if ((*file) == NULL) {
         (*file) = ap_pcalloc(cont, sizeof(struct file_t));
         (*file)->cntxt = cont;
@@ -277,6 +301,9 @@ ap_status_t ap_put_os_file(struct file_t **file, ap_os_file_t *thefile,
  */
 ap_status_t ap_eof(ap_file_t *fptr)
 {
+    if (fptr == NULL)
+        return APR_EBADARG;
+
     if (fptr->buffered) {
         if (feof(fptr->filehand) == 0) {
             return APR_SUCCESS;
@@ -297,6 +324,9 @@ ap_status_t ap_eof(ap_file_t *fptr)
  */
 ap_status_t ap_ferror(ap_file_t *fptr)
 {
+    if (fptr == NULL)
+        return APR_EBADARG;
+
     if (ferror(fptr->filehand)) {
         return (-1);
     }
@@ -312,6 +342,12 @@ ap_status_t ap_ferror(ap_file_t *fptr)
  */
 ap_status_t ap_open_stderr(struct file_t **thefile, ap_context_t *cont)
 {
+    if (thefile == NULL)
+        return APR_EBADARG;
+
+    if (cont == NULL)
+        return APR_ENOCONT;
+
     (*thefile) = ap_pcalloc(cont, sizeof(struct file_t));
     if ((*thefile) == NULL) {
         return APR_ENOMEM;
diff --git a/file_io/unix/pipe.c b/file_io/unix/pipe.c
index 36aa3bcd0..a9028916a 100644
--- a/file_io/unix/pipe.c
+++ b/file_io/unix/pipe.c
@@ -86,6 +86,9 @@ static ap_status_t pipenonblock(struct file_t *thefile)
  */
 ap_status_t ap_set_pipe_timeout(struct file_t *thepipe, ap_int32_t timeout)
 {
+    if(thepipe == NULL)
+        return APR_EBADARG;
+
     if (thepipe->pipe == 1) {
         thepipe->timeout = timeout;
         return APR_SUCCESS;
@@ -105,6 +108,12 @@ ap_status_t ap_create_pipe(struct file_t **in, struct file_t **out, ap_context_t
 {
     int filedes[2];
 
+    if(in == NULL || out == NULL)
+        return APR_EBADARG;
+
+    if(cont == NULL)
+        return APR_ENOCONT;
+
     if (pipe(filedes) == -1) {
         return errno;
     }
@@ -144,6 +153,9 @@ ap_status_t ap_create_namedpipe(char *filename,
 {
     mode_t mode = get_fileperms(perm);
 
+    if(cont == NULL)
+        return APR_ENOCONT;
+
     if (mkfifo(filename, mode) == -1) {
         return errno;
     }
@@ -160,6 +172,9 @@ ap_status_t ap_block_pipe(ap_file_t *thepipe)
 #ifndef BEOS /* this code won't work on BeOS */
     int fd_flags;
 
+    if(thepipe == NULL)
+        return APR_EBADARG;
+
     fd_flags = fcntl(thepipe->filedes, F_GETFL, 0);
 #if defined(O_NONBLOCK)
     fd_flags &= ~O_NONBLOCK;
diff --git a/file_io/unix/readwrite.c b/file_io/unix/readwrite.c
index 06f3a808a..a83eec63a 100644
--- a/file_io/unix/readwrite.c
+++ b/file_io/unix/readwrite.c
@@ -69,6 +69,9 @@ ap_status_t ap_read(struct file_t *thefile, void *buf, ap_ssize_t *nbytes)
 {
     ap_ssize_t rv;
 
+    if(thefile == NULL || nbytes == NULL || (buf == NULL && *nbytes != 0))
+        return APR_EBADARG;
+
     if (thefile->filedes < 0 && !thefile->buffered) {
         *nbytes = 0;
         return APR_EBADF;
@@ -148,6 +151,9 @@ ap_status_t ap_write(struct file_t *thefile, void *buf, ap_ssize_t *nbytes)
 {
     ap_size_t rv;
 
+    if(thefile == NULL || nbytes == NULL || (buf == NULL && *nbytes != 0))
+        return APR_EBADARG;
+
     if (thefile->filedes < 0 && !thefile->buffered) {
         *nbytes = 0;
         return APR_EBADF;
@@ -221,6 +227,10 @@ ap_status_t ap_writev(struct file_t *thefile, const struct iovec *vec,
                       ap_size_t nvec, ap_ssize_t *nbytes)
 {
     int bytes;
+
+    if(thefile == NULL || vec == NULL || nvec < 0 || nbytes == NULL)
+        return APR_EBADARG;
+
     if ((bytes = writev(thefile->filedes, vec, nvec)) < 0) {
         *nbytes = 0;
         return errno;
@@ -240,6 +250,9 @@ ap_status_t ap_writev(struct file_t *thefile, const struct iovec *vec,
  */
 ap_status_t ap_putc(char ch, ap_file_t *thefile)
 {
+    if(thefile == NULL)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         if (fputc(ch, thefile->filehand) == ch) {
             return APR_SUCCESS;
@@ -260,6 +273,9 @@ ap_status_t ap_putc(char ch, ap_file_t *thefile)
  */
 ap_status_t ap_ungetc(char ch, ap_file_t *thefile)
 {
+    if(thefile == NULL)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         if (ungetc(ch, thefile->filehand) == ch) {
             return APR_SUCCESS;
@@ -280,6 +296,9 @@ ap_status_t ap_getc(char *ch, ap_file_t *thefile)
 {
     ssize_t rv;
     
+    if(thefile == NULL || ch == NULL)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         int r;
 
@@ -316,6 +335,9 @@ ap_status_t ap_puts(char *str, ap_file_t *thefile)
     ssize_t rv;
     int len;
 
+    if(thefile == NULL || str == NULL)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         if (fputs(str, thefile->filehand)) {
             return APR_SUCCESS;
@@ -337,6 +359,9 @@ ap_status_t ap_puts(char *str, ap_file_t *thefile)
  */
 ap_status_t ap_flush(ap_file_t *thefile)
 {
+    if(thefile == NULL)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         if (!fflush(thefile->filehand)) {
             return APR_SUCCESS;
@@ -361,6 +386,9 @@ ap_status_t ap_fgets(char *str, int len, ap_file_t *thefile)
     ssize_t rv;
     int i;    
 
+    if(thefile == NULL || str == NULL || len < 0)
+        return APR_EBADARG;
+
     if (thefile->buffered) {
         if (fgets(str, len, thefile->filehand)) {
             return APR_SUCCESS;
@@ -402,6 +430,9 @@ API_EXPORT(int) ap_fprintf(struct file_t *fptr, const char *format, ...)
     char *buf;
     int len;
 
+    if(fptr == NULL || format == NULL)
+        return APR_EBADARG;
+
     buf = malloc(HUGE_STRING_LEN);
     if (buf == NULL) {
         return 0;