httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@covalent.net>
Subject Re: [PATCH] autoindex instead of index.html
Date Thu, 27 Sep 2001 18:21:51 GMT
From: "Greg Ames" <gregames@remulak.net>
Sent: Thursday, September 27, 2001 10:14 AM


> "William A. Rowe, Jr." wrote:
> >
> > -1 on this patch!!!
>
> Forget it.  Your revision 1.58 patch to core.c broke configurations with
> mod_rewrite enabled.  Because of that, I am herebyvetoing your 1.58
> patch.  If I find that the server performs more correctly without it, I
> will back it out.  If you come up with something else that works with
> mod_rewrite in the mean time, great.

Then back it out already, if you like.  From the CVS commit;

  Can it be this simple?  No, probably not, but this fast-hack will get
  us going again for a while.

  We are currently rejecting some internal file_sub_req()'s in the
  translate phase.  I don't like this hack because of risks it potentially
  exposes, but for today, if we have a filename - and we are a subrequest,
  then let it fly without further mapping.  This allows us to serve up
  the default "/" request (run through mod_dir->mod_negotiation->mod_mime)
  without a 400 error.  The right solution is to set up some traps and
  escapes for the subreq mechanism, possibly with a subreq translate hook,
  and drop the URI entirely for these cases.


===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/core.c,v
retrieving revision 1.57
retrieving revision 1.58
diff -u -r1.57 -r1.58
--- httpd-2.0/server/core.c 2001/08/30 14:54:50 1.57
+++ httpd-2.0/server/core.c 2001/08/31 13:45:16 1.58
@@ -2867,13 +2867,19 @@
     void *sconf = r->server->module_config;
     core_server_config *conf = ap_get_module_config(sconf, &core_module);

+    /* XXX We have already been here, or another module did the work
+     * for us.  At this moment, we will enable only file subrequests.
+     */
+    if (r->main && r->filename)
+        return OK;
+
     /* XXX this seems too specific, this should probably become
      * some general-case test
      */
     if (r->proxyreq) {
         return HTTP_FORBIDDEN;
     }
-    if ((r->uri[0] != '/') && strcmp(r->uri, "*")) {
+    if (!r->uri || ((r->uri[0] != '/') && strcmp(r->uri, "*"))) {
  ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
        "Invalid URI in request %s", r->the_request);
  return HTTP_BAD_REQUEST;


This is a very hackish hack.  Let's revisit the original problem.

translate_names was never invoked before for file subrequests.

> > If this is a subrequest, we DON'T WANT TO MUCK THIS UP!  It isn't
> > subject to all this remapping.  [Think sub_req_lookup_file.]
>
> huh?  since when are subrequests not subject to URI translation?  That's
> where this goes south.

Since mod_rewrite returns DECLINED - I entirely agree with your perspective.
Now there are bigger questions - but those can wait a few hours.  Let's
revert back to the original broken behavior, and evaluate a better solution
than the 1.58 patch above.

> > If someone munges filename, then they've broken things. My question is
> > what changed that introduced this bug.
>
> it was your rev. 1.58 patch to core.c.
>
> > And, why rewrite is getting tangled in it.
>
> mod_rewrite's URI translation phase has been setting r->filename for
> subrequests since at least Apache 1.2.
>
> You tried to give an old API a new meaning.  If you want to pass new
> info to ap_core_translate, it would be better if you did it in a way
> that won't impact existing modules (new field, hide it in
> r->request_config for core, whatever).  But if you must use an old API
> in a new way, that's fine, as long as you also change the modules that
> use the API, and document the new restriction.

HERE is the difference.  In 1.2-1.3, we have never called the normal
request cycle for subrequests, we've had hackish, occasionally broken and
unmaintainable variants for each subrequest flavor.  Several months ago
I joined those in common code.  Within the last month I rerouted all that
brokenness through ONE block of code (ap_internal_process_request) that
will draw out any discrepancies.

Now we are back at square one, each ap_internal_process_request() step
needs to be optimized.  Several folks have carefully examined the loc_walk
optimizations that cancel out the penalties here.  dir_walk and file_walk
will soon have similar optimizations.  proxy_walk overrode those costly
operations, and other 3rd party modules may do the same.

Anyone can look at internal subrequest/redirect authn/authz optimizations
to spare those steps.  Anyone can look at the other phases and make the
same decisions.  But we are arguing here about the translate_names phase.

I'd argue the right fix -could- be a translate_name RUN_VERY_FIRST hook
in the request.c module, that says, "Hmmm... a _file_ subreq, better skip
the translate name phase" and returns OK.

File subrequests are for backwards-brokenness (e.g. they escape from a
bunch of potential rules) since any given file may not have a vhost/uri
mapping at all.  That's what we've used forever, I don't see that being
resolved for 2.0.

Would a hook satisfy everyone (preventing mod_rewrite expansion, which
never happened in 2.0 subreq_lookup_file in the first place)?

Bill


Mime
View raw message