William A. Rowe, Jr. wrote:

At 09:32 AM 4/16/2002, you wrote:

brane       02/04/16 07:32:03

Modified: include apr_errno.h
Log:
On Windows, ERROR_PATH_NOT_FOUND is an ENOENT, not an ENOTDIR -- same as OS/2.


I'm somewhat dubious of this change. Can you please point to the use case
(a specific scenario) that this existing define misbehaved? The greatest risk is
that Apache -needs- to see ENOTDIR in the right times to avoid a ton of extra
effort, but most importantly, to make the right security decisions on errors.


Bill


I finally got some time to write a small test program, to compare the errors returned on Windows an Unix for various operations on (nonexistent) files and directories. The Unix variants I tried were HP-UX 10.20 and Solaris 7, where results were identical. I'm attaching the program itself and raw HP-UX and Win32 results.



First, the more interesting differences

   * open dir: Unix: SUCCESS, Windows: EACCES
     Not much can be done about this, and I don't think we should. This
     is a basic difference between the OS's. (Actually, you *can* open
     a directory as a file on Windows, but it's not something you'd
     want to do in normal circumstances. Well, you wouldn't want to do
     it on Unix in normal circumstances, either ...)

   * remove dir: Unix EPERM, Windows EACCES
     Basically the same result; I wonder, though, if it would make
     sense to include EPERM in APR_STATUS_IS_EACCES?

   * rmdir file: Unix ENOTDIR, Windows ERROR_DIRECTORY
     That's "The directory name is invalid" on Windows. Perhaps this
     code should be included in APR_STATUS_IS_ENOTDIR?


Now, the cases where the Unix status is ENOENT and the Windows status is ENOTDIR:


   open under noex
   mkdir under noex
   stat under noex
   remove under noex
   rmdir under noex

These are all tests where the parameter is a path (i.e., contains at least one path separator), and one of the components doesn't exist; this is what originally prompted me to make the change -- too hastily, I admit.

But it's exactly such cases that are giving me grief in Subversion on Windows, so I've been thinking what to do about them. Trying to get 100% correct codes would be a pain, because every time you got an ERROR_PATH_NOT_FOUND, you'd have to do a lot of extra stats. OTOH, if you want to make security decisions, you'd better have correct error information in the first place.

There's another, less clean solution: to include ERROR_PATH_NOT_FOUND in *both* APR_STATUS_IS_ENOENT and APR_STATUS_IS_ENOTDIR. This would fix the particular problem I'm having in Subversion, but might be a major PITA in other ways I'm not aware of.

Suggestions?

--
Brane Äibej   <[EMAIL PROTECTED]>   http://www.xbc.nu/brane/


#include <apr_errno.h>
#include <apr_file_info.h>
#include <apr_file_io.h>
#include <apr_general.h>
#include <apr_pools.h>

#define APR_WANT_STDIO
#include <apr_want.h>

/* Test initialization:
   Create a file 'F' and a directory 'D', and make sure that 'X'
   doesn't exist. */
static apr_status_t init_test (apr_pool_t *pool)
{
    apr_status_t status;
    apr_finfo_t finfo;
    apr_file_t *file;
    char errbuf[1024];

    status = apr_stat(&finfo, "X", APR_FINFO_MIN, pool);
    if (!APR_STATUS_IS_ENOENT(status))
    {
        fprintf(stderr, "X exists; please remove it\n");
        return APR_EEXIST;
    }

    status = apr_file_open(&file, "F",
                           APR_READ | APR_CREATE, APR_OS_DEFAULT,
                           pool);
    if (!status)
        apr_file_close(file);
    else
    {
        fprintf(stderr, "can't open file F: %s\n",
                apr_strerror(status, errbuf, sizeof errbuf));
        return status;
    }

    status = apr_dir_make("D", APR_OS_DEFAULT, pool);
    if (status)
    {
        fprintf(stderr, "can't create empty dir D: %s\n",
                apr_strerror(status, errbuf, sizeof errbuf));
        return status;
    }

    return APR_SUCCESS;
}


static apr_status_t do_stat (const char *name, apr_pool_t *pool)
{
    apr_finfo_t finfo;
    return apr_stat(&finfo, name, APR_FINFO_MIN, pool);
}

static apr_status_t do_mkdir (const char *name, apr_pool_t *pool)
{
    return apr_dir_make(name, APR_OS_DEFAULT, pool);
}

static apr_status_t do_rmdir (const char *name, apr_pool_t *pool)
{
    return apr_dir_remove(name, pool);
}

static apr_status_t do_remove (const char *name, apr_pool_t *pool)
{
    return apr_file_remove(name, pool);
}

static apr_status_t do_open (const char *name, apr_pool_t *pool)
{
    apr_file_t *file;
    apr_status_t status = apr_file_open(&file, name, APR_READ,
                                        APR_OS_DEFAULT, pool);
    if (!status)
        apr_file_close(file);
    return status;
}

static void print_status (apr_status_t status)
{
    char errbuf[1024];
    int prn = 0;
    if (!status)
    {
        printf("SUCCESS");
        prn = 1;
    }
    if (APR_STATUS_IS_ENOENT(status))
    {
        printf("%sENOENT", (prn ? ", ": ""));
        prn = 1;
    }
    if (APR_STATUS_IS_ENOTDIR(status))
    {
        printf("%sENOTDIR", (prn ? ", ": ""));
        prn = 1;
    }
    if (APR_STATUS_IS_EEXIST(status))
    {
        printf("%sEEXIST", (prn ? ", ": ""));
        prn = 1;
    }
    if (APR_STATUS_IS_EACCES(status))
    {
        printf("%sEACCES", (prn ? ", ": ""));
        prn = 1;
    }
    if (prn)
        printf("\n");
    else
        printf("%d: %s\n", (int) status,
               apr_strerror(status, errbuf, sizeof errbuf));
}


int main (void)
{
    apr_status_t status = APR_SUCCESS;
    apr_pool_t *pool = NULL;

    apr_initialize();

    if (!status)
        status = apr_pool_create(&pool, 0);

    if (!status)
        status = init_test(pool);

    if (!status)
    {
        printf("open dir: ............. ");
        print_status(do_open("D", pool));

        printf("open noex: ............ ");
        print_status(do_open("X", pool));

        printf("open noex under file:   ");
        print_status(do_open("F/X", pool));

        printf("open noex under dir: .. ");
        print_status(do_open("D/X", pool));

        printf("open under noex: ...... ");
        print_status(do_open("X/X", pool));

        printf("mkdir file: ........... ");
        print_status(do_mkdir("F", pool));

        printf("mkdir under file: ..... ");
        print_status(do_mkdir("F/X", pool));

        printf("mkdir under noex: ..... ");
        print_status(do_mkdir("X/X", pool));

        printf("stat noex: ............ ");
        print_status(do_stat("X", pool));

        printf("stat noex under file:   ");
        print_status(do_stat("F/X", pool));

        printf("stat noex under dir: .. ");
        print_status(do_stat("D/X", pool));

        printf("stat under noex: ...... ");
        print_status(do_stat("X/X", pool));

        printf("remove noex: .......... ");
        print_status(do_remove("X", pool));

        printf("remove dir: ........... ");
        print_status(do_remove("D", pool));

        printf("remove noex under file: ");
        print_status(do_remove("F/X", pool));

        printf("remove noex under dir:  ");
        print_status(do_remove("D/X", pool));

        printf("remove under noex: .... ");
        print_status(do_remove("X/X", pool));

        printf("rmdir noex: ........... ");
        print_status(do_rmdir("X", pool));

        printf("rmdir file: ........... ");
        print_status(do_rmdir("F", pool));

        printf("rmdir noex under file:  ");
        print_status(do_rmdir("F/X", pool));

        printf("rmdir noex under dir:   ");
        print_status(do_rmdir("D/X", pool));

        printf("rmdir under noex: ..... ");
        print_status(do_rmdir("X/X", pool));
    }

    if (pool)
        apr_pool_destroy(pool);
    apr_terminate();
    return (status != APR_SUCCESS);
}
open dir: ............. SUCCESS
open noex: ............ ENOENT
open noex under file:   ENOTDIR
open noex under dir: .. ENOENT
open under noex: ...... ENOENT
mkdir file: ........... EEXIST
mkdir under file: ..... ENOTDIR
mkdir under noex: ..... ENOENT
stat noex: ............ ENOENT
stat noex under file:   ENOTDIR
stat noex under dir: .. ENOENT
stat under noex: ...... ENOENT
remove noex: .......... ENOENT
remove dir: ........... 1: Not owner
remove noex under file: ENOTDIR
remove noex under dir:  ENOENT
remove under noex: .... ENOENT
rmdir noex: ........... ENOENT
rmdir file: ........... ENOTDIR
rmdir noex under file:  ENOTDIR
rmdir noex under dir:   ENOENT
rmdir under noex: ..... ENOENT
open dir: ............. EACCES
open noex: ............ ENOENT
open noex under file:   ENOTDIR
open noex under dir: .. ENOENT
open under noex: ...... ENOTDIR
mkdir file: ........... EEXIST
mkdir under file: ..... ENOTDIR
mkdir under noex: ..... ENOTDIR
stat noex: ............ ENOENT
stat noex under file:   ENOTDIR
stat noex under dir: .. ENOENT
stat under noex: ...... ENOTDIR
remove noex: .......... ENOENT
remove dir: ........... EACCES
remove noex under file: ENOTDIR
remove noex under dir:  ENOENT
remove under noex: .... ENOTDIR
rmdir noex: ........... ENOENT
rmdir file: ........... 22767: The directory name is invalid.  
rmdir noex under file:  ENOTDIR
rmdir noex under dir:   ENOENT
rmdir under noex: ..... ENOTDIR

Reply via email to