You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Sutton <pa...@ukweb.com> on 1997/05/23 15:13:09 UTC

Re: protocol/610: Multiple virtual hosts over single connection don't work

On Fri, 23 May 1997, Martin Mares wrote:
> If using multiple HTTP/1.1 GET requests on single connection to retrieve
> data from different virtual hosts, the virtual host list is scanned starting
> with the most recently used VH, not with the first one which causes that most VH's
> are ignored.
> >How-To-Repeat:
> GET / HTTP/1.1, Host: first_virtual_host; GET / HTTP/1.1, Host: primary_server_name
> >Fix:
> Set current_conn->server back to the original value after each call to read_request
> from the http_main main loop

Oh no, he's right. check_hostalias and check_serverpath search through the
vhosts starting at the one after the last one used. If they don't find a
match, they then default to using the last used vhost. Result... pages
served from the wrong vhost. This only occurs when the current request is
for a named-based vhost and the previous used vhost was defined later in
the configuration file. 

But his solution doesn't look right. If you reset the current_conf->server
inside the keepalive loop you'll lose the correct server for real virtual
hosts (which set by find_virtual_host before the
read_request/process_request loop). 

Instead you need to search the whole server chain inside
check_{hostalias,serverpath} for a matching vhost. The patch below does
this and fixes the problem. But I'm not happy with it, and hopefully
someone more familiar with the vhosting code can fix it properly. (It
imports a global, doesn't check the main server if it is a named-vhost,
and doesn't reset the vhost in use if no match is found but the previous
request was a named-vhost). 

//pcs

*** /home/paul/remote-cvs/apache/src/http_protocol.c	Thu May 15 19:36:23 1997
--- ./http_protocol.c	Fri May 23 13:48:46 1997
***************
*** 680,685 ****
--- 680,686 ----
    unsigned port = (*hostname) ? atoi(hostname) : 80;
    server_rec *s;
    int l;
+   extern server_rec *server_conf;
  
    if (port && (port != r->server->port))
      return;
***************
*** 691,697 ****
  
    r->hostname = host;
  
!   for (s = r->server->next; s; s = s->next) {
      const char *names;
      server_addr_rec *sar;
  
--- 692,698 ----
  
    r->hostname = host;
  
!   for (s = server_conf->next; s; s = s->next) {
      const char *names;
      server_addr_rec *sar;
  
***************
*** 740,745 ****
--- 741,747 ----
  }
  
  void check_serverpath (request_rec *r) {
+   extern server_rec *server_conf;
    server_rec *s;
  
    /* This is in conjunction with the ServerPath code in
***************
*** 747,753 ****
     * Host-sending request.
     */
  
!   for (s = r->server->next; s; s = s->next) {
      if (s->addrs && s->path && !strncmp(r->uri, s->path, s->pathlen) &&
  	(s->path[s->pathlen - 1] == '/' ||
  	 r->uri[s->pathlen] == '/' ||
--- 749,755 ----
     * Host-sending request.
     */
  
!   for (s = server_conf->next; s; s = s->next) {
      if (s->addrs && s->path && !strncmp(r->uri, s->path, s->pathlen) &&
  	(s->path[s->pathlen - 1] == '/' ||
  	 r->uri[s->pathlen] == '/' ||


Re: protocol/610: Multiple virtual hosts over single connection don't work

Posted by Paul Sutton <pa...@ukweb.com>.
On Fri, 23 May 1997, Dean Gaudet wrote:
> What you're experiencing is already a bug throughout our vhost handling
> code... see the vhosts-in-depth.html page.  In particular, connecting to
> one of the "main server" addresses lets you get at any single vhost, even
> if it's defined as an ip-vhost.  A solution that is consistent with our
> current code is to reset conn->server to the result of
> find_virtual_server() again. 

Yes. I guess the question is whether we are going to fix this bug
in the current code (ie for 1.2). I think it should be fixed, since
it leads to inconsistent results, and there is no work around.

I thought about calling find_virtual_server before running the loops in
check_*, but wasn't sure if it was worth the overhead. Maybe a "solution",
for now, would be to put the result of find_virtual_server into the conn
structure (e.g. "base_server"), then restore conn->server to
conn->base_server at the top of read_request. 

> The only "correct" solution imho is to overhaul the code.  Essentially I
> think the code should work like this: 
> [link named hosts to the physical vhost it runs from]

Agreed. Perhaps also add a new directive to configure name-vhosts instead
of overloading <VirtualHost> (which almost how it was implemented
originally, I seem to recall).

//pcs


Re: protocol/610: Multiple virtual hosts over single connection don't work

Posted by Ed Korthof <ed...@organic.com>.
+1 (looks good, tested briefly).

     -- Ed Korthof        |  Web Server Engineer --
     -- ed@organic.com    |  Organic Online, Inc --
     -- (415) 278-5676    |  Fax: (415) 284-6891 --

On Sun, 25 May 1997, Paul Sutton wrote:

> On Fri, 23 May 1997, Dean Gaudet wrote:
> > What you're experiencing is already a bug throughout our vhost handling
> > code... see the vhosts-in-depth.html page.  In particular, connecting to
> > one of the "main server" addresses lets you get at any single vhost, even
> > if it's defined as an ip-vhost.  A solution that is consistent with our
> > current code is to reset conn->server to the result of
> > find_virtual_server() again. 
> 
> Ok, here is a better version. The result of find_virtual_server
> is stored in the conn structure and reset at the top of each
> read_request.
> 
> //pcs
> 
> diff -c /home/paul/remote-cvs/apache/src/http_main.c ./http_main.c
> *** /home/paul/remote-cvs/apache/src/http_main.c	Thu May 15 19:36:12 1997
> --- ./http_main.c	Sun May 25 14:48:53 1997
> ***************
> *** 1614,1619 ****
> --- 1614,1620 ----
>       conn->local_addr = *saddr;
>       conn->server = find_virtual_server(saddr->sin_addr, ntohs(saddr->sin_port),
>   				       server);
> +     conn->base_server = conn->server;
>       conn->client = inout;
>       
>       conn->remote_addr = *remaddr;
> diff -c /home/paul/remote-cvs/apache/src/http_protocol.c ./http_protocol.c
> *** /home/paul/remote-cvs/apache/src/http_protocol.c	Thu May 15 19:36:23 1997
> --- ./http_protocol.c	Sun May 25 14:50:24 1997
> ***************
> *** 761,766 ****
> --- 761,767 ----
>       request_rec *r = (request_rec *)pcalloc (conn->pool, sizeof(request_rec));
>   
>       r->connection = conn;
> +     conn->server = conn->base_server;
>       r->server = conn->server;
>       r->pool = make_sub_pool(conn->pool);
>   
> diff -c /home/paul/remote-cvs/apache/src/httpd.h ./httpd.h
> *** /home/paul/remote-cvs/apache/src/httpd.h	Thu May 15 19:36:24 1997
> --- ./httpd.h	Sun May 25 14:50:55 1997
> ***************
> *** 533,538 ****
> --- 533,539 ----
>     
>     pool *pool;
>     server_rec *server;
> +   server_rec *base_server;      /* Physical vhost this conn come in on */
>     
>     /* Information about the connection itself */
>   
> 
> 
> 


Re: protocol/610: Multiple virtual hosts over single connection don't work

Posted by Paul Sutton <pa...@ukweb.com>.
On Fri, 23 May 1997, Dean Gaudet wrote:
> What you're experiencing is already a bug throughout our vhost handling
> code... see the vhosts-in-depth.html page.  In particular, connecting to
> one of the "main server" addresses lets you get at any single vhost, even
> if it's defined as an ip-vhost.  A solution that is consistent with our
> current code is to reset conn->server to the result of
> find_virtual_server() again. 

Ok, here is a better version. The result of find_virtual_server
is stored in the conn structure and reset at the top of each
read_request.

//pcs

diff -c /home/paul/remote-cvs/apache/src/http_main.c ./http_main.c
*** /home/paul/remote-cvs/apache/src/http_main.c	Thu May 15 19:36:12 1997
--- ./http_main.c	Sun May 25 14:48:53 1997
***************
*** 1614,1619 ****
--- 1614,1620 ----
      conn->local_addr = *saddr;
      conn->server = find_virtual_server(saddr->sin_addr, ntohs(saddr->sin_port),
  				       server);
+     conn->base_server = conn->server;
      conn->client = inout;
      
      conn->remote_addr = *remaddr;
diff -c /home/paul/remote-cvs/apache/src/http_protocol.c ./http_protocol.c
*** /home/paul/remote-cvs/apache/src/http_protocol.c	Thu May 15 19:36:23 1997
--- ./http_protocol.c	Sun May 25 14:50:24 1997
***************
*** 761,766 ****
--- 761,767 ----
      request_rec *r = (request_rec *)pcalloc (conn->pool, sizeof(request_rec));
  
      r->connection = conn;
+     conn->server = conn->base_server;
      r->server = conn->server;
      r->pool = make_sub_pool(conn->pool);
  
diff -c /home/paul/remote-cvs/apache/src/httpd.h ./httpd.h
*** /home/paul/remote-cvs/apache/src/httpd.h	Thu May 15 19:36:24 1997
--- ./httpd.h	Sun May 25 14:50:55 1997
***************
*** 533,538 ****
--- 533,539 ----
    
    pool *pool;
    server_rec *server;
+   server_rec *base_server;      /* Physical vhost this conn come in on */
    
    /* Information about the connection itself */
  


Re: protocol/610: Multiple virtual hosts over single connection don't work

Posted by Dean Gaudet <dg...@arctic.org>.
What you're experiencing is already a bug throughout our vhost handling
code... see the vhosts-in-depth.html page.  In particular, connecting to
one of the "main server" addresses lets you get at any single vhost, even
if it's defined as an ip-vhost.  A solution that is consistent with our
current code is to reset conn->server to the result of
find_virtual_server() again. 

BTW your patch misses mod_rewrite, which also does vhost lookup. 

The only "correct" solution imho is to overhaul the code.  Essentially I
think the code should work like this: 

- use ip address to perform a (hashed) lookup of a set of virtual hosts
which all share the same ip address

- use name techniques (Host:, http://host/, ServerPath, ServerAlias,
yadda) to select one host out of that set, be sure to test port properly

Unfortunately, even that's not quite so clear, but it's a start.  Consider
this definition: 

<VirtualHost 1.1.1.1:80 2.2.2.2:8080>
ServerName foo
</VirtualHost>

<VirtualHost 1.1.1.1:8080 2.2.2.2:80>
ServerName bar
</VirtualHost>

The current situation is far worse though.  We average a couple bugs a
month.

Dean

On Fri, 23 May 1997, Paul Sutton wrote:

> On Fri, 23 May 1997, Martin Mares wrote:
> > If using multiple HTTP/1.1 GET requests on single connection to retrieve
> > data from different virtual hosts, the virtual host list is scanned starting
> > with the most recently used VH, not with the first one which causes that most VH's
> > are ignored.
> > >How-To-Repeat:
> > GET / HTTP/1.1, Host: first_virtual_host; GET / HTTP/1.1, Host: primary_server_name
> > >Fix:
> > Set current_conn->server back to the original value after each call to read_request
> > from the http_main main loop
> 
> Oh no, he's right. check_hostalias and check_serverpath search through the
> vhosts starting at the one after the last one used. If they don't find a
> match, they then default to using the last used vhost. Result... pages
> served from the wrong vhost. This only occurs when the current request is
> for a named-based vhost and the previous used vhost was defined later in
> the configuration file. 
> 
> But his solution doesn't look right. If you reset the current_conf->server
> inside the keepalive loop you'll lose the correct server for real virtual
> hosts (which set by find_virtual_host before the
> read_request/process_request loop). 
> 
> Instead you need to search the whole server chain inside
> check_{hostalias,serverpath} for a matching vhost. The patch below does
> this and fixes the problem. But I'm not happy with it, and hopefully
> someone more familiar with the vhosting code can fix it properly. (It
> imports a global, doesn't check the main server if it is a named-vhost,
> and doesn't reset the vhost in use if no match is found but the previous
> request was a named-vhost). 
> 
> //pcs
> 
> *** /home/paul/remote-cvs/apache/src/http_protocol.c	Thu May 15 19:36:23 1997
> --- ./http_protocol.c	Fri May 23 13:48:46 1997
> ***************
> *** 680,685 ****
> --- 680,686 ----
>     unsigned port = (*hostname) ? atoi(hostname) : 80;
>     server_rec *s;
>     int l;
> +   extern server_rec *server_conf;
>   
>     if (port && (port != r->server->port))
>       return;
> ***************
> *** 691,697 ****
>   
>     r->hostname = host;
>   
> !   for (s = r->server->next; s; s = s->next) {
>       const char *names;
>       server_addr_rec *sar;
>   
> --- 692,698 ----
>   
>     r->hostname = host;
>   
> !   for (s = server_conf->next; s; s = s->next) {
>       const char *names;
>       server_addr_rec *sar;
>   
> ***************
> *** 740,745 ****
> --- 741,747 ----
>   }
>   
>   void check_serverpath (request_rec *r) {
> +   extern server_rec *server_conf;
>     server_rec *s;
>   
>     /* This is in conjunction with the ServerPath code in
> ***************
> *** 747,753 ****
>      * Host-sending request.
>      */
>   
> !   for (s = r->server->next; s; s = s->next) {
>       if (s->addrs && s->path && !strncmp(r->uri, s->path, s->pathlen) &&
>   	(s->path[s->pathlen - 1] == '/' ||
>   	 r->uri[s->pathlen] == '/' ||
> --- 749,755 ----
>      * Host-sending request.
>      */
>   
> !   for (s = server_conf->next; s; s = s->next) {
>       if (s->addrs && s->path && !strncmp(r->uri, s->path, s->pathlen) &&
>   	(s->path[s->pathlen - 1] == '/' ||
>   	 r->uri[s->pathlen] == '/' ||
> 
>