Bug 40373

Summary: mod_dir adds trailing slash after internal redirect by mod_rewrite to a non-folder uri
Product: Apache httpd-2 Reporter: Bob Ionescu <bobsiegen>
Component: mod_rewriteAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: NEW ---    
Severity: normal CC: bugzilla-apache
Priority: P2 Keywords: PatchAvailable
Version: 2.5-HEAD   
Target Milestone: ---   
Hardware: Other   
OS: other   
Attachments: proposed patch
debugging stuff of hook processing
patch against trunk r723565

Description Bob Ionescu 2006-08-31 01:02:28 UTC
Behavior:
DocumentRoot /var/www
<Directory /var/www>
RewriteEngine on
RewriteRule ^foo/(.*) /bar/index.php?path=$1 [L]
</Directory>

Request /foo/bar
which resolves as r->filename to /var/www/foo/bar, the folder bar exists.

Problem: The RewriteRule matches against the local path foo/bar, rewriting to
/bar/index.php?path=bar, splitting args and going into the internal redirect
with /bar/index.php as expected.
Now, before the redirect processing of mod_rewrite occures, mod_dir sends an 301
redirect to /foo/bar/?path=bar which results in a new request by the client, of
course.

It looks like mod_dir acts after mod_rewrite calls ap_internal_redirect but with
the old r->uri and not the one from the internal redirect.

As far as I understand ap_internal_redirect() creates a "new" request and apache
runs it but this does not stop the processing of fixup_hooks for the "original"
request? fixup_dir is registerd as APR_HOOK_LAST while mod_rewrite's hook_fixup
is registerd as APR_HOOK_FIRST.

Proposed patch: Returning DECLINED in mod_dir if r->handler is
"redirect-handler". That should work for the 2.0 branch as well.
Comment 1 Bob Ionescu 2006-08-31 01:03:44 UTC
Created attachment 18773 [details]
proposed patch
Comment 2 William A. Rowe Jr. 2006-08-31 01:34:25 UTC
I actually think we need to move up one step higher...

since rewrite acted on the url it shouldn't have (as you noted) trusted the
old r->filename, worse, it shouldn't have trusted the old r->finfo.

Because rewrite is leaving stale finfo there, we have serious problems.

I think the patch is a hack around only one wrinkle of one module... and would
love help in determining where rewrite went wrong and solving the underlying
issue so that no module will strike this unexpected scenario.
Comment 3 Bob Ionescu 2006-08-31 19:02:27 UTC
I think the actually rewriting process is correct, but the internal redirect
goes some way wrong:

I've done some debugging using ap_log_rerror to see when which hook gets active
and what are the values of r->uri and r->filename.

I think I found the dilemma: mod_rewrite sets r->handler to 'redirect-handler'
in the fixup_hook. Now mod_rewrite has an other hook called 'handler_redirect'
(APR_HOOK_MIDDLE) which finally calls ap_internal_redirect(). But interestingly
this hook is registered as APR_HOOK_MIDDLE, but fuxup hooks registered as
APR_HOOK_LAST (such as mod_rewrite's mime hook or mod_dir's fixup_dir) are being
processed before the handler_redirect hook runs.
I attached a portion of my error log showing the order of processing.

The processing of mod_rewrite's hook_mimetype is useless here, because the
request won't be served, the mime-type is lost with the internal redirect (bug
36590, fix: using ap_internal_redirect_handler() if the H flag is used and we
need an internal redirect [I don't know if this will preserve the mime-type/T
flag, too].)

So from how it currently works - I can't see a reason why mod_rewrite's fixup
hook doesn't call ap_internal_redirect() directly, because any past processing
after hook_fixup seems to be useless to me. And I don't know how a fix of bug
36590 could break many existing cases, because the defined handler will never
run/mime-type never set to something which is served.

An other case came just into my mind: mod_rewrite sets r->handler to be
redirect-handler. Now, if we're using the new H flag we're overriding this
handler with sthg. other in the hook_mimetype and an the internal redirect won't
take place any more, resulting in a 404, because 'redirect:/url/path' cannot be
found. The hook "handler_redirect" seems to run after the fixup hook_mimetype. I
don't know if this is just an issue with my built... may be someone can try to
reproduce this.

Actually, the rewrite log entry "[INTERNAL REDIRECT]" is wrong, because at this
state only r->handler is set.

Yes, if mod_rewrite sets r->handler to be 'redirect-handler' the module leaves
r->uri with the value created by ap_process_request_internal and below from core
while r->filename has 'redirect:(url-path)'. Copying the url-path from
r->filename back to r->uri ("updating r->uri") won't be a solution because this
is also a dirty fix which would cause the same problem in an other case.
(rewriting /abc to /foo/bar, r->uri would be updated now to /foo/bar, mod_dir
acts and would 301' to /foo/bar/ before the internal redirect takes place. So
there is no chance due to the missing internal redirect processing to match e.g.
now in per-server context against /foo/bar and rewrite it to
/index.php?args=/foo/bar).
Comment 4 Bob Ionescu 2006-08-31 19:02:43 UTC
Created attachment 18779 [details]
debugging stuff of hook processing
Comment 5 Bob Ionescu 2008-12-13 14:07:49 UTC
Comment on attachment 18773 [details]
proposed patch

Another approach: mod_dir checks, if r->finfo.filetype is set to APR_DIR. That is true if r->filename is an existing dir, of course. If that's the case, mod_dir checks if r->uri does not end with a trailing slash.

Now, mod_rewrite's fixup_hook modifies r->filename to sthg. redirect:/url-path and leaves r->uri unchanged. Copying r->filename or the new URL-path for the internal redirect to r->uri (PT-Flag, ignored in per-dir context) would not help, because if you rewrite to /bar/index.php and copy that to r->uri, r->uri won't end with a trailing slash, hence we would get a redirect now to /bar/index.php/ (instead of /foo/bar/ without modifying r->uri).

Therefore I'd suggest we set r->finfo.filetype to NULL in mod_rewrite's fixup-hook where we force r->handler to be redirect-handler. After a rewrite with applying a substitution in per-dir context occurred, we changed r->filename to redirect:/URL-path, which doesn't resolve to a directory anymore at this stage of processing.
Comment 6 Bob Ionescu 2008-12-13 14:09:38 UTC
Created attachment 23012 [details]
patch against trunk r723565
Comment 7 Bob Ionescu 2008-12-13 14:12:52 UTC
*** Bug 31210 has been marked as a duplicate of this bug. ***
Comment 8 fictive 2010-11-27 10:52:25 UTC
This problem is still present in Apache 2.2
What is the status of this bug?
Comment 9 disassembler 2014-10-28 20:06:43 UTC
Encountered the same situation today on 2.4.10.

Found a workaround which could be applied in original poster's configuration as follows:

DocumentRoot /var/www
<Directory /var/www>
  DirectorySlash Off
  RewriteEngine On
  RewriteOptions AllowNoSlash
  RewriteRule ^foo/(.*) /bar/index.php?path=$1 [L]
</Directory>