Re: [Patch] Simplifying mod_alias
On 12/21/2014 02:48 PM, Graham Leggett wrote: On 27 Jan 2014, at 12:11 AM, GRAHAM LEGGETT minf...@sharp.fm wrote: A look at mod_alias shows it has 7 directives: • Alias • AliasMatch • Redirect • RedirectMatch • RedirectPermanent • RedirectTemp • ScriptAlias • ScriptAliasMatch In theory we only need these three: • Alias • Redirect • ScriptAlias What I'm keen to do is enable expression support and deprecate all but the above, with the following as the preferred configuration method (same as the one used by ProxyPass): Location /foo Alias /var/lib/bar …stuff... /Location or LocationMatch ^/foo/(?bar[^/]+) Alias /var/lib/%{env:MATCH_BAR}/baz …stuff... /LocationMatch In theory this would be faster as we would not be scanning the list of Aliases followed by the list of Locations each time, and things get a lot simpler to use. This patch implements the above. Only one small comment on the test cases: Doesn't the httpd.conf parts need to be in an ifversion block to ensure the test suite still runs with 2.4.x / 2.2.x? Regards Rüdiger
Re: svn commit: r1647009 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml docs/manual/mod/mod_proxy_fcgi.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/
On 12/20/2014 04:56 PM, cove...@apache.org wrote: Author: covener Date: Sat Dec 20 15:56:16 2014 New Revision: 1647009 URL: http://svn.apache.org/r1647009 Log: Allow SetHandler+UDS+fcgi to take advantage of dedicated workers including opting in to connection reuse and other proxy options (max=, etc). adds 'enablereuse' proxyoption and a minor MMN bump to share proxy_desocketfy outside of mod_proxy.c, which is required to match workers to URLs. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml httpd/httpd/trunk/docs/manual/mod/mod_proxy_fcgi.xml httpd/httpd/trunk/include/ap_mmn.h httpd/httpd/trunk/modules/proxy/mod_proxy.c httpd/httpd/trunk/modules/proxy/mod_proxy.h httpd/httpd/trunk/modules/proxy/proxy_util.c Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1647009r1=1647008r2=1647009view=diff == --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original) +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Sat Dec 20 15:56:16 2014 */ @@ -1477,15 +1486,16 @@ static const char * return add_proxy(cmd, dummy, f1, r1, 1); } -static char *de_socketfy(apr_pool_t *p, char *url) +PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url) { char *ptr; /* * We could be passed a URL during the config stage that contains * the UDS path... ignore it */ +char *url_copy = apr_pstrdup(p, url); Why do we need to make a copy? if (!strncasecmp(url, unix:, 5) -((ptr = ap_strchr(url, '|')) != NULL)) { +((ptr = ap_strchr(url_copy, '|')) != NULL)) { /* move past the 'unix:...|' UDS path info */ char *ret, *c; Regards Rüdiger
Re: [Patch] Simplifying mod_alias
On 21 Dec 2014, at 10:48 PM, Eric Covener cove...@gmail.com wrote: I don't see how adding expression or Location support as necessitating, or benefiting in a meaningful way, from the deprecation / movement of the other directives. I am assuming the *match directives could either a) provide their own contexts for the back-references to work or b) simply have their 2nd parameter evaluated the way it is today. The expression support is a superset of the regex support, making the regex support redundant. The *Match parameters are self contained, you cannot make a backreference outside the scope of that single directive. In contrast the LocationMatch/DirectoryMatch sections are not self contained, their backreferences are exposed to expressions and can be used and reused in many unrelated directives. I have heard growing criticism of httpd for being too complicated, and this is an attempt to address that. Supporting 7 directives to do the job of just 3 makes people’s eyes bleed. The aim of the _compat module is to ensure that existing users are not left in the lurch, they load the compat module and life continues as before. We already have clear precedence of this working well - major changes occurred between httpd v2.2 and v2.4 in authnz and this passed by without any notable problems. Regards, Graham —
Re: [Patch] Simplifying mod_alias
On Mon, Dec 22, 2014 at 5:15 AM, Graham Leggett minf...@sharp.fm wrote: The expression support is a superset of the regex support, making the regex support redundant. The *Match parameters are self contained, you cannot make a backreference outside the scope of that single directive. In contrast the LocationMatch/DirectoryMatch sections are not self contained, their backreferences are exposed to expressions and can be used and reused in many unrelated directives. I have heard growing criticism of httpd for being too complicated, and this is an attempt to address that. Supporting 7 directives to do the job of just 3 makes people’s eyes bleed. You'll still have 7 directives though. Some of them will be marked deprecated because they're less flexible. Sometimes less flexible is a sign of simplicity and not something to be relegated to a compat module. You'll also have two modules and two pages in the manual. The examples for Redirect will now have configuration sections with named back-references and won't work in htaccess. These users whose eyes bleed at the difference between Redirect and RedirectMatch or trying to capture the first component of the path aren't exactly going to do cartwheels when at the idea of new configuration sections and named back-references to accomplish a basic task. I still don't see any reason to call the existing *Match deprecated to add expression support.
Re: [Patch] Simplifying mod_alias
Am 22.12.2014 um 11:15 schrieb Graham Leggett: On 21 Dec 2014, at 10:48 PM, Eric Covener cove...@gmail.com wrote: I don't see how adding expression or Location support as necessitating, or benefiting in a meaningful way, from the deprecation / movement of the other directives. I am assuming the *match directives could either a) provide their own contexts for the back-references to work or b) simply have their 2nd parameter evaluated the way it is today. The expression support is a superset of the regex support, making the regex support redundant. The *Match parameters are self contained, you cannot make a backreference outside the scope of that single directive. In contrast the LocationMatch/DirectoryMatch sections are not self contained, their backreferences are exposed to expressions and can be used and reused in many unrelated directives. I have heard growing criticism of httpd for being too complicated, and this is an attempt to address that. Supporting 7 directives to do the job of just 3 makes people’s eyes bleed. The aim of the _compat module is to ensure that existing users are not left in the lurch, they load the compat module and life continues as before. We already have clear precedence of this working well - major changes occurred between httpd v2.2 and v2.4 in authnz and this passed by without any notable problems. as user i will tell you something about the without any notable problems: if you use the new directives in the main configuration and somewhere below (vhost or even .htaccess) compat directives you get undefined behavior been there done that the result was rewrite *900 vhost configs* on a lot of machines using mod_version and test that over weeks to have a uninterrupted upgrade path - please don't pretend such changes are nice for users at all they are only nice for the few users which don't really matter with their httpd and a single vhost on a notebook but not on serious setups where admins are not overwheelmed with 7 directives frankly even for hobby users it's a problem because in the future you need to take care if the new hoster supports the compat-module and how to deal with .htaccess files in case you copy things to different hosts with different confogurtaions and httpd versions signature.asc Description: OpenPGP digital signature
Re: [Patch] Simplifying mod_alias
On 22 Dec 2014, at 14:53, Reindl Harald h.rei...@thelounge.net wrote: as user i will tell you something about the without any notable problems: if you use the new directives in the main configuration and somewhere below (vhost or even .htaccess) compat directives you get undefined behavior been there done that The compat module being discussed in this thread doesn't exist yet, so you could not have been there nor done that. Regards, Graham --
Re: [Patch] Simplifying mod_alias
Am 22.12.2014 um 14:26 schrieb Graham Leggett: On 22 Dec 2014, at 14:53, Reindl Harald h.rei...@thelounge.net wrote: as user i will tell you something about the without any notable problems: if you use the new directives in the main configuration and somewhere below (vhost or even .htaccess) compat directives you get undefined behavior been there done that The compat module being discussed in this thread doesn't exist yet, so you could not have been there nor done that. read my post again including the part you stripped i refered to the major changes occurred between httpd v2.2 and v2.4 in authnz and this passed by without any notable problems signature.asc Description: OpenPGP digital signature
Re: svn commit: r1647009 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml docs/manual/mod/mod_proxy_fcgi.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/
On Mon, Dec 22, 2014 at 4:56 AM, Ruediger Pluem rpl...@apache.org wrote: Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1647009r1=1647008r2=1647009view=diff == --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original) +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Sat Dec 20 15:56:16 2014 */ @@ -1477,15 +1486,16 @@ static const char * return add_proxy(cmd, dummy, f1, r1, 1); } -static char *de_socketfy(apr_pool_t *p, char *url) +PROXY_DECLARE(char *) ap_proxy_de_socketfy(apr_pool_t *p, const char *url) { char *ptr; /* * We could be passed a URL during the config stage that contains * the UDS path... ignore it */ +char *url_copy = apr_pstrdup(p, url); Why do we need to make a copy? if (!strncasecmp(url, unix:, 5) -((ptr = ap_strchr(url, '|')) != NULL)) { +((ptr = ap_strchr(url_copy, '|')) != NULL)) { /* move past the 'unix:...|' UDS path info */ char *ret, *c; Thanks -- I was trying to keep the parameter const and the return value non-const when I moved it from static to API, but it was not well done and it seems none of the callers actually need that. I removed the copy and made the return value const r1647334.