httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: mod_rewrite/proxy UDS issues
Date Tue, 25 Feb 2014 17:18:27 GMT
On Tue, Feb 25, 2014 at 4:21 PM, Jim Jagielski <jim@jagunet.com> wrote:
> Of course, this doesn't mean that Yann should wait for
> me... you seem to have a good grasp.

The following (attached) patch does the job, but I'm not it is "elegant".
It introduces the new optional ap_proxy_worker_real_url() and
proxy_worker_full_url() functions that can be used by both mod_rewrite and
mod_proxy to split/unsplit the URL wherever it needs to.

I propose it here but you probably have a better
idea...

Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c    (revision 1570613)
+++ modules/mappers/mod_rewrite.c    (working copy)
@@ -97,6 +97,7 @@
 #include "util_mutex.h"

 #include "mod_ssl.h"
+#include "mod_proxy.h"

 #include "mod_rewrite.h"
 #include "ap_expr.h"
@@ -109,6 +110,11 @@ static ap_dbd_t *(*dbd_acquire)(request_rec*) = NU
 static void (*dbd_prepare)(server_rec*, const char*, const char*) = NULL;
 static const char* really_last_key = "rewrite_really_last";

+static char *(*proxy_worker_real_url)(apr_pool_t *p, char *url,
+                                      char **split) = NULL;
+static char *(*proxy_worker_full_url)(apr_pool_t *p, char *url,
+                                      char *split) = NULL;
+
 /*
  * in order to improve performance on running production systems, you
  * may strip all rewritelog code entirely from mod_rewrite by using the
@@ -4145,6 +4151,13 @@ static int apply_rewrite_rule(rewriterule_entry *p
      * ourself).
      */
     if (p->flags & RULEFLAG_PROXY) {
+        char *real = NULL, *split = NULL;
+
+        if (proxy_worker_real_url &&
+                (real = proxy_worker_real_url(r->pool, r->filename,
&split))) {
+            r->filename = real;
+        }
+
         /* For rules evaluated in server context, the mod_proxy fixup
          * hook can be relied upon to escape the URI as and when
          * necessary, since it occurs later.  If in directory context,
@@ -4164,6 +4177,9 @@ static int apply_rewrite_rule(rewriterule_entry *p
         rewritelog((r, 2, ctx->perdir, "forcing proxy-throughput with %s",
                     r->filename));

+        if (proxy_worker_full_url && real) {
+            r->filename = proxy_worker_full_url(r->pool, r->filename,
split);
+        }
         r->filename = apr_pstrcat(r->pool, "proxy:", r->filename, NULL);
         apr_table_setn(r->notes, "rewrite-proxy", "1");
         return 1;
@@ -4384,6 +4400,8 @@ static int pre_config(apr_pool_t *pconf,
     }
     dbd_acquire = APR_RETRIEVE_OPTIONAL_FN(ap_dbd_acquire);
     dbd_prepare = APR_RETRIEVE_OPTIONAL_FN(ap_dbd_prepare);
+    proxy_worker_real_url =
APR_RETRIEVE_OPTIONAL_FN(ap_proxy_worker_real_url);
+    proxy_worker_full_url =
APR_RETRIEVE_OPTIONAL_FN(ap_proxy_worker_full_url);
     return OK;
 }

Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h    (revision 1570613)
+++ modules/proxy/mod_proxy.h    (working copy)
@@ -595,13 +595,34 @@ typedef __declspec(dllimport) const char *


 /* Connection pool API */
+
 /**
+ * Return an UDS un-aware URL from the given @a url.
+ * @param p        memory pool used for the returned URL
+ * @param url      the URL to split
+ * @param split    output for the UDS part of the URL (if not NULL)
+ * @return         the non-UDS part of the URL (if applicable),
+ *                 or the given @a url otherwise
+ */
+APR_DECLARE_OPTIONAL_FN(char *, ap_proxy_worker_real_url,
+                        (apr_pool_t *p, char *url, char **split));
+
+/**
+ * Return an UDS aware URL from the given @a url and @a split.
+ * @param p        memory pool used for the returned URL
+ * @param url      the non-UDS part of the URL
+ * @param split    the UDS part of the URL (or NULL)
+ * @return         the full URL
+ */
+APR_DECLARE_OPTIONAL_FN(char *, ap_proxy_worker_full_url,
+                        (apr_pool_t *p, char *url, char *split));
+
+/**
  * Return the user-land, UDS aware worker name
  * @param p        memory pool used for displaying worker name
  * @param worker   the worker
  * @return         name
  */
-
 PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
                                            proxy_worker *worker);

Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c    (revision 1570613)
+++ modules/proxy/proxy_util.c    (working copy)
@@ -1507,6 +1507,52 @@ PROXY_DECLARE(char *) ap_proxy_worker_name(apr_poo
     return apr_pstrcat(p, "unix:", worker->s->uds_path, "|",
worker->s->name, NULL);
 }

+static char *ap_proxy_worker_real_url(apr_pool_t *p, char *url, char
**split)
+{
+    char *ptr;
+    /*
+     * We could be passed a URL that contains the UDS path...
+     * If any, save it in *split, and strip it.
+     */
+    if (split) {
+        *split = NULL;
+    }
+    if (!strncasecmp(url, "unix:", 5) &&
+        ((ptr = ap_strchr(url, '|')) != NULL)) {
+        /* move past the 'unix:...|' UDS path info */
+        char *ret, *c;
+
+        ret = ptr + 1;
+        /* special case: "unix:....|scheme:" is OK, expand
+         * to "unix:....|scheme://localhost"
+         * */
+        c = ap_strchr(ret, ':');
+        if (c == NULL) {
+            return NULL;
+        }
+        if (split) {
+            *split = apr_pstrmemdup(p, url, ptr - url);
+        }
+        if (c[1] == '\0') {
+            return apr_pstrcat(p, ret, "//localhost", NULL);
+        }
+        else {
+            return ret;
+        }
+    }
+    return url;
+}
+
+static char *ap_proxy_worker_full_url(apr_pool_t *p, char *url, char
*split)
+{
+    if (split) {
+        return apr_pstrcat(p, split, "|", url, NULL);
+    }
+    else {
+        return url;
+    }
+}
+
 PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
                                                   proxy_balancer *balancer,
                                                   proxy_server_conf *conf,
@@ -1905,8 +1951,13 @@ PROXY_DECLARE(int) ap_proxy_pre_request(proxy_work

     access_status = proxy_run_pre_request(worker, balancer, r, conf, url);
     if (access_status == DECLINED && *balancer == NULL) {
-        *worker = ap_proxy_get_worker(r->pool, NULL, conf, *url);
+        char *real_url = *url, *uds_url = NULL;
+        if (apr_table_get(r->notes, "rewrite-proxy")) {
+            real_url = ap_proxy_worker_real_url(r->pool, *url, &uds_url);
+        }
+        *worker = ap_proxy_get_worker(r->pool, NULL, conf, real_url);
         if (*worker) {
+            *url = real_url;
             ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                           "%s: found worker %s for %s",
                           (*worker)->s->scheme, (*worker)->s->name, *url);
@@ -1931,8 +1982,6 @@ PROXY_DECLARE(int) ap_proxy_pre_request(proxy_work
         }
         else if (r->proxyreq == PROXYREQ_REVERSE) {
             if (conf->reverse) {
-                char *ptr;
-                char *ptr2;
                 ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                               "*: found reverse proxy worker for %s",
*url);
                 *balancer = NULL;
@@ -1952,27 +2001,20 @@ PROXY_DECLARE(int) ap_proxy_pre_request(proxy_work
                  * NOTE: Here we use a quick note lookup, but we could also
                  * check to see if r->filename starts with 'proxy:'
                  */
-                if (apr_table_get(r->notes, "rewrite-proxy") &&
-                    (ptr2 = ap_strcasestr(r->filename, "unix:")) &&
-                    (ptr = ap_strchr(ptr2, '|'))) {
+                if (uds_url) {
+                    apr_status_t rv;
                     apr_uri_t urisock;
-                    apr_status_t rv;
-                    *ptr = '\0';
-                    rv = apr_uri_parse(r->pool, ptr2, &urisock);
+                    rv = apr_uri_parse(r->pool, uds_url, &urisock);
                     if (rv == APR_SUCCESS) {
-                        char *rurl = ptr+1;
                         char *sockpath = ap_runtime_dir_relative(r->pool,
urisock.path);
                         apr_table_setn(r->notes, "uds_path", sockpath);
-                        *url = apr_pstrdup(r->pool, rurl); /* so we get
the scheme for the uds */
+                        *url = real_url; /* so we get the scheme for the
uds */
                         /* r->filename starts w/ "proxy:", so add after
that */
-                        memmove(r->filename+6, rurl, strlen(rurl)+1);
+                        r->filename = apr_pstrcat(r->pool, "proxy:",
real_url, NULL);
                         ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                                       "*: rewrite of url due to UDS(%s):
%s (%s)",
                                       sockpath, *url, r->filename);
                     }
-                    else {
-                        *ptr = '|';
-                    }
                 }
             }
         }
@@ -3481,4 +3523,6 @@ void proxy_util_register_hooks(apr_pool_t *p)
 {
     APR_REGISTER_OPTIONAL_FN(ap_proxy_retry_worker);
     APR_REGISTER_OPTIONAL_FN(ap_proxy_clear_connection);
+    APR_REGISTER_OPTIONAL_FN(ap_proxy_worker_real_url);
+    APR_REGISTER_OPTIONAL_FN(ap_proxy_worker_full_url);
 }
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c    (revision 1570613)
+++ modules/proxy/mod_proxy.c    (working copy)
@@ -32,6 +32,9 @@ APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup,
                          conn_rec *, request_rec *, char *));
 #endif

+static char *(*proxy_worker_real_url)(apr_pool_t *p, char *url,
+                                      char **split) = NULL;
+
 #ifndef MAX
 #define MAX(x,y) ((x) >= (y) ? (x) : (y))
 #endif
@@ -1444,36 +1447,6 @@ static const char *
     return add_proxy(cmd, dummy, f1, r1, 1);
 }

-static char *de_socketfy(apr_pool_t *p, char *url)
-{
-    char *ptr;
-    /*
-     * We could be passed a URL during the config stage that contains
-     * the UDS path... ignore it
-     */
-    if (!strncasecmp(url, "unix:", 5) &&
-        ((ptr = ap_strchr(url, '|')) != NULL)) {
-        /* move past the 'unix:...|' UDS path info */
-        char *ret, *c;
-
-        ret = ptr + 1;
-        /* special case: "unix:....|scheme:" is OK, expand
-         * to "unix:....|scheme://localhost"
-         * */
-        c = ap_strchr(ret, ':');
-        if (c == NULL) {
-            return NULL;
-        }
-        if (c[1] == '\0') {
-            return apr_pstrcat(p, ret, "//localhost", NULL);
-        }
-        else {
-            return ret;
-        }
-    }
-    return url;
-}
-
 static const char *
     add_pass(cmd_parms *cmd, void *dummy, const char *arg, int is_regex)
 {
@@ -1565,7 +1538,8 @@ static const char *
     }

     new->fake = apr_pstrdup(cmd->pool, f);
-    new->real = apr_pstrdup(cmd->pool, de_socketfy(cmd->pool, r));
+    new->real = apr_pstrdup(cmd->pool,
+                            proxy_worker_real_url(cmd->temp_pool, r,
NULL));
     new->flags = flags;
     if (use_regex) {
         new->regex = ap_pregcomp(cmd->pool, f, AP_REG_EXTENDED);
@@ -1614,7 +1588,7 @@ static const char *
         new->balancer = balancer;
     }
     else {
-        proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL,
conf, de_socketfy(cmd->pool, r));
+        proxy_worker *worker = ap_proxy_get_worker(cmd->temp_pool, NULL,
conf, new->real);
         int reuse = 0;
         if (!worker) {
             const char *err = ap_proxy_define_worker(cmd->pool, &worker,
NULL, conf, r, 0);
@@ -1626,14 +1600,15 @@ static const char *
             reuse = 1;
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server,
APLOGNO(01145)
                          "Sharing worker '%s' instead of creating new
worker '%s'",
-                         ap_proxy_worker_name(cmd->pool, worker),
new->real);
+                         ap_proxy_worker_name(cmd->temp_pool, worker),
new->real);
         }

         for (i = 0; i < arr->nelts; i++) {
             if (reuse) {
                 ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server,
APLOGNO(01146)
                              "Ignoring parameter '%s=%s' for worker '%s'
because of worker sharing",
-                             elts[i].key, elts[i].val,
ap_proxy_worker_name(cmd->pool, worker));
+                             elts[i].key, elts[i].val,
ap_proxy_worker_name(cmd->temp_pool,
+
 worker));
             } else {
                 const char *err = set_worker_param(cmd->pool, worker,
elts[i].key,
                                                    elts[i].val);
@@ -2093,7 +2068,9 @@ static const char *add_member(cmd_parms *cmd, void
     }

     /* Try to find existing worker */
-    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf,
de_socketfy(cmd->temp_pool, name));
+    worker = ap_proxy_get_worker(cmd->temp_pool, balancer, conf,
+                                 proxy_worker_real_url(cmd->temp_pool,
+                                                       name, NULL));
     if (!worker) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server,
APLOGNO(01147)
                      "Defining worker '%s' for balancer '%s'",
@@ -2179,7 +2156,9 @@ static const char *
         }
     }
     else {
-        worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf,
de_socketfy(cmd->temp_pool, name));
+        worker = ap_proxy_get_worker(cmd->temp_pool, NULL, conf,
+                                     proxy_worker_real_url(cmd->temp_pool,
+                                                           name, NULL));
         if (!worker) {
             if (in_proxy_section) {
                 err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
@@ -2319,7 +2298,9 @@ static const char *proxysection(cmd_parms *cmd, vo
         }
         else {
             worker = ap_proxy_get_worker(cmd->temp_pool, NULL, sconf,
-                                         de_socketfy(cmd->temp_pool,
(char*)conf->p));
+
proxy_worker_real_url(cmd->temp_pool,
+
(char*)conf->p,
+                                                               NULL));
             if (!worker) {
                 err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
                                           sconf, conf->p, 0);
@@ -2712,6 +2693,8 @@ static void register_hooks(apr_pool_t *p)

     /* register optional functions within proxy_util.c */
     proxy_util_register_hooks(p);
+
+    proxy_worker_real_url =
APR_RETRIEVE_OPTIONAL_FN(ap_proxy_worker_real_url);
 }

 AP_DECLARE_MODULE(proxy) =
[EOS]

Mime
View raw message