Thx...
I haven't looked over it yet, but I do see that mod_rewrite
is patched... why can't we set that note var and do
all the required magic in mod_proxy? Does mod_rewrite
itself really need to know about "unix:..."??
Why are they optional functions?
On Feb 25, 2014, at 12:18 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
> On Tue, Feb 25, 2014 at 4:21 PM, Jim Jagielski <j...@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]
> <httpd-trunk-uds2.patch>
