You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/07/18 09:17:57 UTC
IE4 PR2 hopelessness
The keepalive changes in the patch I posted yesterday are total crap.
The force-response-1.0 part works, and is necessary because without it
we'll do things like Transfer-Encoding: chunked, but send HTTP/1.0.
Here are the two pr2 bugs I know of:
1 seems to handle keep-alive only on 200 responses, all others need to be
closed by the server before the client will continue
2 the Java VM makes HTTP/1.1 requests but does not understand HTTP/1.1
responses, in particular it does not understand a chunked response.
See PR#875, the user has a CGI which sends a response to a java applet.
Naturally the response is chunked in 1.1.
Now here comes the fun. Problem 1 is really painful on redirects.
Redirects are generated during translate_name(). BrowserMatch is done
during header_parse -- which occurs *after* translate_name. Hence
set_keepalive does not have any nokeepalive variable to test, and it
happily follows 1.1 and does keep-alive.
So I ask myself, "why does header_parse come *after* a handful of other
phases?" The obvious answer is that if it came before then you couldn't
have per_dir modifications to header_parse routines. But the
header_parser was added specifically so that we could use mod_browser to
kludge around screwed up clients... well, we can't use it to work around
this screwed up client.
My suggestion for now: make mod_browser use translate_name instead of
header_parse. A cleaner solution is to add yet another api phase.
Note that this means mod_browser is going to run during a
sub_req_lookup_uri(), but I don't think this is a problem (and using
something like is_initial_request does not work, see next message).
Either way, we end up with a nokeepalive env var when we need it. Then we
need to do the Right Thing with it in set_keepalive. I think what I do
in the patch below is the Right Thing, I'm sure Roy will disagree :)
I have no solution (that I'm happy with) for problem 2. The user-agent
is the same whether the browser is making a regular or a java request. So
what I do below is a complete hack -- the env var "downgrade-request-1.0"
causes the server to pretend it got a 1.0 request.
So, the patch below, plus this:
BrowserMatch "MSIE 4.0b2;" force-response-1.0 downgrade-request-1.0 nokeepalive
*SHOULD* solve these two problems.
Dean
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.139
diff -u -r1.139 http_protocol.c
--- http_protocol.c 1997/07/15 22:36:51 1.139
+++ http_protocol.c 1997/07/18 07:16:30
@@ -281,8 +281,9 @@
* and the response status does not require a close;
* and the response generator has not already indicated close;
* and the client did not request non-persistence (Connection: close);
+ * and we haven't been configured to ignore the buggy twit
+ * or they're a buggy twit coming through a HTTP/1.1 proxy
* and the client is requesting an HTTP/1.0-style keep-alive
- * and we haven't been configured to ignore the buggy twit,
* or the client claims to be HTTP/1.1 compliant (perhaps a proxy);
* THEN we can be persistent, which requires more headers be output.
*
@@ -304,9 +305,10 @@
!status_drops_connection(r->status) &&
!wimpy &&
!find_token(r->pool, conn, "close") &&
- (((ka_sent = find_token(r->pool, conn, "keep-alive")) &&
- !table_get(r->subprocess_env, "nokeepalive")) ||
- (r->proto_num >= 1001))
+ (!table_get(r->subprocess_env, "nokeepalive") ||
+ table_get(r->headers_in, "Via")) &&
+ ((ka_sent = find_token(r->pool, conn, "keep-alive")) ||
+ (r->proto_num >= 1001))
) {
char header[256];
int left = r->server->keep_alive_max - r->connection->keepalives;
@@ -1044,8 +1046,9 @@
if (!r->status_line)
r->status_line = status_lines[index_of_response(r->status)];
-
- if (table_get(r->subprocess_env,"force-response-1.0"))
+
+ if (r->proto_num == 1000
+ && table_get(r->subprocess_env,"force-response-1.0"))
protocol = "HTTP/1.0";
else
protocol = SERVER_PROTOCOL;
Index: http_request.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_request.c,v
retrieving revision 1.62
diff -u -r1.62 http_request.c
--- http_request.c 1997/07/15 21:39:57 1.62
+++ http_request.c 1997/07/18 07:16:31
@@ -947,6 +947,10 @@
return;
}
+ if (table_get (r->subprocess_env, "downgrade-request-1.0")) {
+ r->proto_num = 1000;
+ }
+
/* NB: directory_walk() clears the per_dir_config, so we don't inherit from
location_walk() above */
Index: mod_browser.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_browser.c,v
retrieving revision 1.11
diff -u -r1.11 mod_browser.c
--- mod_browser.c 1997/07/17 22:27:34 1.11
+++ mod_browser.c 1997/07/18 07:16:31
@@ -166,7 +166,7 @@
}
}
- return OK;
+ return DECLINED;
}
module MODULE_VAR_EXPORT browser_module = {
@@ -178,13 +178,13 @@
merge_browser_config, /* merge server configs */
browser_module_cmds, /* command table */
NULL, /* handlers */
- NULL, /* filename translation */
+ parse_headers_browser_module,/* filename translation */
NULL, /* check_user_id */
NULL, /* check auth */
NULL, /* check access */
NULL, /* type_checker */
NULL, /* fixups */
NULL, /* logger */
- parse_headers_browser_module,/* header parser */
+ NULL, /* header parser */
NULL /* child_init */
};
Re: IE4 PR2 hopelessness
Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 18 Jul 1997, Dean Gaudet wrote:
> My suggestion for now: make mod_browser use translate_name instead of
> header_parse. A cleaner solution is to add yet another api phase.
> Note that this means mod_browser is going to run during a
> sub_req_lookup_uri(), but I don't think this is a problem (and using
> something like is_initial_request does not work, see next message).
Ok, this is the next message. Trying to work around these pr2 bugs lead
me into areas of the server that I only think about in my nightmares.
I innocently added:
if (r->prev || r->main) {
/* subrequest, we've already dealt with the environment */
return DECLINED;
}
to the mod_browser routine. In theory this would avoid mod_browser
from running on every subrequest... but in practice it fails to do what
we want.
Consider how mod_negotiation does its thing: internal_redirect.
An internal_redirect is *not* a subrequest in that it does not copy/merge
things from the main request the same way that subrequests do. With the
above test the internal_redirect request would not have the env vars set.
You can't just test r->main either 'cause internal_redirects have
r->main set to the initial request. r->prev != NULL indicates that r
is an internal_redirect.
At any rate ... while I'm going through this I'm thinking about the extra
API phase that I'd want to clean up mod_browser. That's when I start
to realise that to do this all properly, the various tests of env-vars
need to do something like
table_get((r->main ? r->main : r)->subprocess_env, "enable-kludge")
Rather than just using r->subprocess_env.
And the reason this all bugs me is that none of it seems clean ... and
that's probably because the rules for generating a subrequest from a
request or generating a redirect from a request aren't very well defined.
Consider all the dinking around in subrequests that goes on in
mod_negotiation, and mod_include. Is it valid? Is that part of the API?
Look at how many little bugs we fixed in mod_negotiation during the
1.2 betas.
Bleh.
Dean