You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2013/12/30 20:50:53 UTC

svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Author: minfrin
Date: Mon Dec 30 19:50:52 2013
New Revision: 1554300

URL: http://svn.apache.org/r1554300
Log:
core: Support named groups and backreferences within the LocationMatch,
DirectoryMatch, FilesMatch and ProxyMatch directives.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/include/ap_mmn.h
    httpd/httpd/trunk/include/ap_regex.h
    httpd/httpd/trunk/include/http_core.h
    httpd/httpd/trunk/modules/proxy/mod_proxy.c
    httpd/httpd/trunk/modules/proxy/mod_proxy.h
    httpd/httpd/trunk/server/core.c
    httpd/httpd/trunk/server/request.c
    httpd/httpd/trunk/server/util_pcre.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Dec 30 19:50:52 2013
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) core: Support named groups and backreferences within the LocationMatch,
+     DirectoryMatch, FilesMatch and ProxyMatch directives. [Graham Leggett]
+
   *) mod_authz_user: Support the expression parser within the require
      directives. [Graham Leggett]
 

Modified: httpd/httpd/trunk/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_mmn.h (original)
+++ httpd/httpd/trunk/include/ap_mmn.h Mon Dec 30 19:50:52 2013
@@ -443,6 +443,7 @@
  * 20130924.1 (2.5.0-dev)  Add ap_proxy_connection_reusable()
  * 20131112.0 (2.5.0-dev)  Add parse_errorlog_arg to ap_errorlog_provider
  * 20131112.1 (2.5.0-dev)  Add suspend_connection and resume_connection hooks
+ * 20131112.2 (2.5.0-dev)  Add ap_regname
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -450,7 +451,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20131112
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 1                  /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 2                  /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

Modified: httpd/httpd/trunk/include/ap_regex.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_regex.h?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/include/ap_regex.h (original)
+++ httpd/httpd/trunk/include/ap_regex.h Mon Dec 30 19:50:52 2013
@@ -149,6 +149,15 @@ AP_DECLARE(int) ap_regexec_len(const ap_
 AP_DECLARE(apr_size_t) ap_regerror(int errcode, const ap_regex_t *preg,
                                    char *errbuf, apr_size_t errbuf_size);
 
+/**
+ * Return an array of named regex backreferences
+ * @param preg The precompiled regex
+ * @param names The array to which the names will be added
+ * @param upper If non zero, uppercase the names
+ */
+AP_DECLARE(int) ap_regname(const ap_regex_t *preg,
+                           apr_array_header_t *names, int upper);
+
 /** Destroy a pre-compiled regex.
  * @param preg The pre-compiled regex to free.
  */

Modified: httpd/httpd/trunk/include/http_core.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_core.h (original)
+++ httpd/httpd/trunk/include/http_core.h Mon Dec 30 19:50:52 2013
@@ -619,6 +619,10 @@ typedef struct {
 
     unsigned int allow_encoded_slashes_set : 1;
     unsigned int decode_encoded_slashes_set : 1;
+
+    /** Named back references */
+    apr_array_header_t *refs;
+
 } core_dir_config;
 
 /* macro to implement off by default behaviour */

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Mon Dec 30 19:50:52 2013
@@ -744,22 +744,52 @@ static int proxy_walk(request_rec *r)
      */
     const char *proxyname = r->filename + 6;
     int j;
+    apr_pool_t *rxpool = NULL;
 
     for (j = 0; j < num_sec; ++j)
     {
         entry_config = sec_proxy[j];
         entry_proxy = ap_get_module_config(entry_config, &proxy_module);
 
-        /* XXX: What about case insensitive matching ???
-         * Compare regex, fnmatch or string as appropriate
-         * If the entry doesn't relate, then continue
-         */
-        if (entry_proxy->r
-              ? ap_regexec(entry_proxy->r, proxyname, 0, NULL, 0)
-              : (entry_proxy->p_is_fnmatch
-                   ? apr_fnmatch(entry_proxy->p, proxyname, 0)
-                   : strncmp(proxyname, entry_proxy->p,
-                                        strlen(entry_proxy->p)))) {
+        int nmatch = 0;
+        int i;
+        ap_regmatch_t *pmatch = NULL;
+
+        if (entry_proxy->r) {
+
+            if (entry_proxy->refs && entry_proxy->refs->nelts) {
+                if (!rxpool) {
+                    apr_pool_create(&rxpool, r->pool);
+                }
+                nmatch = entry_proxy->refs->nelts;
+                pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
+            }
+
+            if (ap_regexec(entry_proxy->r, proxyname, nmatch, pmatch, 0)) {
+                continue;
+            }
+
+            for (i = 0; i < nmatch; i++) {
+                if (pmatch[i].rm_so >= 0 && pmatch[i].rm_eo >= 0 &&
+                        ((const char **)entry_proxy->refs->elts)[i]) {
+                    apr_table_setn(r->subprocess_env,
+                            ((const char **)entry_proxy->refs->elts)[i],
+                            apr_pstrndup(r->pool,
+                                    proxyname + pmatch[i].rm_so,
+                                    pmatch[i].rm_eo - pmatch[i].rm_so));
+                }
+            }
+        }
+
+        else if (
+            /* XXX: What about case insensitive matching ???
+             * Compare regex, fnmatch or string as appropriate
+             * If the entry doesn't relate, then continue
+             */
+            entry_proxy->p_is_fnmatch ? apr_fnmatch(entry_proxy->p,
+                    proxyname, 0) :
+                    strncmp(proxyname, entry_proxy->p,
+                            strlen(entry_proxy->p))) {
             continue;
         }
         per_dir_defaults = ap_merge_per_dir_configs(r->pool, per_dir_defaults,
@@ -768,6 +798,10 @@ static int proxy_walk(request_rec *r)
 
     r->per_dir_config = per_dir_defaults;
 
+    if (rxpool) {
+        apr_pool_destroy(rxpool);
+    }
+
     return OK;
 }
 
@@ -1314,6 +1348,7 @@ static void *merge_proxy_dir_config(apr_
     new->p = add->p;
     new->p_is_fnmatch = add->p_is_fnmatch;
     new->r = add->r;
+    new->refs = add->refs;
 
     /* Put these in the dir config so they work inside <Location> */
     new->raliases = apr_array_append(p, base->raliases, add->raliases);
@@ -2238,6 +2273,11 @@ static const char *proxysection(cmd_parm
     conf->p = cmd->path;
     conf->p_is_fnmatch = apr_fnmatch_test(conf->p);
 
+    if (r) {
+        conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
+        ap_regname(r, conf->refs, 1);
+    }
+
     ap_add_per_proxy_conf(cmd->server, new_dir_conf);
 
     if (*arg != '\0') {

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Dec 30 19:50:52 2013
@@ -219,6 +219,10 @@ typedef struct {
     unsigned int error_override_set:1;
     unsigned int alias_set:1;
     unsigned int add_forwarded_headers:1;
+
+    /** Named back references */
+    apr_array_header_t *refs;
+
 } proxy_dir_conf;
 
 /* if we interpolate env vars per-request, we'll need a per-request

Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Mon Dec 30 19:50:52 2013
@@ -215,6 +215,7 @@ static void *merge_core_dir_configs(apr_
     conf->d_is_fnmatch = new->d_is_fnmatch;
     conf->d_components = new->d_components;
     conf->r = new->r;
+    conf->refs = new->refs;
     conf->condition = new->condition;
 
     if (new->opts & OPT_UNSET) {
@@ -2214,6 +2215,11 @@ static const char *dirsection(cmd_parms 
     conf->d = cmd->path;
     conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
 
+    if (r) {
+        conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
+        ap_regname(r, conf->refs, 1);
+    }
+
     /* Make this explicit - the "/" root has 0 elements, that is, we
      * will always merge it, and it will always sort and merge first.
      * All others are sorted and tested by the number of slashes.
@@ -2290,6 +2296,11 @@ static const char *urlsection(cmd_parms 
     conf->d_is_fnmatch = apr_fnmatch_test(conf->d) != 0;
     conf->r = r;
 
+    if (r) {
+        conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
+        ap_regname(r, conf->refs, 1);
+    }
+
     ap_add_per_url_conf(cmd->server, new_url_conf);
 
     if (*arg != '\0') {
@@ -2372,6 +2383,11 @@ static const char *filesection(cmd_parms
     conf->d_is_fnmatch = apr_fnmatch_test(conf->d) != 0;
     conf->r = r;
 
+    if (r) {
+        conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
+        ap_regname(r, conf->refs, 1);
+    }
+
     ap_add_file_conf(cmd->pool, (core_dir_config *)mconfig, new_file_conf);
 
     if (*arg != '\0') {

Modified: httpd/httpd/trunk/server/request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/server/request.c (original)
+++ httpd/httpd/trunk/server/request.c Mon Dec 30 19:50:52 2013
@@ -741,6 +741,7 @@ AP_DECLARE(int) ap_directory_walk(reques
         apr_size_t buflen;
         char *buf;
         unsigned int seg, startseg;
+        apr_pool_t *rxpool = NULL;
 
         /* Invariant: from the first time filename_len is set until
          * it goes out of scope, filename_len==strlen(r->filename)
@@ -1196,6 +1197,10 @@ AP_DECLARE(int) ap_directory_walk(reques
          */
         for (; sec_idx < num_sec; ++sec_idx) {
 
+            int nmatch = 0;
+            int i;
+            ap_regmatch_t *pmatch = NULL;
+
             core_dir_config *entry_core;
             entry_core = ap_get_core_module_config(sec_ent[sec_idx]);
 
@@ -1203,10 +1208,29 @@ AP_DECLARE(int) ap_directory_walk(reques
                 continue;
             }
 
-            if (ap_regexec(entry_core->r, r->filename, 0, NULL, 0)) {
+            if (entry_core->refs && entry_core->refs->nelts) {
+                if (!rxpool) {
+                    apr_pool_create(&rxpool, r->pool);
+                }
+                nmatch = entry_core->refs->nelts;
+                pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
+            }
+
+            if (ap_regexec(entry_core->r, r->filename, nmatch, pmatch, 0)) {
                 continue;
             }
 
+            for (i = 0; i < nmatch; i++) {
+                if (pmatch[i].rm_so >= 0 && pmatch[i].rm_eo >= 0 &&
+                    ((const char **)entry_core->refs->elts)[i]) {
+                    apr_table_setn(r->subprocess_env, 
+                                   ((const char **)entry_core->refs->elts)[i],
+                                   apr_pstrndup(r->pool,
+                                   r->filename + pmatch[i].rm_so,
+                                   pmatch[i].rm_eo - pmatch[i].rm_so));
+                }
+            }
+
             /* If we haven't already continue'd above, we have a match.
              *
              * Calculate our full-context core opts & override.
@@ -1245,6 +1269,10 @@ AP_DECLARE(int) ap_directory_walk(reques
             last_walk->merged = now_merged;
         }
 
+        if (rxpool) {
+            apr_pool_destroy(rxpool);
+        }
+
         /* Whoops - everything matched in sequence, but either the original
          * walk found some additional matches (which we need to truncate), or
          * this walk found some additional matches.
@@ -1382,6 +1410,7 @@ AP_DECLARE(int) ap_location_walk(request
         int matches = cache->walked->nelts;
         int cached_matches = matches;
         walk_walked_t *last_walk = (walk_walked_t*)cache->walked->elts;
+        apr_pool_t *rxpool = NULL;
 
         cached &= auth_internal_per_conf;
         cache->cached = entry_uri;
@@ -1403,16 +1432,48 @@ AP_DECLARE(int) ap_location_walk(request
              * not slash terminated, then this uri must be slash
              * terminated (or at the end of the string) to match.
              */
-            if (entry_core->r
-                ? ap_regexec(entry_core->r, r->uri, 0, NULL, 0)
-                : (entry_core->d_is_fnmatch
+            if (entry_core->r) {
+
+                int nmatch = 0;
+                int i;
+                ap_regmatch_t *pmatch = NULL;
+
+                if (entry_core->refs && entry_core->refs->nelts) {
+                    if (!rxpool) {
+                        apr_pool_create(&rxpool, r->pool);
+                    }
+                    nmatch = entry_core->refs->nelts;
+                    pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
+                }
+
+                if (ap_regexec(entry_core->r, r->uri, nmatch, pmatch, 0)) {
+                    continue;
+                }
+
+                for (i = 0; i < nmatch; i++) {
+                    if (pmatch[i].rm_so >= 0 && pmatch[i].rm_eo >= 0 && 
+                        ((const char **)entry_core->refs->elts)[i]) {
+                        apr_table_setn(r->subprocess_env,
+                                       ((const char **)entry_core->refs->elts)[i],
+                                       apr_pstrndup(r->pool,
+                                       r->uri + pmatch[i].rm_so,
+                                       pmatch[i].rm_eo - pmatch[i].rm_so));
+                    }
+                }
+
+            }
+            else {
+
+                if ((entry_core->d_is_fnmatch
                    ? apr_fnmatch(entry_core->d, cache->cached, APR_FNM_PATHNAME)
                    : (strncmp(entry_core->d, cache->cached, len)
                       || (len > 0
                           && entry_core->d[len - 1] != '/'
                           && cache->cached[len] != '/'
                           && cache->cached[len] != '\0')))) {
-                continue;
+                    continue;
+                }
+
             }
 
             /* If we merged this same section last time, reuse it
@@ -1447,6 +1508,10 @@ AP_DECLARE(int) ap_location_walk(request
             last_walk->merged = now_merged;
         }
 
+        if (rxpool) {
+            apr_pool_destroy(rxpool);
+        }
+
         /* Whoops - everything matched in sequence, but either the original
          * walk found some additional matches (which we need to truncate), or
          * this walk found some additional matches.
@@ -1556,6 +1621,7 @@ AP_DECLARE(int) ap_file_walk(request_rec
         int matches = cache->walked->nelts;
         int cached_matches = matches;
         walk_walked_t *last_walk = (walk_walked_t*)cache->walked->elts;
+        apr_pool_t *rxpool = NULL;
 
         cached &= auth_internal_per_conf;
         cache->cached = test_file;
@@ -1568,12 +1634,42 @@ AP_DECLARE(int) ap_file_walk(request_rec
             core_dir_config *entry_core;
             entry_core = ap_get_core_module_config(sec_ent[sec_idx]);
 
-            if (entry_core->r
-                ? ap_regexec(entry_core->r, cache->cached , 0, NULL, 0)
-                : (entry_core->d_is_fnmatch
-                   ? apr_fnmatch(entry_core->d, cache->cached, APR_FNM_PATHNAME)
-                   : strcmp(entry_core->d, cache->cached))) {
-                continue;
+            if (entry_core->r) {
+
+                int nmatch = 0;
+                int i;
+                ap_regmatch_t *pmatch = NULL;
+
+                if (entry_core->refs && entry_core->refs->nelts) {
+                    if (!rxpool) {
+                        apr_pool_create(&rxpool, r->pool);
+                    }
+                    nmatch = entry_core->refs->nelts;
+                    pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
+                }
+
+                if (ap_regexec(entry_core->r, cache->cached, nmatch, pmatch, 0)) {
+                    continue;
+                }
+
+                for (i = 0; i < nmatch; i++) {
+                    if (pmatch[i].rm_so >= 0 && pmatch[i].rm_eo >= 0 && 
+                        ((const char **)entry_core->refs->elts)[i]) {
+                        apr_table_setn(r->subprocess_env,
+                                       ((const char **)entry_core->refs->elts)[i],
+                                       apr_pstrndup(r->pool,
+                                       cache->cached + pmatch[i].rm_so,
+                                       pmatch[i].rm_eo - pmatch[i].rm_so));
+                    }
+                }
+
+            }
+            else {
+                if ((entry_core->d_is_fnmatch
+                       ? apr_fnmatch(entry_core->d, cache->cached, APR_FNM_PATHNAME)
+                       : strcmp(entry_core->d, cache->cached))) {
+                    continue;
+                }
             }
 
             /* If we merged this same section last time, reuse it
@@ -1608,6 +1704,10 @@ AP_DECLARE(int) ap_file_walk(request_rec
             last_walk->merged = now_merged;
         }
 
+        if (rxpool) {
+            apr_pool_destroy(rxpool);
+        }
+
         /* Whoops - everything matched in sequence, but either the original
          * walk found some additional matches (which we need to truncate), or
          * this walk found some additional matches.

Modified: httpd/httpd/trunk/server/util_pcre.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300&r1=1554299&r2=1554300&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_pcre.c (original)
+++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013
@@ -45,6 +45,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #include "httpd.h"
 #include "apr_strings.h"
+#include "apr_tables.h"
 #include "pcre.h"
 
 #define APR_WANT_STRFUNC
@@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * 
     const char *errorptr;
     int erroffset;
     int errcode = 0;
-    int options = 0;
+    int options = PCRE_DUPNAMES;
 
     if ((cflags & AP_REG_ICASE) != 0)
         options |= PCRE_CASELESS;
@@ -256,4 +257,38 @@ AP_DECLARE(int) ap_regexec_len(const ap_
     }
 }
 
+AP_DECLARE(int) ap_regname(const ap_regex_t *preg,
+                           apr_array_header_t *names, int upper)
+{
+    int namecount;
+    int nameentrysize;
+    int i;
+    char *nametable;
+
+    pcre_fullinfo((const pcre *)preg->re_pcre, NULL,
+                       PCRE_INFO_NAMECOUNT, &namecount);
+    pcre_fullinfo((const pcre *)preg->re_pcre, NULL,
+                       PCRE_INFO_NAMEENTRYSIZE, &nameentrysize);
+    pcre_fullinfo((const pcre *)preg->re_pcre, NULL,
+                       PCRE_INFO_NAMETABLE, &nametable);
+
+    for (i = 0; i < namecount; i++) {
+        const char *offset = nametable + i * nameentrysize;
+        int capture = ((offset[0] << 8) + offset[1]);
+        while (names->nelts <= capture) {
+            apr_array_push(names);
+        }
+        if (upper) {
+            char *name = ((char **)names->elts)[capture] = 
+                apr_pstrdup(names->pool, offset + 2);
+            ap_str_toupper(name);
+        }
+        else {
+            ((const char **)names->elts)[capture] = offset + 2;
+        }
+    }
+
+    return namecount;
+}
+
 /* End of pcreposix.c */



Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Ruediger Pluem <rp...@apache.org>.

Jim Jagielski wrote:
> Just looking for verification here ;)

No worries.

> 
> Thx!
> On Mar 19, 2014, at 12:46 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
> 
>> As noted, from how I understand it, currently we allow it to
>> build BUT the behavior is not as expected or designed, since
>> the expected behavior *requires* PCRE_DUPNAMES. If we require
>> PCRE_DUPNAMES then we require it, right?
>>
> 
> 

Correct. As far as I remember Grahams proposal was to put the different behaviors in the documentation
as the behavior only differs in specific cases (same capture name used more then once). So IMHO the feature still
makes sense even without PCRE_DUPNAMES available.
To be honest that documentation part somehow felt through the cracks :-).

Regards

Rüdiger

Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Just looking for verification here ;)

Thx!
On Mar 19, 2014, at 12:46 PM, Jim Jagielski <ji...@jaguNET.com> wrote:

> As noted, from how I understand it, currently we allow it to
> build BUT the behavior is not as expected or designed, since
> the expected behavior *requires* PCRE_DUPNAMES. If we require
> PCRE_DUPNAMES then we require it, right?
> 


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
As noted, from how I understand it, currently we allow it to
build BUT the behavior is not as expected or designed, since
the expected behavior *requires* PCRE_DUPNAMES. If we require
PCRE_DUPNAMES then we require it, right?

On Mar 19, 2014, at 12:26 PM, Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com> wrote:

> 
> 
>> -----Original Message-----
>> From: Jim Jagielski > Sent: Mittwoch, 19. März 2014 16:37
>> To: httpd
>> Subject: Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES
>> include/ap_mmn.h include/ap_regex.h include/http_core.h
>> modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c
>> server/request.c server/util_pcre.c
>> 
>> As I understand it, we require PCRE_DUPNAMES functionality, right?
>> So I think we need to check for it at configure/build time
>> and bail if it isn't available.
>> 
>> The configure part is done in
>> 
>> 	http://svn.apache.org/r1579259
>> 
> 
> Keep in mind that this causes trunk to fail building on RHEL 5 with the system provided PCRE
> 


RE: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Jim Jagielski > Sent: Mittwoch, 19. März 2014 16:37
> To: httpd
> Subject: Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES
> include/ap_mmn.h include/ap_regex.h include/http_core.h
> modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c
> server/request.c server/util_pcre.c
> 
> As I understand it, we require PCRE_DUPNAMES functionality, right?
> So I think we need to check for it at configure/build time
> and bail if it isn't available.
> 
> The configure part is done in
> 
> 	http://svn.apache.org/r1579259
> 

Keep in mind that this causes trunk to fail building on RHEL 5 with the system provided PCRE

Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
As I understand it, we require PCRE_DUPNAMES functionality, right?
So I think we need to check for it at configure/build time
and bail if it isn't available.

The configure part is done in

	http://svn.apache.org/r1579259



Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Ahhh. Likely we can catch this at build time via configure

On Jan 15, 2014, at 8:15 AM, Graham Leggett <mi...@sharp.fm> wrote:

> On 15 Jan 2014, at 3:04 PM, Jim Jagielski <ji...@jaguNET.com> wrote:
> 
>> Sounds good to me :)
> 
> Had to do some digging to get my head around the impact.
> 
> If the PCRE_DUPNAMES is missing, the list of names of variables is shorter than the list of variables defined, and you could have a variable value applied to the wrong name. I think we can live with this as long as we clearly document that people should expect undefined behaviour on older versions of pcre if they use duplicate names inside regexes.
> 
> Example:
> 
> /(?<sitename>[^/]+)/(?<sitename>[^/]+)/(?<othername>[^/]+)
> 
> In older pcre, the second captured value "sitename" will be applied to "othername".
> 
> Regards,
> Graham
> --
> 


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 15 Jan 2014, at 3:04 PM, Jim Jagielski <ji...@jaguNET.com> wrote:

> Sounds good to me :)

Had to do some digging to get my head around the impact.

If the PCRE_DUPNAMES is missing, the list of names of variables is shorter than the list of variables defined, and you could have a variable value applied to the wrong name. I think we can live with this as long as we clearly document that people should expect undefined behaviour on older versions of pcre if they use duplicate names inside regexes.

Example:

/(?<sitename>[^/]+)/(?<sitename>[^/]+)/(?<othername>[^/]+)

In older pcre, the second captured value "sitename" will be applied to "othername".

Regards,
Graham
--


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Sounds good to me :)

On Jan 14, 2014, at 2:41 PM, Ruediger Pluem <rp...@apache.org> wrote:

> Ping?
> 
> Regards
> 
> Rüdiger
> 
> Ruediger Pluem wrote:
>> 
>> 
>> minfrin@apache.org wrote:
>>> Author: minfrin
>>> Date: Mon Dec 30 19:50:52 2013
>>> New Revision: 1554300
>>> 
>>> URL: http://svn.apache.org/r1554300
>>> Log:
>>> core: Support named groups and backreferences within the LocationMatch,
>>> DirectoryMatch, FilesMatch and ProxyMatch directives.
>>> 
>>> Modified:
>>>    httpd/httpd/trunk/CHANGES
>>>    httpd/httpd/trunk/include/ap_mmn.h
>>>    httpd/httpd/trunk/include/ap_regex.h
>>>    httpd/httpd/trunk/include/http_core.h
>>>    httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>>    httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>>    httpd/httpd/trunk/server/core.c
>>>    httpd/httpd/trunk/server/request.c
>>>    httpd/httpd/trunk/server/util_pcre.c
>>> 
>> 
>>> Modified: httpd/httpd/trunk/server/util_pcre.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300&r1=1554299&r2=1554300&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/server/util_pcre.c (original)
>>> +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013
>>> #define APR_WANT_STRFUNC
>>> @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * 
>>>     const char *errorptr;
>>>     int erroffset;
>>>     int errcode = 0;
>>> -    int options = 0;
>>> +    int options = PCRE_DUPNAMES;
>> 
>> This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6
>> on RHEL 5.
>> 
>> How about
>> 
>> Index: util_pcre.c
>> ===================================================================
>> --- util_pcre.c (revision 1556947)
>> +++ util_pcre.c (working copy)
>> @@ -125,7 +125,12 @@
>>     const char *errorptr;
>>     int erroffset;
>>     int errcode = 0;
>> +    /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
>> +#ifdef PCRE_DUPNAMES
>>     int options = PCRE_DUPNAMES;
>> +#else
>> +    int options = 0;
>> +#endif
>> 
>>     if ((cflags & AP_REG_ICASE) != 0)
>>         options |= PCRE_CASELESS;
>> 
>> 
>> instead?
>> 
>>> 
>>>     if ((cflags & AP_REG_ICASE) != 0)
>>>         options |= PCRE_CASELESS;
>> 
>> Regards
>> 
>> Rüdiger
>> 
> 


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Ruediger Pluem <rp...@apache.org>.
Ping?

Regards

Rüdiger

Ruediger Pluem wrote:
> 
> 
> minfrin@apache.org wrote:
>> Author: minfrin
>> Date: Mon Dec 30 19:50:52 2013
>> New Revision: 1554300
>>
>> URL: http://svn.apache.org/r1554300
>> Log:
>> core: Support named groups and backreferences within the LocationMatch,
>> DirectoryMatch, FilesMatch and ProxyMatch directives.
>>
>> Modified:
>>     httpd/httpd/trunk/CHANGES
>>     httpd/httpd/trunk/include/ap_mmn.h
>>     httpd/httpd/trunk/include/ap_regex.h
>>     httpd/httpd/trunk/include/http_core.h
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>     httpd/httpd/trunk/server/core.c
>>     httpd/httpd/trunk/server/request.c
>>     httpd/httpd/trunk/server/util_pcre.c
>>
> 
>> Modified: httpd/httpd/trunk/server/util_pcre.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300&r1=1554299&r2=1554300&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/server/util_pcre.c (original)
>> +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013
>>  #define APR_WANT_STRFUNC
>> @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * 
>>      const char *errorptr;
>>      int erroffset;
>>      int errcode = 0;
>> -    int options = 0;
>> +    int options = PCRE_DUPNAMES;
> 
> This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6
> on RHEL 5.
> 
> How about
> 
> Index: util_pcre.c
> ===================================================================
> --- util_pcre.c (revision 1556947)
> +++ util_pcre.c (working copy)
> @@ -125,7 +125,12 @@
>      const char *errorptr;
>      int erroffset;
>      int errcode = 0;
> +    /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
> +#ifdef PCRE_DUPNAMES
>      int options = PCRE_DUPNAMES;
> +#else
> +    int options = 0;
> +#endif
> 
>      if ((cflags & AP_REG_ICASE) != 0)
>          options |= PCRE_CASELESS;
> 
> 
> instead?
> 
>>  
>>      if ((cflags & AP_REG_ICASE) != 0)
>>          options |= PCRE_CASELESS;
> 
> Regards
> 
> Rüdiger
> 

Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Ruediger Pluem <rp...@apache.org>.

minfrin@apache.org wrote:
> Author: minfrin
> Date: Mon Dec 30 19:50:52 2013
> New Revision: 1554300
> 
> URL: http://svn.apache.org/r1554300
> Log:
> core: Support named groups and backreferences within the LocationMatch,
> DirectoryMatch, FilesMatch and ProxyMatch directives.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/include/ap_regex.h
>     httpd/httpd/trunk/include/http_core.h
>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>     httpd/httpd/trunk/server/core.c
>     httpd/httpd/trunk/server/request.c
>     httpd/httpd/trunk/server/util_pcre.c
> 

> Modified: httpd/httpd/trunk/server/util_pcre.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300&r1=1554299&r2=1554300&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_pcre.c (original)
> +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013
>  #define APR_WANT_STRFUNC
> @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * 
>      const char *errorptr;
>      int erroffset;
>      int errcode = 0;
> -    int options = 0;
> +    int options = PCRE_DUPNAMES;

This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6
on RHEL 5.

How about

Index: util_pcre.c
===================================================================
--- util_pcre.c (revision 1556947)
+++ util_pcre.c (working copy)
@@ -125,7 +125,12 @@
     const char *errorptr;
     int erroffset;
     int errcode = 0;
+    /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
+#ifdef PCRE_DUPNAMES
     int options = PCRE_DUPNAMES;
+#else
+    int options = 0;
+#endif

     if ((cflags & AP_REG_ICASE) != 0)
         options |= PCRE_CASELESS;


instead?

>  
>      if ((cflags & AP_REG_ICASE) != 0)
>          options |= PCRE_CASELESS;

Regards

Rüdiger

Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Jan 2014, at 7:26 PM, Stefan Fritsch <sf...@sfritsch.de> wrote:

> I am in favor of adding a prefix. If there are important use cases for 
> setting arbitrary variables, one could (later) add a special opt-in 
> mechanism, e.g. using "noprefix:foo" in the regex leads to variable 
> "foo" without the prefix being set.

"MATCH_" prefix added in r1555266, and the backport vote reset.

Regards,
Graham
--



Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Rainer Jung <ra...@kippdata.de>.
On 01.01.2014 18:26, Stefan Fritsch wrote:
> Am Mittwoch, 1. Januar 2014, 14:06:17 schrieb Graham Leggett:
>>> Maybe making ap_regname() accept an optional prefix string that
>>> is 
>>> prepended to each name would be a good idea?
>>>
>>>
>>>
>>> Maybe the use in <LocationMatch> and friends should add some
>>> prefix to  the names? Like "m_" or "match_" or "m:"? This would
>>> make it more difficult to shoot oneself in the foot by allowing a
>>> remote attacker to set env variables that have some special
>>> meanings elsewhere in httpd (or in an executed cgi script).
>>> And/or maybe these values should be filtered out again when
>>> exporting them to cgi env variables?
>> I wondered about this, on one hand it is nice to be able to set any
>> variable, but on the other hand there is a lot of safety in
>> preventing someone from being able to shadow an existing variable.
>> I had "MATCH_FOO" in mind originally.
> 
> I am in favor of adding a prefix. If there are important use cases for 
> setting arbitrary variables, one could (later) add a special opt-in 
> mechanism, e.g. using "noprefix:foo" in the regex leads to variable 
> "foo" without the prefix being set.

+1, mandatory prefix for now, optional configurability. Default should
be with prefix. Of your three examples I like "match_" best. "m_" might
be less likely for future conflicts, "m:" looks good, but I don't know
whether then using the variable name in other config directives could
lead to problems because of the colon in the name.

Thanks for pushing this nice enhancement!

Regards,

Rainer


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Stefan Fritsch <sf...@sfritsch.de>.
Am Mittwoch, 1. Januar 2014, 14:06:17 schrieb Graham Leggett:
> > Maybe making ap_regname() accept an optional prefix string that
> > is 
> > prepended to each name would be a good idea?
> >
> > 
> >
> > Maybe the use in <LocationMatch> and friends should add some
> > prefix to  the names? Like "m_" or "match_" or "m:"? This would
> > make it more difficult to shoot oneself in the foot by allowing a
> > remote attacker to set env variables that have some special
> > meanings elsewhere in httpd (or in an executed cgi script).
> > And/or maybe these values should be filtered out again when
> > exporting them to cgi env variables?
> I wondered about this, on one hand it is nice to be able to set any
> variable, but on the other hand there is a lot of safety in
> preventing someone from being able to shadow an existing variable.
> I had "MATCH_FOO" in mind originally.

I am in favor of adding a prefix. If there are important use cases for 
setting arbitrary variables, one could (later) add a special opt-in 
mechanism, e.g. using "noprefix:foo" in the regex leads to variable 
"foo" without the prefix being set.

Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Jan 2014, at 1:59 PM, Stefan Fritsch <sf...@sfritsch.de> wrote:

> I definitely like this idea. While I haven't done a full review of the 
> patch, I have a few questions:
> 
> Aren't the apr_table keys case insensitive anyway? Why do we need the 
> case conversion of the key names?

All the variables in subprocess_env are all uppecased, before I added the uppercasing the variables were the only ones lowercased when they were listed and it looked wrong.

> Maybe making ap_regname() accept an optional prefix string that is 
> prepended to each name would be a good idea?
> 
> Maybe the use in <LocationMatch> and friends should add some prefix to 
> the names? Like "m_" or "match_" or "m:"? This would make it more 
> difficult to shoot oneself in the foot by allowing a remote attacker 
> to set env variables that have some special meanings elsewhere in 
> httpd (or in an executed cgi script). And/or maybe these values should 
> be filtered out again when exporting them to cgi env variables?

I wondered about this, on one hand it is nice to be able to set any variable, but on the other hand there is a lot of safety in preventing someone from being able to shadow an existing variable. I had "MATCH_FOO" in mind originally.

Regards,
Graham
--


Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c

Posted by Stefan Fritsch <sf...@sfritsch.de>.
Am Montag, 30. Dezember 2013, 19:50:53 schrieb minfrin@apache.org:
> Author: minfrin
> Date: Mon Dec 30 19:50:52 2013
> New Revision: 1554300
> 
> URL: http://svn.apache.org/r1554300
> Log:
> core: Support named groups and backreferences within the
> LocationMatch, DirectoryMatch, FilesMatch and ProxyMatch
> directives.

I definitely like this idea. While I haven't done a full review of the 
patch, I have a few questions:

Aren't the apr_table keys case insensitive anyway? Why do we need the 
case conversion of the key names?

Maybe making ap_regname() accept an optional prefix string that is 
prepended to each name would be a good idea?

Maybe the use in <LocationMatch> and friends should add some prefix to 
the names? Like "m_" or "match_" or "m:"? This would make it more 
difficult to shoot oneself in the foot by allowing a remote attacker 
to set env variables that have some special meanings elsewhere in 
httpd (or in an executed cgi script). And/or maybe these values should 
be filtered out again when exporting them to cgi env variables?

Cheers,
Stefan