On Wed, 1 Apr 2015 00:36:23 +0200
Graham Leggett <minf...@sharp.fm> wrote:
> Hi all,
>
> I am picking apart some strange behaviour where use of the Alias
> directive inside a Location along with a file path that doesn’t exist
> (/_thumbs/i/dont/exist.jpg) triggers a loop of redirects to
> …/index.html/index.html/index.html/…
>
> The loop of redirects is caused by mod_dir being told that the non
> existent file is actually a directory (r->finfo.filetype == APR_DIR)
> instead of a nonexistent file (r->finfo.filetype == APR_NOFILE).
> Working backwards to find where a non existent file is being
> mislabelled as a directory, it leads me to ap_directory_walk(),
> specifically these lines:
>
> do {
> [snip]
>
> if (*seg_name == '/')
> ++seg_name;
>
> /* If nothing remained but a '/' string, we are finished
> * XXX: NO WE ARE NOT!!! Now process this puppy!!! */
> if (!*seg_name) {
> break;
> }
>
> [snip]
> } while (thisinfo.filetype == APR_DIR);
>
> What is happening is that we are reaching the break, which aborts the
> loop but leaves the value thisinfo.filetype == APR_DIR, thus
> triggering the downstream mod_dir mayhem.
>
> Further down in the loop, we have some code that looks like this that
> explicitly sets the file to APR_NOFILE before breaking:
>
> if (APR_STATUS_IS_ENOENT(rv)) {
> /* Nothing? That could be nice. But our directory
> * walk is done.
> */
> thisinfo.filetype = APR_NOFILE;
> break;
> }
>
> In theory following the pattern above, we might change the code above
> to:
>
> if (!*seg_name) {
> + thisinfo.filetype = APR_NOFILE;
> break;
> }
Would we do so for /foo where foo is the directory, or /foo/ where foo
is a container? I believe we want to do a force .filetype = APR_NOFILE
where the last finfo responds with something other than OK?
> However I don’t fully understand what the comment "XXX: NO WE ARE
> NOT!!! Now process this puppy!!!” actually means.
http://marc.info/?l=apache-cvs&m=103967676907505
suggests that this was a grammatical argument, that the original
comment suggested that the walk function was finished, but in fact,
this 'break' continued processing of the walk function.
Why this has only just begun to occur is very likely related to other
recently reverted behavior changes, I'd look to see if an inappropriate
patch was not fully reverted?
