httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: mod_rewrite/proxy UDS issues
Date Mon, 24 Feb 2014 13:22:15 GMT
What sort of problem? I cannot recreate any...

Why doesn't the magic in ap_proxy_pre_request() looking
for "rewrite-proxy" happen? It should. In any case, I
think if there's a bug, then it's because mod_rewrite
isn't setting that note when needed. Doesn't that
make sense?

On Feb 21, 2014, at 8:56 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:

> Helo,
> 
> I'm facing some issue(s).while validating mod_rewrite+proxy with uds.
> 
> Here is my simple conf :
> 
>     RewriteEngine on
>     RewriteRule "^/(.*)$" "unix:/tmp/backend.sock|http://localhost/$1" [P,NE]
> 
>     <Proxy "unix:/tmp/backend.sock|http://localhost" disablereuse=off>
>     </Proxy>
> 
> First, the (pseudo-)scheme "unix:" is unknown for mod_rewrite::is_absolute_uri(), hence
mod_rewrite::fully_qualify_uri() will do its job (prepend request's scheme://host:port) against
the rewritten URL, setting r->filename to "http://requested-host/unix:/tmp/backend.sock|http://localhost/...".
> 
> I had to use the following patch to go further :
> 
> Index: modules/mappers/mod_rewrite.c
> ===================================================================
> --- modules/mappers/mod_rewrite.c    (revision 1570613)
> +++ modules/mappers/mod_rewrite.c    (working copy)
> @@ -608,6 +608,13 @@ static unsigned is_absolute_uri(char *uri, int *su
>              return 6;
>          }
>          break;
> +
> +    case 'u':
> +    case 'U':
> +        if (!strncasecmp(uri, "nix:", 4)) {        /* unix: (UDS) */
> +            return 5;
> +        }
> +        break;
>      }
>  
>      return 0;
> [END]
> 
> Then, when mod_proxy looks up the worker in ap_proxy_pre_request(), it does not de_socketfy()
the given (rewritten) URL. Hence it won't find any worker starting with "unix:" since this
part is stripped when the worker is registered, and the code always reaches the default (reverse)
worker.
> Is the config above supposed to reuse backend connections?
> To achieve this, I had to apply the following patch :
> 
> Index: modules/proxy/mod_proxy.h
> ===================================================================
> --- modules/proxy/mod_proxy.h    (revision 1570613)
> +++ modules/proxy/mod_proxy.h    (working copy)
> @@ -595,17 +595,25 @@ typedef __declspec(dllimport) const char *
>  
>  
>  /* Connection pool API */
> +
>  /**
>   * Return the user-land, UDS aware worker name
> - * @param p        memory pool used for displaying worker name
> + * @param p        memory pool used for the returned worker name
>   * @param worker   the worker
>   * @return         name
>   */
> -
>  PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
>                                             proxy_worker *worker);
>  
>  /**
> + * Return the user-land, UDS un-aware worker URL
> + * @param p        memory pool used for the returned worker URL
> + * @param url      the URL to de-socketfy
> + * @return         the de-socketfied URL (if applicable), given url otherwise
> + */
> +PROXY_DECLARE(char *) ap_proxy_worker_url(apr_pool_t *p, char *url);
> +
> +/**
>   * Get the worker from proxy configuration
>   * @param p        memory pool used for finding worker
>   * @param balancer the balancer that the worker belongs to
> Index: modules/proxy/proxy_util.c
> ===================================================================
> --- modules/proxy/proxy_util.c    (revision 1570613)
> +++ modules/proxy/proxy_util.c    (working copy)
> @@ -1507,6 +1507,36 @@ PROXY_DECLARE(char *) ap_proxy_worker_name(apr_poo
>      return apr_pstrcat(p, "unix:", worker->s->uds_path, "|", worker->s->name,
NULL);
>  }
>  
> +PROXY_DECLARE(char *) ap_proxy_worker_url(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;
> +}
> +
>  PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
>                                                    proxy_balancer *balancer,
>                                                    proxy_server_conf *conf,
> @@ -1905,8 +1935,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 *worker_url = *url;
> +        if (apr_table_get(r->notes, "rewrite-proxy")) {
> +            worker_url = ap_proxy_worker_url(r->pool, *url);
> +        }
> +        *worker = ap_proxy_get_worker(r->pool, NULL, conf, worker_url);
>          if (*worker) {
> +            *url = worker_url;
>              ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
>                            "%s: found worker %s for %s",
>                            (*worker)->s->scheme, (*worker)->s->name, *url);
> Index: modules/proxy/mod_proxy.c
> ===================================================================
> --- modules/proxy/mod_proxy.c    (revision 1570613)
> +++ modules/proxy/mod_proxy.c    (working copy)
> @@ -1444,36 +1444,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 +1535,7 @@ 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, ap_proxy_worker_url(cmd->temp_pool,
r));
>      new->flags = flags;
>      if (use_regex) {
>          new->regex = ap_pregcomp(cmd->pool, f, AP_REG_EXTENDED);
> @@ -1614,7 +1584,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 +1596,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 +2064,8 @@ 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,
> +                                 ap_proxy_worker_url(cmd->temp_pool, name));
>      if (!worker) {
>          ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01147)
>                       "Defining worker '%s' for balancer '%s'",
> @@ -2179,7 +2151,8 @@ 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,
> +                                ap_proxy_worker_url(cmd->temp_pool, name));
>          if (!worker) {
>              if (in_proxy_section) {
>                  err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
> @@ -2319,7 +2292,8 @@ 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));
> +                                         ap_proxy_worker_url(cmd->temp_pool,
> +                                                             (char*)conf->p));
>              if (!worker) {
>                  err = ap_proxy_define_worker(cmd->pool, &worker, NULL,
>                                            sconf, conf->p, 0);
> [END]
> 
> Most of the patch is due to the "export" of de_socketfy() to ap_proxy_worker_url() since
it is now used both in proxy_util.c (ap_proxy_pre_request) and mod_proxy.c (as before).
> 
> Both patches are attached as a single single patch.
> 
> Regards,
> Yann.
> <httpd-trunk-uds.patch>


Mime
View raw message