You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2006/05/17 21:16:43 UTC
svn commit: r407357 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c
Author: rpluem
Date: Wed May 17 12:16:43 2006
New Revision: 407357
URL: http://svn.apache.org/viewvc?rev=407357&view=rev
Log:
* Handle the cases "no proxy request" and "reverse proxy request" in the same
manner, when setting scheme and port_str. This is needed because if a cached
entry is looked up by mod_cache's quick handler r->proxyreq
is still unset in the reverse proxy case as it only gets set in the
translate name hook (either by ProxyPass or mod_rewrite) which is run
after the quick handler hook. This is different to the forward proxy
case where it gets set before the quick handler is run (in the
post_read_request hook).
If a cache entry is created by the CACHE_SAVE filter we always have
r->proxyreq set correctly.
Also set scheme to ap_http_scheme(r) instead of "http" to handle SSL
correctly.
PR: 39593
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/cache/cache_storage.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=407357&r1=407356&r2=407357&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed May 17 12:16:43 2006
@@ -2,6 +2,9 @@
Changes with Apache 2.3.0
[Remove entries to the current 2.0 and 2.2 section below, when backported]
+ *) mod_cache: Make caching of reverse SSL proxies possible again. PR 39593.
+ [Ruediger Pluem]
+
*) Add support for fcgi:// proxies to mod_rewrite.
[Markus Schiegl <ms schiegl.com>]
Modified: httpd/httpd/trunk/modules/cache/cache_storage.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.c?rev=407357&r1=407356&r2=407357&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_storage.c Wed May 17 12:16:43 2006
@@ -364,8 +364,13 @@
hostname = "_default_";
}
- /* Copy the scheme, ensuring that it is lower case. If the parsed uri
- * contains no string or if this is not a proxy request.
+ /*
+ * Copy the scheme, ensuring that it is lower case. If the parsed uri
+ * contains no string or if this is not a proxy request get the http
+ * scheme for this request. As r->parsed_uri.scheme is not set if this
+ * is a reverse proxy request, it is ensured that the cases
+ * "no proxy request" and "reverse proxy request" are handled in the same
+ * manner (see above why this is needed).
*/
if (r->proxyreq && r->parsed_uri.scheme) {
/* Copy the scheme */
@@ -375,15 +380,18 @@
}
}
else {
- scheme = "http";
+ scheme = ap_http_scheme(r);
}
- /* If the content is locally generated, use the port-number of the
- * current server. Otherwise. copy the URI's port-string (which may be a
- * service name). If the URI contains no port-string, use apr-util's
- * notion of the default port for that scheme - if available.
+ /*
+ * If this is a proxy request, but not a reverse proxy request (see comment
+ * above why these cases must be handled in the same manner), copy the
+ * URI's port-string (which may be a service name). If the URI contains
+ * no port-string, use apr-util's notion of the default port for that
+ * scheme - if available. Otherwise use the port-number of the current
+ * server.
*/
- if(r->proxyreq) {
+ if(r->proxyreq && (r->proxyreq != PROXYREQ_REVERSE)) {
if (r->parsed_uri.port_str) {
port_str = apr_pcalloc(p, strlen(r->parsed_uri.port_str) + 2);
port_str[0] = ':';
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/22/2006 04:49 PM, Joe Orton wrote:
>
> Great, thanks. In reply to your other questions: "casts are bad" :),
> and I don't think that performance is a good excuse for randomly
> manually inlining functions either (sometimes overall performance is
> improved by reducing executable size since more code can then be cached
> in any case). You could already pick cycle-counting holes in the
> inlined code too; e.g. use of pcalloc is redundant since only the NUL
> terminator is preserved.
Thanks for explanation. You convinced me :-).
Fixed in r408729.
Regards
Rüdiger
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, May 21, 2006 at 12:31:00PM +0200, Ruediger Pluem wrote:
> On 05/18/2006 10:50 AM, Joe Orton wrote:
> > cache_storage.c: In function `cache_generate_key_default':
> > cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
>
> Fixed in r408154. Thanks.
Great, thanks. In reply to your other questions: "casts are bad" :),
and I don't think that performance is a good excuse for randomly
manually inlining functions either (sometimes overall performance is
improved by reducing executable size since more code can then be cached
in any case). You could already pick cycle-counting holes in the
inlined code too; e.g. use of pcalloc is redundant since only the NUL
terminator is preserved.
Regards,
joe
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/18/2006 10:50 AM, Joe Orton wrote:
> On Wed, May 17, 2006 at 07:16:43PM -0000,
>
>>--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
>>+++ httpd/httpd/trunk/modules/cache/cache_storage.c Wed May 17 12:16:43 2006
>
> ...
>
>>@@ -375,15 +380,18 @@
>> }
>> }
>> else {
>>- scheme = "http";
>>+ scheme = ap_http_scheme(r);
>> }
>
>
> cache_storage.c: In function `cache_generate_key_default':
> cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
Fixed in r408154. Thanks.
Regards
Rüdiger
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 17, 2006 at 07:16:43PM -0000, rpluem@apache.org wrote:
> --- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_storage.c Wed May 17 12:16:43 2006
...
> @@ -375,15 +380,18 @@
> }
> }
> else {
> - scheme = "http";
> + scheme = ap_http_scheme(r);
> }
cache_storage.c: In function `cache_generate_key_default':
cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
The below fixes the warning but I've no test setup for the cache handy,
does it still work? (the lower-casing code just above here could be
simplified like this too)
Index: modules/cache/cache_storage.c
===================================================================
--- modules/cache/cache_storage.c (revision 407503)
+++ modules/cache/cache_storage.c (working copy)
@@ -320,8 +320,8 @@
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
char**key)
{
- char *port_str, *scheme, *hn;
- const char * hostname;
+ char *port_str, *hn;
+ const char *hostname, *scheme;
int i;
/*
@@ -373,11 +373,10 @@
* manner (see above why this is needed).
*/
if (r->proxyreq && r->parsed_uri.scheme) {
- /* Copy the scheme */
- scheme = apr_pcalloc(p, strlen(r->parsed_uri.scheme) + 1);
- for (i = 0; r->parsed_uri.scheme[i]; i++) {
- scheme[i] = apr_tolower(r->parsed_uri.scheme[i]);
- }
+ /* Copy the scheme and lower-case it. */
+ char *lcs = apr_pstrdup(p, r->parsed_uri.scheme);
+ ap_str_tolower(lcs);
+ scheme = lcs;
}
else {
scheme = ap_http_scheme(r);