Re: [RELEASE CANDIDATE] Apache-Test-1.12
On Jun 25, 2004, at 4:49 PM, David Wheeler wrote:
Hrm, nope. I just installed the latest on another box and the
redirection still works properly. Odd...
So let's introduce some tests to see where it happens, hrm??
diff -Naur old/t/conf/extra.conf.in new/t/conf/extra.conf.in
--- old/t/conf/extra.conf.inTue Sep 30 14:08:16 2003
+++ new/t/conf/extra.conf.inFri Jun 25 19:26:16 2004
@@ -1,4 +1,6 @@
#this file will be Include-d by @ServerRoot@/httpd.conf
#the subclass inside t/TEST added the authname and allowed_users
variables
-
+IfModule mod_alias.c
+ Redirect /redirect http://@ServerName@/redirected/
+/IfModule
diff -Naur old/t/redirect.t new/t/redirect.t
--- old/t/redirect.t Wed Dec 31 17:00:00 1969
+++ new/t/redirect.t Fri Jun 25 19:34:41 2004
@@ -0,0 +1,23 @@
+use strict;
+use warnings FATAL = 'all';
+
+use Apache::Test;
+use Apache::TestRequest;
+
+plan tests = 6, have_module('mod_alias.c') have_lwp;
+
+my $url = '/redirect';
+
+# Allow request to be redirected.
+ok my $res = GET $url;
+ok ! $res-is_redirect;
+
+# Don't let request be redirected.
+ok $res = GET($url, redirect_ok = 0);
+ok $res-is_redirect;
+
+# Allow no more requests to be redirected.
+Apache::TestRequest::user_agent(reset = 1,
+requests_redirectable = 0);
+ok $res = GET $url;
+ok $res-is_redirect;
Regards,
David
PATCH: various mod_proxy issues
I've rolled a fairly extensive mod_proxy patch, which seems rather
big to commit without review. Comments please:
(1) Bug #10722 - cookie paths and domains in reverse proxy
Following my patch to 2.0.49, I've adapted it for 2.1, taking into account
Jim's patch. In doing so, I made some organisational changes:
* moved rewriting of headers that need it (ap_proxy_date_canon and
ap_proxy_location_reverse_map) to a new function process_proxy_header
called from ap_proxy_read_headers.
* Removed the same from ap_proxy_http_process_response
* moved ap_proxy_read_headers from proxy_utils to proxy_http
* Retained Jim's patch, but removed the line merging err_headers_out
(2) Bug #29554 - URL munging
I've ported Francois-Rene Rideau's patch to 2.1, subject to the
question over proxy_fixup discussed in my last post.
Any problems with committing this?
--
Nick Kewdiff -u proxy-old/mod_proxy.c proxy/mod_proxy.c
--- proxy-old/mod_proxy.c 2004-06-26 07:18:10.0 +0100
+++ proxy/mod_proxy.c 2004-06-26 07:13:46.0 +0100
@@ -94,9 +94,10 @@
static int proxy_detect(request_rec *r)
{
void *sconf = r-server-module_config;
-proxy_server_conf *conf;
-
-conf = (proxy_server_conf *) ap_get_module_config(sconf, proxy_module);
+proxy_server_conf *conf =
+ (proxy_server_conf *) ap_get_module_config(sconf, proxy_module);
+int i, len;
+struct proxy_alias *ent = (struct proxy_alias *)conf-aliases-elts;
/* Ick... msvc (perhaps others) promotes ternary short results to int */
@@ -121,6 +122,19 @@
r-uri = r-unparsed_uri;
r-filename = apr_pstrcat(r-pool, proxy:, r-uri, NULL);
r-handler = proxy-server;
+} else {
+/* test for a ProxyPass */
+for (i = 0; i conf-aliases-nelts; i++) {
+len = alias_match(r-unparsed_uri, ent[i].fake);
+if (len 0) {
+r-filename = apr_pstrcat(r-pool, proxy:, ent[i].real,
+ r-unparsed_uri + len, NULL);
+r-handler = proxy-server;
+r-proxyreq = PROXYREQ_REVERSE;
+r-uri = r-unparsed_uri;
+break;
+}
+}
}
return DECLINED;
}
@@ -140,26 +154,6 @@
return OK;
}
-/* XXX: since r-uri has been manipulated already we're not really
- * compliant with RFC1945 at this point. But this probably isn't
- * an issue because this is a hybrid proxy/origin server.
- */
-
-for (i = 0; i conf-aliases-nelts; i++) {
-len = alias_match(r-uri, ent[i].fake);
-
- if (len 0) {
- if ((ent[i].real[0] == '!' ) ( ent[i].real[1] == 0 )) {
- return DECLINED;
- }
-
- r-filename = apr_pstrcat(r-pool, proxy:, ent[i].real,
- (r-uri + len ), NULL);
- r-handler = proxy-server;
- r-proxyreq = PROXYREQ_REVERSE;
- return OK;
- }
-}
return DECLINED;
}
@@ -221,7 +215,7 @@
return OK;
}
-
+#if 0
/* -- */
/* Fixup the filename */
@@ -236,6 +230,13 @@
if (!r-proxyreq || !r-filename || strncmp(r-filename, proxy:, 6) != 0)
return DECLINED;
+/* We definitely shouldn't canonicalize a proxy_pass.
+ * But should we really canonicalize a STD_PROXY??? -- Fahree
+ */
+if (r-proxyreq == PROXYREQ_REVERSE) {
+return OK;
+}
+
/* XXX: Shouldn't we try this before we run the proxy_walk? */
url = r-filename[6];
@@ -250,7 +251,7 @@
return OK; /* otherwise; we've done the best we can */
}
-
+#endif
/* Send a redirection if the request contains a hostname which is not */
/* fully qualified, i.e. doesn't have a domain name appended. Some proxy */
/* servers like Netscape's allow this and access hosts from the local */
@@ -439,6 +440,10 @@
ps-proxies = apr_array_make(p, 10, sizeof(struct proxy_remote));
ps-aliases = apr_array_make(p, 10, sizeof(struct proxy_alias));
ps-raliases = apr_array_make(p, 10, sizeof(struct proxy_alias));
+ps-cookie_paths = apr_array_make(p, 10, sizeof(struct proxy_alias));
+ps-cookie_domains = apr_array_make(p, 10, sizeof(struct proxy_alias));
+ps-cookie_path_str = apr_strmatch_precompile(p, path=, 0) ;
+ps-cookie_domain_str = apr_strmatch_precompile(p, domain=, 0) ;
ps-noproxies = apr_array_make(p, 10, sizeof(struct noproxy_entry));
ps-dirconn = apr_array_make(p, 10, sizeof(struct dirconn_entry));
ps-allowed_connect_ports = apr_array_make(p, 10, sizeof(int));
@@ -474,6 +479,12 @@
ps-sec_proxy = apr_array_append(p, base-sec_proxy, overrides-sec_proxy);
ps-aliases = apr_array_append(p, base-aliases, overrides-aliases);
ps-raliases = apr_array_append(p, base-raliases, overrides-raliases);
+ps-cookie_paths
+= apr_array_append(p, base-cookie_paths, overrides-cookie_paths);
+ps-cookie_domains
+
Re: URI lossage with ProxyPass
On Sat, 26 Jun 2004, Nick Kew wrote: On Thu, 17 Jun 2004, Francois-Rene Rideau wrote: I've reviewed this in the context of httpd-2.1, and it looks good to me with essentially the same patch. It works on your testcase, and I'm 99% satisfied that it doesn't break anything. Ready to commit if we +1 here too - we had something ourselfs which did this in a dirty/specific ase; and this patch makes that unessesary and is more general. Dw.
Flood?
Hi, I notice that the last release of the flood utility was in 2002. I had to make some trivial changes to get it to compile with my installation of apache 2.0.49 (include apr_poll.h, add an argument to the call to apr_poll). Now, it compiles but segfaults immediately, even with the example scripts. Is someone still maintaining this utility? Is there an interest in getting it working again, or giving it an overhaul? Or are their other tools for profile oriented benchmarking that are used as alternatives? I'd be happy to audit the code, if necessary. regards, lex
Re: PATCH: various mod_proxy issues
Nick Kew wrote: (2) Bug #29554 - URL munging I've ported Francois-Rene Rideau's patch to 2.1, subject to the question over proxy_fixup discussed in my last post. Looking at this issue, I think the real solution is to a) canonicalise all URLS (reverse and forward) for internal Apache use. b) use the original untouched URL to pass to the backend. I am uncomfortable with the distinction between forward and reverse proxy - I think this patch might fix this bug for the forward case, but not for the reverse case, which doesn't ultimately fix the problem. Any problems with committing this? If it's possible to separate the patch out into two different patches - one for each bug, it will make things easier to review. The cookie issue has been around for a while, it would be good to see a patch in v2.1 (and ultimately v2.0) - I don't want to see it held up by the canonicalisation issue. Regards, Graham --
Re: URI lossage with ProxyPass
Nick Kew wrote: Can anyone see why proxy_fixup should not be removed altogether? Proxy fixup seems to do the job of making sure the URL /%41%42%43 matches ProxyPass /ABC http://xxx/ABC;, so I don't think it should be removed altogether. The URL that is sent to the backend server however has no business being touched at all en route - a copy of the untouched URL should be kept alongside the canonicalised URL, and the untouched URL should be sent to the backend server just as the client sent it. Regards, Graham --
Re: cvs commit: httpd-2.0/modules/experimental util_ldap.c util_ldap_cache.c util_ldap_cache.h
[EMAIL PROTECTED] wrote:
+#if APR_HAS_SHARED_MEMORY
+/* If the cache file already exists then delete it. Otherwise we are
+ * going to run into problems creating the shared memory. */
+apr_file_remove(st-cache_file, ptemp);
+if (st-cache_file) {
+char *lck_file = apr_pstrcat (st-pool, st-cache_file, .lck, NULL);
+apr_file_remove(lck_file, ptemp);
+}
+#endif
Does this patch support the idea of the cache file being NULL (ie
anonymous shared memory?).
The previous code insisted on specifying a cache file, which didn't work
properly under Linux. Now, not specifying a cache filename means use
anonymous shared memory, I'd just like to check that this NULL filename
is case is still handled properly.
Regards,
Graham
--
Re: cvs commit: httpd-2.0/modules/experimental util_ldap.c util_ldap_cache.c util_ldap_cache.h
No, I didn't change anything that would allow for anonymous shared
memory. This should probably check for a NULL before calling
apr_file_remove().
Brad
Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com
[EMAIL PROTECTED] Saturday, June 26, 2004 9:30:42 AM
[EMAIL PROTECTED] wrote:
+#if APR_HAS_SHARED_MEMORY
+/* If the cache file already exists then delete it.
Otherwise we are
+ * going to run into problems creating the shared memory.
*/
+apr_file_remove(st-cache_file, ptemp);
+if (st-cache_file) {
+char *lck_file = apr_pstrcat (st-pool,
st-cache_file, .lck, NULL);
+apr_file_remove(lck_file, ptemp);
+}
+#endif
Does this patch support the idea of the cache file being NULL (ie
anonymous shared memory?).
The previous code insisted on specifying a cache file, which didn't
work
properly under Linux. Now, not specifying a cache filename means use
anonymous shared memory, I'd just like to check that this NULL
filename
is case is still handled properly.
Regards,
Graham
--
Re: cvs commit: httpd-2.0/modules/experimental util_ldap.c util_ldap_cache.c util_ldap_cache.h
Brad Nicholes wrote: No, I didn't change anything that would allow for anonymous shared memory. This should probably check for a NULL before calling apr_file_remove(). +apr_file_remove(st-cache_file, ptemp); Will this line segfault if ptempt is NULL? If not, then it should be fine. If so, we should probably test for NULLness first (as you recommend above). Regards, Graham --
Re: URI lossage with ProxyPass
On Sat, 26 Jun 2004, Graham Leggett wrote: Nick Kew wrote: Can anyone see why proxy_fixup should not be removed altogether? Proxy fixup seems to do the job of making sure the URL /%41%42%43 matches ProxyPass /ABC http://xxx/ABC;, so I don't think it should be removed altogether. I don't think that's right. Both proxy_detect and proxy_trans happen before proxy_fixup, and the comment in proxy_fixup refers to its relationship with mod_rewrite. The patched apache fails that test, but simply reinstating proxy_fixup makes no difference to that. Now I'm confused. I think you're right in your other post: separate patches for separate bugs. And not necessarily at 4 a.m. But having come this far, I want to see both fixed:-) And a trawl of bugzilla tells me that the URI Lossage is bug #15207 and probably others, while bug #16812 is a trivial corollary to the cookie patch. -- Nick Kew
Re: cvs commit: httpd-2.0/modules/experimental util_ldap.c util_ldap_cache.c util_ldap_cache.h
ptemp shouldn't ever be NULL on a post_config, right? I just fixed the code so that it checks for a NULL file name before calling apr_file_remove(). Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com [EMAIL PROTECTED] Saturday, June 26, 2004 10:01:07 AM Brad Nicholes wrote: No, I didn't change anything that would allow for anonymous shared memory. This should probably check for a NULL before calling apr_file_remove(). +apr_file_remove(st-cache_file, ptemp); Will this line segfault if ptempt is NULL? If not, then it should be fine. If so, we should probably test for NULLness first (as you recommend above). Regards, Graham --
Re: Proxy Cookie Support (Bug #10722)
On Fri, 25 Jun 2004, Nick Kew wrote: OK, that's what I needed to know. I'll still have to modify my patch slightly to work with yours, but at least it's now clear what's going on. DO make sure that it is still possible for, say, mod_perl or mod_jk to remove/kill a coppy with an expired Set-Cookie line on an error; as some session mechanisms rely on that. Dw.
Re: URI lossage with ProxyPass
On Sat, 26 Jun 2004, Graham Leggett wrote: Nick Kew wrote: Can anyone see why proxy_fixup should not be removed altogether? Proxy fixup seems to do the job of making sure the URL /%41%42%43 matches ProxyPass /ABC http://xxx/ABC;, so I don't think it should be removed altogether. OK, the reason for that is that the patch moved ProxyPass-ing from proxy_trans to proxy_detect. The latter happens before canonicalisation, which is both why the patch works and why it breaks the above. A fix is for alias_match() to recognise %xx sequences. I've now implemented it, but also separated out the URI-trouble stuff with #ifdef FIX_15207 on the grounds that it's still subject to debate. That still leaves us a proxy_fixup with no purpose I can see. Perhaps someone who uses it with mod_rewrite can say if it does anything for you? -- Nick Kew
