You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <je...@ebuilt.com> on 2002/03/03 08:38:15 UTC
Re: cvs commit: httpd-2.0/server util_filter.c
On Sun, Mar 03, 2002 at 06:04:08AM -0000, rbb@apache.org wrote:
> rbb 02/03/02 22:04:08
>
> Modified: . STATUS
> include util_filter.h
> server util_filter.c
> Log:
> This finishes the mod_dir/mod_negotiation bug. This final part of the
> solution ensures that we don't lose filters if they are added later than
> we expect. The problem could be seen if a connection filter was added
> after a request-based filter was added in the past. The problem was that
> the request-based filters pointed to the first filter in the connection
> record, so the new connection filter was never called. Now, all filters
> are put on their correct filter lists, and we are sure to always update
> all pointers when adding a filter.
Please fix this or back it out (your choice).
You just broke mod_dir and mod_negotation far worse than they
were before.
The header filters aren't getting added correctly, and at times,
too many header filters are getting added. I have included
two simple traces below (stock httpd-std.conf - no modifications).
I still fail to see why you think protocol filters would solve
this problem. I've explained my reasoning behind why I think this
isn't a good idea. I've also pointed out a suggestion that I
believe would work that is rather simple and would increase our
flexibility. I'd like to hear your reasons why my approach isn't
feasible.
Regardless, these commits are broken. -- justin
Trying 127.0.0.1...
Connected to localhost.localdomain.
Escape character is '^]'.
GET / HTTP/1.1
Host: foo
HTTP/1.1 200 OK
Date: Sun, 03 Mar 2002 07:28:10 GMT
Server: Apache/2.0.33-dev (Unix) DAV/2
Content-Location: index.html.en
Vary: negotiate,accept,accept-language,accept-charset
TCN: choice
Content-Length: 1456
Content-Type: text/html; charset=ISO-8859-1
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Test Page for Apache Installation</title>
</head>
<!-- Background white, links blue (unvisited), navy (visited), red
(active) -->
<body bgcolor="#FFFFFF" text="#000000" link="#0000FF"
vlink="#000080" alink="#FF0000">
<p>If you can see this, it means that the installation of the <a
href="http://www.apache.org/foundation/preFAQ.html">Apache web
server</a> software on this system was successful. You may now add
content to this directory and replace this page.</p>
<hr width="50%" size="8" />
<h2 align="center">Seeing this instead of the website you
expected?</h2>
<p>This page is here because the site administrator has changed the
configuration of this web server. Please <strong>contact the person
responsible for maintaining this server with questions.</strong>
The Apache Software Foundation, which wrote the web server software
this site administrator is using, has nothing to do with
maintaining this site and cannot help resolve configuration
issues.</p>
<hr width="50%" size="8" />
<p>The Apache <a href="manual/">documentation</a> has been included
with this distribution.</p>
<p>You are free to use the image below on an Apache-powered web
server. Thanks for using Apache!</p>
<div align="center"><img src="apache_pb.gif" alt="" /></div>
</body>
</html>
HTTP/1.1 200 OK
Date: Sun, 03 Mar 2002 07:28:10 GMT
Server: Apache/2.0.33-dev (Unix) DAV/2
Content-Location: index.html.en
Vary: negotiate,accept,accept-language,accept-charset
TCN: choice
Content-Location: index.html.en
TCN: choice
Content-Length: 1456
Content-Type: text/html; charset=ISO-8859-1
HTTP/1.1 200 OK
Date: Sun, 03 Mar 2002 07:28:10 GMT
Server: Apache/2.0.33-dev (Unix) DAV/2
Content-Location: index.html.en
Vary: negotiate,accept,accept-language,accept-charset
TCN: choice
Content-Location: index.html.en
TCN: choice
Content-Location: index.html.en
TCN: choice
Content-Length: 1456
Content-Type: text/html; charset=ISO-8859-1
Trying 127.0.0.1...
Connected to localhost.localdomain.
Escape character is '^]'.
GET /manual/ HTTP/1.1
Host: foo
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta name="generator" content="HTML Tidy, see www.w3.org" />
<title>Apache HTTP Server Version 2.0 Documentation</title>
</head>
<body bgcolor="#FFFFFF" text="#000000" link="#0000FF"
vlink="#000080" alink="#FF0000">
<div align="center">
<table cellspacing="0" cellpadding="0" border="0"
width="600">
<tr>
<td align="center"><img src="images/apache_header.gif"
width="600" height="62" border="0"
alt="[Apache Documentation]" /></td>
</tr>
<tr>
<td align="center" bgcolor="#4f4f4f">
<table cellspacing="1" cellpadding="4" border="0"
width="100%">
<tr>
<td align="center" bgcolor="#bebebe"><a
href="faq/"><strong>FAQ</strong></a> </td>
<td align="center" bgcolor="#bebebe"><a
href="sitemap.html"><strong>SiteMap</strong></a>
</td>
<td align="center" bgcolor="#bebebe"><a
href="mod/directives.html"><strong>Directives</strong></a>
</td>
<td align="center" bgcolor="#bebebe"><a
href="mod/"><strong>Modules</strong></a> </td>
<td align="center" bgcolor="#bebebe"><a
href="http://www.apache.org/search.html"><strong>Search</strong></a>
</td>
</tr>
</table>
</td>
</tr>
<tr>
<td> </td>
</tr>
<tr>
<td align="center" height="30">
<h3>Apache HTTP Server Version 2.0</h3>
</td>
</tr>
</table>
</div>
<div align="center">
<table cellspacing="0" cellpadding="0" border="0"
width="600">
<tr>
<td align="center">
<form method="post" action="http://search.apache.org/">
<input type="hidden" name="what"
value="httpd.apache.org" /> <input type="hidden"
name="results" value="20" /> <input type="hidden"
name="version" value="2" /> <input type="text"
name="keyword" size="20" /> <input type="submit"
value="Search" />
</form>
</td>
</tr>
</table>
<!--
<table cellspacing="0" cellpadding="0" border="0" width="600">
<tr>
<td align="center"><hr size="1" noshade width="100%">
</td>
</tr>
</table>
-->
<table cellspacing="0" cellpadding="0" border="0"
width="600">
<tr>
<td align="center" valign="top">
<table border="0" cellpadding="4" cellspacing="0"
bgcolor="#ffffff" width="280">
<tr>
<td align="center" bgcolor="#e9e9e9">
<strong>Release Notes</strong> </td>
</tr>
<tr>
<td><a href="new_features_2_0.html">New Features in
Version 2.0</a> </td>
</tr>
<tr>
<td><a href="upgrading.html">Upgrading to Version
2.0</a> </td>
</tr>
<tr>
<td><a href="LICENSE">Apache License</a> </td>
</tr>
</table>
<table border="0" cellpadding="4" cellspacing="0"
bgcolor="#ffffff" width="280">
<tr>
<td align="center" bgcolor="#e9e9e9">
<strong>Reference Manual</strong> </td>
</tr>
<tr>
<td><a href="install.html">Compiling and
Installing</a> </td>
</tr>
<tr>
<td><a href="invoking.html">Starting</a> </td>
</tr>
<tr>
<td><a href="stopping.html">Stopping or
Restarting</a> </td>
</tr>
<tr>
<td><a href="mod/directives.html">Run-time
Configuration Directives</a> </td>
</tr>
<tr>
<td>Modules: <a href="mod/index-bytype.html">By
Type</a> or <a href="mod/">Alphabetical</a> </td>
</tr>
<tr>
<td><a href="mpm.html">Multi-Processing Modules
(MPMs)</a> </td>
</tr>
<tr>
<td><a href="programs/">Server and Supporting
Programs</a> </td>
</tr>
<tr>
<td><a href="dso.html">Dynamic Shared Object (DSO)
Support</a> </td>
</tr>
</table>
<table border="0" cellpadding="4" cellspacing="0"
bgcolor="#ffffff" width="280">
<tr>
<td align="center" bgcolor="#e9e9e9">
<strong>Platform Specific Notes</strong> </td>
</tr>
<tr>
<td><a href="platform/windows.html">Microsoft
Windows</a> </td>
</tr>
<tr>
<td><a href="platform/netware.html">Novell NetWare
</a> </td>
</tr>
</table>
</td>
<td align="center" valign="top" bgcolor="#cccccc">
<table border="0" cellpadding="0" cellspacing="0"
bgcolor="#cccccc">
<tr>
<td align="center"><img src="images/pixel.gif"
width="1" height="1" border="0" alt="." /></td>
</tr>
</table>
</td>
<td align="center" valign="top">
<table border="0" cellpadding="4" cellspacing="0"
bgcolor="#ffffff" width="280">
<tr>
<td align="center" bgcolor="#e9e9e9"><strong>Using
the Apache HTTP Server</strong> </td>
</tr>
<tr>
<td><a href="configuring.html">Configuration
Files</a> </td>
</tr>
<tr>
<td><a href="server-wide.html">Server-Wide
Configuration</a> </td>
</tr>
<tr>
<td><a href="logs.html">Log Files</a> </td>
</tr>
<tr>
<td><a href="urlmapping.html">Mapping URLs to the
Filesystem</a> </td>
</tr>
<tr>
<td><a href="vhosts/">Virtual Hosts</a> </td>
</tr>
<tr>
<td><a href="ssl/">SSL/TLS Strong Encryption</a></td>
</tr>
<tr>
<td><a href="howto/ssi.html">Server Side
Includes</a> </td>
</tr>
<tr>
<td><a href="howto/cgi.html">Dynamic Content with
CGI</a> </td>
</tr>
<tr>
<td><a href="handler.html">Handlers</a> </td>
</tr>
<tr>
<td><a href="filter.html">Filters</a> </td>
</tr>
<tr>
<td><a href="content-negotiation.html">Content
negotiation</a> </td>
</tr>
<tr>
<td><a href="env.html">Environment Variables</a>
</td>
</tr>
<tr>
<td><a href="suexec.html">Using SetUserID Execution
for CGI</a> </td>
</tr>
<tr>
<td><a href="misc/perf-tuning.html">General
Performance hints</a> </td>
</tr>
<tr>
<td><a href="misc/security_tips.html">Security
tips</a> </td>
</tr>
<tr>
<td><a href="misc/rewriteguide.html">URL Rewriting
Guide</a> </td>
</tr>
</table>
<table border="0" cellpadding="4" cellspacing="0"
bgcolor="#ffffff" width="280">
<tr>
<td align="center" bgcolor="#e9e9e9"><strong>Other
Topics</strong> </td>
</tr>
<tr>
<td><a href="faq/">Frequently Asked Questions</a>
</td>
</tr>
<tr>
<td><a href="sitemap.html">SiteMap</a>
</td>
</tr>
<tr>
<td><a href="misc/tutorials.html">Tutorials</a>
</td>
</tr>
<tr>
<td><a href="developer/">Documentation for
Developers</a> </td>
</tr>
<tr>
<td><a href="misc/">Other Notes</a> </td>
</tr>
</table>
</td>
</tr>
</table>
</div>
<p align="center">Maintained by the <a
href="http://httpd.apache.org/docs-project/">Apache HTTP Server
Documentation Project</a>.</p>
<!--#include virtual="footer.html" -->
</body>
</html>
Re: cvs commit: httpd-2.0/server util_filter.c
Posted by 'Justin Erenkrantz' <je...@ebuilt.com>.
On Sun, Mar 03, 2002 at 10:20:52AM -0800, Ryan Bloom wrote:
> Was that the one about returning a 302 to the client? No way. If every
> time we do a multiviews or a mod_dir request we have to send a 302 to
> the client, we will cut our throughput horribly. You are actually
> talking about making most of the requests we serve require two requests.
> Think about it, the most common request people make to a web server is
> http://www.example.com/. To make that request return a 302 EVERY time
> is just wrong.
Eh, you obviously missed my point. I was saying conceptually what
we'd do is to return a 302. That's the real thing we *want* to do.
There should be no argument there. I never suggested returning a
302 every time - I just said that's the ideal.
But, instead do an internal_redirect from a hook and allow the
original request to be aborted. I'd suggest that you read
my entire message and attempt to understand what I'm saying.
> Plus, the protocol filters are already working. Please re-read this
> patch, it didn't add the protocol filters, it just changed how we insert
> filters. In reality, this patch is only broken because of the way we
> use our filter list.
Nope. I think you've missed how the filters are chained together
and that they won't be added correctly under any circumstances.
But, I'll wait for you to commit what you meant so I can decipher
your true meaning. Regardless, this just gets filters working.
We still have problems with headers and subreqs too. -- justin
RE: cvs commit: httpd-2.0/server util_filter.c
Posted by Ryan Bloom <rb...@covalent.net>.
> > Hooks are not the solution to every problem. We already have people
> > complaining that there are so many hooks that they don't really know
> > what is possible with the server, nor do they know what they have to
do
> > to make their modules work. Why would we add more hooks at this
point
> > when there are other cleaner solutions?
>
> I think you may have missed my message yesterday. It's not adding a
> hook - it's allowing a hook to be able to abort processing of a
> request. That's all. internal_redirect will take care of
> everything. fast_redirect is bogus because it skips hooks in the
> new request. I believe that is broken behavior. If we were to
> rely on internal_redirect, everything falls into place - without
> having to add all of this protocol_filters code that unnecessarily
> complicate matters.
Was that the one about returning a 302 to the client? No way. If every
time we do a multiviews or a mod_dir request we have to send a 302 to
the client, we will cut our throughput horribly. You are actually
talking about making most of the requests we serve require two requests.
Think about it, the most common request people make to a web server is
http://www.example.com/. To make that request return a 302 EVERY time
is just wrong.
Plus, the protocol filters are already working. Please re-read this
patch, it didn't add the protocol filters, it just changed how we insert
filters. In reality, this patch is only broken because of the way we
use our filter list.
Ryan
Re: cvs commit: httpd-2.0/server util_filter.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 11:54 AM 3/3/2002, you wrote:
>I think you may have missed my message yesterday. It's not adding a
>hook - it's allowing a hook to be able to abort processing of a
>request. That's all. internal_redirect will take care of
>everything. fast_redirect is bogus because it skips hooks in the
>new request. I believe that is broken behavior. If we were to
>rely on internal_redirect, everything falls into place - without
>having to add all of this protocol_filters code that unnecessarily
>complicate matters.
Justin, yes - it adds complexity. But we can't simply 'abort processing'
to speed things up. Perhaps we need to do so, as well, but I'm simply
responding to 'this is bogus'.
Perhaps it is. However, again consider mod_dir. We have a request.
It asks for a dir. We find the dir, and look at DirectoryIndex. We look
for the first item, 'index.php'. Nope, no php, destroy that subrequest.
We look for 'index.html'. We find that file. In checking for that file, we
have run through the process_request cycle.
What you are asking is that we re-invoke the entire process_request
cycle. That is doable, but it will be costly. And -that- cost, the cycles
to completely repeat process_request() for index.html, is why I changed
my focus from throwing this code path away, to making this code path
work correctly, if it is at all possible or realistic.
Rbb's derivative of your fix of my patch identified and solved the filters
problem. It helped Ryan notice that we were -really- messed up on
r->connection->output_filters never growing any connection filters added
to r->output_filters. That was bogus, I'm glad to see it now fixed.
There should be no problem 'appropriating' the connection+protocol
filters from the subreq->main request and using them to serve the
subreq as the top level request. If you see such an issue, please,
raise it.
Bill
Re: cvs commit: httpd-2.0/server util_filter.c
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sun, Mar 03, 2002 at 08:47:17AM -0800, Ryan Bloom wrote:
> My list insertion code was wrong, which was causing this bug. Also,
> this patch is required even without the rest of the
> mod_dir/mod_negotiation patch. We have a bug, it allows a connection
> filter to be added and never called. Try this:
>
> Crete a request
> Add connection filters
> Add request filters
> Add connection filters
>
> The second set won't be added. I am fixing this now, it was a simple
> mistake of where the if was done.
Thanks for figuring out what is wrong.
> Because your suggestion makes it even harder impossible to write
> modules. Module authors have already told us which filters should
> continue through redirects and sub-requests. They did that through
> using a different filter type. Why should they then have to repeat that
> information in a function?
>
> Hooks are not the solution to every problem. We already have people
> complaining that there are so many hooks that they don't really know
> what is possible with the server, nor do they know what they have to do
> to make their modules work. Why would we add more hooks at this point
> when there are other cleaner solutions?
I think you may have missed my message yesterday. It's not adding a
hook - it's allowing a hook to be able to abort processing of a
request. That's all. internal_redirect will take care of
everything. fast_redirect is bogus because it skips hooks in the
new request. I believe that is broken behavior. If we were to
rely on internal_redirect, everything falls into place - without
having to add all of this protocol_filters code that unnecessarily
complicate matters.
And, the hook that I was asking about is already there -
insert_filters. (I just didn't realize it was there.) I believe
there is no justification for adding protocol-level filters - as
they merely hide the problem rather than fixing it. If you
wish to add it, they should be added with a proto_rec as
OtherBill suggested. And, if you add that, I ask for you to
open 2.1. -- justin
RE: cvs commit: httpd-2.0/server util_filter.c
Posted by Ryan Bloom <rb...@covalent.net>.
My list insertion code was wrong, which was causing this bug. Also,
this patch is required even without the rest of the
mod_dir/mod_negotiation patch. We have a bug, it allows a connection
filter to be added and never called. Try this:
Crete a request
Add connection filters
Add request filters
Add connection filters
The second set won't be added. I am fixing this now, it was a simple
mistake of where the if was done.
> I still fail to see why you think protocol filters would solve
> this problem. I've explained my reasoning behind why I think this
> isn't a good idea. I've also pointed out a suggestion that I
> believe would work that is rather simple and would increase our
> flexibility. I'd like to hear your reasons why my approach isn't
> feasible.
Because your suggestion makes it even harder impossible to write
modules. Module authors have already told us which filters should
continue through redirects and sub-requests. They did that through
using a different filter type. Why should they then have to repeat that
information in a function?
Hooks are not the solution to every problem. We already have people
complaining that there are so many hooks that they don't really know
what is possible with the server, nor do they know what they have to do
to make their modules work. Why would we add more hooks at this point
when there are other cleaner solutions?
However, I have been tracing this patch. The problem is that we are
using a simple list, but we don't actually have a simple list. We have
multiple lists converging in one place. The subrequests are breaking
the filter list now, because I have converted this to a doubly linked
list. I will be working on fixing this ASAP, but it may a few hours to
fix.
This has VERY little to do with the mod_dir/mod_negotiation bug though.
That fix was put committed in the commit before this one. This patch
fixes a major problem with our filter lists, and we need to have this
fixed.
Ryan