Re: [Patch] Simplifying mod_alias

2014-12-22 Thread Ruediger Pluem


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/

2014-12-22 Thread Ruediger Pluem


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

2014-12-22 Thread 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.

Regards,
Graham
—



Re: [Patch] Simplifying mod_alias

2014-12-22 Thread Eric Covener
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

2014-12-22 Thread Reindl Harald


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

2014-12-22 Thread 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.

Regards,
Graham
--



Re: [Patch] Simplifying mod_alias

2014-12-22 Thread Reindl Harald


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/

2014-12-22 Thread Eric Covener
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.