httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Canonical/Filename testing to APR
Date Mon, 16 Oct 2000 00:28:07 GMT

  I've cleaned up some of old, lingering symbols in os.h since
they were apr'ized.  I didn't try to finish, since I'm really
focusing on this next point, and nobody likes mixed up actions
in the cvs list.  I needed to commit to get them out of the way.

These symbols need to leave the core:

  extern int ap_os_is_path_absolute(const char *file);
  #define ap_os_canonical_filename(p,f)  (f)
  #define ap_os_case_canonical_filename(p,f)  (f)
  #define ap_os_systemcase_filename(p,f)  (f)
  #define ap_os_is_filename_valid(f)          (1)

I'll go over each one, and give you a rundown...

ap_os_is_path_absolute(const char *file) becomes apr_is_path_absolute()
and we need a companion apr_is_path_relative() for mod_include, etc.

ap_os_systemcase_filename() is referenced nowhere.  It should be in 
nearly 80% of the cases we are playing with.  Unix uses systemcase 
names, and so should win32/os2/etc.

ap_os_canonical_filename is a unix noop that win32, os2, etc play some
games with.  They aren't consistent games.  OS2 full-paths the string.
Win32 cleans up the string.  This needs a facelift.

ap_os_case_canonical_filename is a unix noop that win32, os2, etc all
simply lowercase the ap_os_canonical_filename, except that it reparses
the -already-canonized- name?!?  This is really the worst brutilization
of the names, now that they don't match the input or filesystem name.

ap_os_is_filename_valid simply becomes apr_is_filename_valid.  A bunch
of other util.c functions should also move out with these basic functions.
We aught to consolidate somewhat.

---------

Since an apr_canonical_filename should return a predictable name that cannot
be bastardized or circumvented, it should logically become the systemcase.
It should also perform the no2slash conversion and resolve /../ and kill /./
references.  Consider what this http_request fragment sets out to do...

    r->filename   = ap_os_case_canonical_filename(r->pool, r->filename);

    res = get_path_info(r);
    if (res != OK) {
        return res;
    }

    r->filename   = ap_os_canonical_filename(r->pool, r->filename);

    test_filename = apr_pstrdup(r->pool, r->filename);

    ap_no2slash(test_filename);

This is very, very silly.  Let us assume for a moment that apr_canonical_path
has a single purpose - fix the entire file path to the proper canonical path.

Now we have a simpler code stream...

    r->filename = apr_canonical_path(r->pool, r->filename);

    res = get_path_info(r);
    if (res != OK) {
        return res; 
    }

  we are very done.  A true apr_canonical_path has cleaned out all of the
nasty '/../', '/./', '//', and any platform specific deviances, such as the
ignored trailing '.'s in win32, and provided the true path name, case, long
names and all.  Since this fixes -any- element, it aught to be canonical_path,
not _filename.  

  Can everyone buy this underlying concept before I start specific changes to 
the APR and proposed patches to the core?

  On a related issue... I intend to make apr_get_canonical_path and friends
for apr_file_t manipulation.  I'm thinking of a flag (to apr_open, etc)
that says 'already canonical' to bypass testing.  I'm also thinking that
apr_get_canonical_filename will format canonical if the canonical_filename
pointer is null, but cache the value otherwise.  This should greatly improve
the win32/os2 situations.

Bill



Mime
View raw message