You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2016/02/11 15:57:04 UTC

svn commit: r1729847 - /httpd/httpd/trunk/modules/proxy/proxy_util.c

Author: ylavic
Date: Thu Feb 11 14:57:04 2016
New Revision: 1729847

URL: http://svn.apache.org/viewvc?rev=1729847&view=rev
Log:
mod_proxy: follow up to r1729826: really copy conn->ssl_hostname.

Modified:
    httpd/httpd/trunk/modules/proxy/proxy_util.c

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1729847&r1=1729846&r2=1729847&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 11 14:57:04 2016
@@ -2721,14 +2721,20 @@ PROXY_DECLARE(int) ap_proxy_connect_back
              * restore any ssl_hostname for this connection set earlier by
              * ap_proxy_determine_connection().
              */
-            const char *ssl_hostname = conn->ssl_hostname;
+            char ssl_hostname[PROXY_WORKER_MAX_HOSTNAME_SIZE];
+            if (!conn->ssl_hostname || PROXY_STRNCPY(ssl_hostname,
+                                                     conn->ssl_hostname)) {
+                ssl_hostname[0] = '\0';
+            }
 
             socket_cleanup(conn);
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
                          "%s: backend socket is disconnected.",
                          proxy_function);
 
-            conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
+            if (ssl_hostname[0]) {
+                conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
+            }
         }
     }
     while ((backend_addr || conn->uds_path) && !connected) {



Re: svn commit: r1729847 - /httpd/httpd/trunk/modules/proxy/proxy_util.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 11, 2016 at 4:42 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
> In general this looks fine. We only run in trouble if something provided in the Host header is longer then
> PROXY_WORKER_MAX_HOSTNAME_SIZE (in case of ProxyPreserveHost on). Then we loose the SNI hostname on these requests.
> Probably never happens, but could be fun to debug if it happens :-).
> So probably HUGE_STRING_LEN would be better, but a huge waste of stack resources in most cases for sure.

It seems that RFC1035 limits host/DNS names to 255/253 (not clear if
it's for each or both when the host name is dotted with the domain
name), so I choose 512 (should be enough in any case including the
trailing \0) as the limit in r1732986...

Backport proposed in r1732988.

Thanks for the review,
Yann.

Re: svn commit: r1729847 - /httpd/httpd/trunk/modules/proxy/proxy_util.c

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

On 02/11/2016 03:57 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Feb 11 14:57:04 2016
> New Revision: 1729847
> 
> URL: http://svn.apache.org/viewvc?rev=1729847&view=rev
> Log:
> mod_proxy: follow up to r1729826: really copy conn->ssl_hostname.
> 
> Modified:
>     httpd/httpd/trunk/modules/proxy/proxy_util.c

In general this looks fine. We only run in trouble if something provided in the Host header is longer then
PROXY_WORKER_MAX_HOSTNAME_SIZE (in case of ProxyPreserveHost on). Then we loose the SNI hostname on these requests.
Probably never happens, but could be fun to debug if it happens :-).
So probably HUGE_STRING_LEN would be better, but a huge waste of stack resources in most cases for sure.

Regards

RĂ¼diger

> 
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1729847&r1=1729846&r2=1729847&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 11 14:57:04 2016
> @@ -2721,14 +2721,20 @@ PROXY_DECLARE(int) ap_proxy_connect_back
>               * restore any ssl_hostname for this connection set earlier by
>               * ap_proxy_determine_connection().
>               */
> -            const char *ssl_hostname = conn->ssl_hostname;
> +            char ssl_hostname[PROXY_WORKER_MAX_HOSTNAME_SIZE];
> +            if (!conn->ssl_hostname || PROXY_STRNCPY(ssl_hostname,
> +                                                     conn->ssl_hostname)) {
> +                ssl_hostname[0] = '\0';
> +            }
>  
>              socket_cleanup(conn);
>              ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
>                           "%s: backend socket is disconnected.",
>                           proxy_function);
>  
> -            conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
> +            if (ssl_hostname[0]) {
> +                conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
> +            }
>          }
>      }
>      while ((backend_addr || conn->uds_path) && !connected) {
> 
> 
>