Re: [PATCH] RewriteCond and SSL environment variables
On Thu, Mar 04, 2004 at 07:41:54AM +0100, André Malo wrote:
I'd prefer the %{SSL:...} variant and using ssl_var_lookup_ssl. All other
Agreed. It makes sense to me to make a specific point out of where those
variables came from.
vh
Mads Toftum
--
`Darn it, who spiked my coffee with water?!' - lwall
Re: [PATCH] RewriteCond and SSL environment variables
On Thu, Mar 04, 2004 at 07:41:54AM +0100, André Malo wrote:
* Mathihalli, Madhusudan [EMAIL PROTECTED] wrote:
Here's a slightly modified version of Joe's patch to
- not segfault if rewrite_ssl_var_lookup is not available (mod_ssl not
loaded)- use SSL environment variables as %{ENV:HTTPS} or
%{ENV:SSL_PROTOCOL}
I'd prefer the %{SSL:...} variant and using ssl_var_lookup_ssl. All other
stuff is already available in mod_rewrite. All the logic in ssl_var_lookup
is just not necessary for this purpose.
I'm not really convinced about using ssl_var_lookup_ssl: that function
does not handle the HTTPS variable, and it would be potentially
confusing to users and hard to document since only some subset of the
SSL variables could be used. (it would also need a new optional
function in mod_ssl)
I'll commit the original patch with Madhu's fix to check
rewrite_ssl_lookup unless there are any strong objections.
joe
Re: [PATCH] RewriteCond and SSL environment variables
* Joe Orton [EMAIL PROTECTED] wrote:
I'm not really convinced about using ssl_var_lookup_ssl: that function
does not handle the HTTPS variable, and it would be potentially
confusing to users and hard to document since only some subset of the
SSL variables could be used. (it would also need a new optional
function in mod_ssl)
I'll commit the original patch with Madhu's fix to check
rewrite_ssl_lookup unless there are any strong objections.
Then it's time to change it. The ssl_var_lookup function is totally messy
and _slow_.
We want SSL variables, so let's only grab them.
I'm -1 (vote not veto) on using the generic ssl_var_lookup function. Maybe
we should put the HTTPS check into an own function (we could use %{HTTPS} in
mod_rewrite then). That way, other modules, that want to check (only) HTTPS,
also don't need to run though all the mess of ssl_var_lookup.
nd
Re: [PATCH] RewriteCond and SSL environment variables
On Thu, Mar 04, 2004 at 11:08:25AM +0100, André Malo wrote:
* Joe Orton [EMAIL PROTECTED] wrote:
I'm not really convinced about using ssl_var_lookup_ssl: that function
does not handle the HTTPS variable, and it would be potentially
confusing to users and hard to document since only some subset of the
SSL variables could be used. (it would also need a new optional
function in mod_ssl)
I'll commit the original patch with Madhu's fix to check
rewrite_ssl_lookup unless there are any strong objections.
Then it's time to change it. The ssl_var_lookup function is totally messy
and _slow_.
We want SSL variables, so let's only grab them.
I'm -1 (vote not veto) on using the generic ssl_var_lookup function. Maybe
we should put the HTTPS check into an own function (we could use %{HTTPS} in
mod_rewrite then). That way, other modules, that want to check (only) HTTPS,
also don't need to run though all the mess of ssl_var_lookup.
That sounds like a great idea. I still think that %{SSL:..} should use
ssl_var_lookup for consistency; sure, the function is messy and does
lots of nasty strcasecmp's but I'd really think that the overhead is
lost in the noise for an SSL connection. It's easy to optimise a few
out anyway...
Index: modules/mappers/mod_rewrite.c
===
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.252
diff -u -r1.252 mod_rewrite.c
--- modules/mappers/mod_rewrite.c 9 Feb 2004 20:29:20 - 1.252
+++ modules/mappers/mod_rewrite.c 4 Mar 2004 12:27:41 -
@@ -89,6 +89,8 @@
#include http_protocol.h
#include http_vhost.h
+#include mod_ssl.h
+
#include mod_rewrite.h
#if !defined(OS2) !defined(WIN32) !defined(BEOS) !defined(NETWARE)
@@ -380,6 +382,9 @@
static apr_global_mutex_t *rewrite_log_lock = NULL;
#endif
+/* Optional functions imported from mod_ssl when loaded: */
+static APR_OPTIONAL_FN_TYPE(ssl_var_lookup) *rewrite_ssl_lookup = NULL;
+static APR_OPTIONAL_FN_TYPE(ssl_is_https) *rewrite_is_https = NULL;
/*
* +---+
@@ -1610,6 +1615,10 @@
result = getenv(var);
}
}
+else if (var[4] !strncasecmp(var, SSL, 3) rewrite_ssl_lookup) {
+result = rewrite_ssl_lookup(r-pool, r-server, r-connection, r,
+var + 4);
+}
}
else if (var[4] == ':') {
if (var[5]) {
@@ -1693,6 +1702,13 @@
return (char *)result;
}
break;
+
+case 5:
+if (!strcmp(var, HTTPS)) {
+int flag = rewrite_is_https rewrite_is_https(r-connection);
+return apr_pstrdup(r-pool, flag ? on : off);
+}
+break;
case 8:
switch (var[6]) {
@@ -3995,6 +4011,9 @@
}
}
}
+
+rewrite_ssl_lookup = APR_RETRIEVE_OPTIONAL_FN(ssl_var_lookup);
+rewrite_is_https = APR_RETRIEVE_OPTIONAL_FN(ssl_is_https);
return OK;
}
Re: [PATCH] RewriteCond and SSL environment variables
Maybe
we should put the HTTPS check into an own function (we could use %{HTTPS} in
mod_rewrite then). That way, other modules, that want to check (only) HTTPS,
also don't need to run though all the mess of ssl_var_lookup.
I'm not familiar with mod_ssl internals, but is there any reason we can't
move subprocess_env population to something early like post-read-request?
as a per-connection thingy, HTTPS ought to be know before the request cycle
begins, right? maybe not for other things, though...
anyway, this same thing came up recently with mod_env
http://marc.theaimsgroup.com/?l=apache-httpd-devm=107655981117848w=2
in that case I did trace the code and couldn't see any reason that the
population couldn't appear sooner, other than perhaps the history of
subprocess_env (but I haven't been around _that_ long :)
--Geoff
RE: [PATCH] RewriteCond and SSL environment variables
-Original Message- From: Geoffrey Young [mailto:[EMAIL PROTECTED] [SNIP] I'm not familiar with mod_ssl internals, but is there any reason we can't move subprocess_env population to something early like post-read-request? as a per-connection thingy, HTTPS ought to be know before the request cycle begins, right? maybe not for other things, though... +1. I don't see any harm in making the HTTPS available in the subprocess_env soon after read_request. (Infact, I think we *should* do it that way) -Madhu
[PATCH] RewriteCond and SSL environment variables
Here's a slightly modified version of Joe's patch to
- not segfault if rewrite_ssl_var_lookup is not available (mod_ssl not loaded)
- use SSL environment variables as %{ENV:HTTPS} or %{ENV:SSL_PROTOCOL}
I tested the patch with the following rules, and it appeared to work without causing
any problems.
RewriteCond %{ENV:HTTPS} =on
RewriteCond %{ENV:SSL_CIPHER_USEKEYSIZE} =128
RewriteRule ^/manual.*html$ /manual.html [L]
RewriteRule ^.*$ - [F]
-Madhu
Index: mod_rewrite.c
===
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.252
diff -u -r1.252 mod_rewrite.c
--- mod_rewrite.c 9 Feb 2004 20:29:20 - 1.252
+++ mod_rewrite.c 3 Mar 2004 22:59:07 -
@@ -380,6 +380,13 @@
static apr_global_mutex_t *rewrite_log_lock = NULL;
#endif
+/* support for ssl_var_lookup() */
+APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup,
+(apr_pool_t *, server_rec *,
+ conn_rec *, request_rec *,
+ char *));
+static APR_OPTIONAL_FN_TYPE(ssl_var_lookup) *rewrite_ssl_var_lookup = NULL;
+
/*
* +---+
@@ -1601,7 +1608,15 @@
if (var[3] == ':') {
if (var[4] !strncasecmp(var, ENV, 3)) {
var += 4;
-result = apr_table_get(r-notes, var);
+if (!strncasecmp(var, SSL, 3) || !strncasecmp(var, HTTPS, 5)) {
+result = ((rewrite_ssl_var_lookup == NULL) || (r == NULL))
+ ? (char *)NULL
+ : rewrite_ssl_var_lookup(r-pool, r-server,
+ r-connection, r, var);
+}
+else {
+result = apr_table_get(r-notes, var);
+}
if (!result) {
result = apr_table_get(r-subprocess_env, var);
@@ -3995,6 +4010,8 @@
}
}
}
+
+rewrite_ssl_var_lookup = APR_RETRIEVE_OPTIONAL_FN(ssl_var_lookup);
return OK;
}
Re: [PATCH] RewriteCond and SSL environment variables
Mathihalli, Madhusudan wrote:
Here's a slightly modified version of Joe's patch to
- not segfault if rewrite_ssl_var_lookup is not available (mod_ssl not loaded)
- use SSL environment variables as %{ENV:HTTPS} or %{ENV:SSL_PROTOCOL}
I tested the patch with the following rules, and it appeared to work without causing
any problems.
RewriteCond %{ENV:HTTPS} =on
RewriteCond %{ENV:SSL_CIPHER_USEKEYSIZE} =128
RewriteRule ^/manual.*html$ /manual.html [L]
RewriteRule ^.*$ - [F]
looks like a perfect candidate for adding new tests to the perl-framework
;)
--Geoff
Re: [PATCH] RewriteCond and SSL environment variables
--On Wednesday, March 3, 2004 3:04 PM -0800 Mathihalli, Madhusudan [EMAIL PROTECTED] wrote: +/* support for ssl_var_lookup() */ +APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup, +(apr_pool_t *, server_rec *, + conn_rec *, request_rec *, + char *)); +static APR_OPTIONAL_FN_TYPE(ssl_var_lookup) *rewrite_ssl_var_lookup = NULL; + Shouldn't you include mod_ssl.h instead? -- justin
RE: [PATCH] RewriteCond and SSL environment variables
-Original Message- From: Justin Erenkrantz [mailto:[EMAIL PROTECTED] [SNIP] --On Wednesday, March 3, 2004 3:04 PM -0800 Mathihalli, Madhusudan [EMAIL PROTECTED] wrote: +/* support for ssl_var_lookup() */ +APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup, +(apr_pool_t *, server_rec *, + conn_rec *, request_rec *, + char *)); +static APR_OPTIONAL_FN_TYPE(ssl_var_lookup) *rewrite_ssl_var_lookup = NULL; + Shouldn't you include mod_ssl.h instead? -- justin I thought it'll be a little too much :) What if I don't compile with mod_ssl - I'll have a whole bunch of declarations with no definitions associated with it. Besides, mod_proxy.c follows the same logic. -Madhu
RE: [PATCH] RewriteCond and SSL environment variables
--On Wednesday, March 3, 2004 4:24 PM -0800 Mathihalli, Madhusudan [EMAIL PROTECTED] wrote: I thought it'll be a little too much :) What if I don't compile with mod_ssl - I'll have a whole bunch of declarations with no definitions associated with it. Besides, mod_proxy.c follows the same logic. No, Joe just re-did mod_proxy.c and mod_ssl.h to stop that insanity. Run 'cvs up' against HEAD. ;-) -- justin
RE: [PATCH] RewriteCond and SSL environment variables
-Original Message- From: Justin Erenkrantz [mailto:[EMAIL PROTECTED] [SNIP] --On Wednesday, March 3, 2004 4:24 PM -0800 Mathihalli, Madhusudan [EMAIL PROTECTED] wrote: I thought it'll be a little too much :) What if I don't compile with mod_ssl - I'll have a whole bunch of declarations with no definitions associated with it. Besides, mod_proxy.c follows the same logic. No, Joe just re-did mod_proxy.c and mod_ssl.h to stop that insanity. Run 'cvs up' against HEAD. ;-) -- justin Gosh.. don't know how I missed it - thanks ! Anyways with the new mod_ssl.h - yup, we can do with including mod_ssl.h. -Madhu
Re: [PATCH] RewriteCond and SSL environment variables
* Mathihalli, Madhusudan [EMAIL PROTECTED] wrote:
Here's a slightly modified version of Joe's patch to
- not segfault if rewrite_ssl_var_lookup is not available (mod_ssl not
loaded)- use SSL environment variables as %{ENV:HTTPS} or
%{ENV:SSL_PROTOCOL}
I'd prefer the %{SSL:...} variant and using ssl_var_lookup_ssl. All other
stuff is already available in mod_rewrite. All the logic in ssl_var_lookup
is just not necessary for this purpose.
nd
