You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/09/13 05:06:40 UTC
[PATCH] PR#1049: name-based, multi-port servers don't work
Yeah yeah I said I gave up on this code. I lied. Under discussion is
the port test at the top of check_hostaliases.
In 1.3 with this port test, name-based vhosts can only work if they are on
the same port as the main server, requests on other ports always go to
the main server (or a _default_). Without the test, name-based vhosts
work on any port (but still must have the same ip as the main server).
The main server will still pick up all otherwise unmatched hits (unless
there's a _default_ host on that port). Removing the test is a good thing
IMHO.
It's pretty much the same in 1.2 with some weirdness due to the weirdness of
* and _default_ handling. Removing the test is a good thing still. So I'm
proposing it for 1.2 as well.
Note that there are port tests further on to make sure the server selected
really should live on the port claimed by the Host: header.
Dean
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
retrieving revision 1.161
diff -u -r1.161 http_protocol.c
--- http_protocol.c 1997/09/12 18:56:02 1.161
+++ http_protocol.c 1997/09/13 02:51:05
@@ -762,9 +762,6 @@
int l;
server_rec_chain *src;
- if (port && (port != r->server->port))
- return;
-
l = strlen(host)-1;
if ((host[l]) == '.') {
host[l] = '\0';
Re: [PATCH] PR#1049: name-based, multi-port servers don't work
Posted by Dean Gaudet <dg...@arctic.org>.
I don't think the standard has anything to say about what to do when a
request comes in on port N but has a full-uri or Host: header listing port
M where M != N ... so we can do anything we want. I'm not sure what's the
most sensible. In the presence of the proxy it makes sense to trust the
physical socket port more than the string from the request -- especially
if the request is a proxy request.
So I'd +1 a patch which changes it to:
unsigned port = ntohs(r->connection.local_addr.sin_port);
and completely ignores the port that came with the request.
Dean
On Tue, 16 Sep 1997, Martin Kraemer wrote:
> On Tue, Sep 16, 1997 at 10:31:40AM -0700, Dean Gaudet wrote:
>
> > But r->server->port may not be the port supplied in the URI. And I seem
> > to recall us changing this from r->server->port to 80 ages ago ... and I
> > don't recall why. I'll have to dig through the cvs history.
>
> Of course. This routine's purpose is to _find_ the server. The initial
> setting of r->server is just a sensible default.
>
> But how about using
> unsigned port = ... ? ... : ntohs(r->connection.local_addr.sin_port);
> This would use the REAL port the request came in on. There are only few
> places in apache where it is used (check_default_server() uses it).
>
> Martin
> --
> | S I E M E N S | <Ma...@mch.sni.de> | Siemens Nixdorf
> | ------------- | Voice: +49-89-636-46021 | Informationssysteme AG
> | N I X D O R F | FAX: +49-89-636-44994 | 81730 Munich, Germany
> ~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
>
Re: [PATCH] PR#1049: name-based, multi-port servers don't work
Posted by Martin Kraemer <Ma...@mch.sni.de>.
On Tue, Sep 16, 1997 at 10:31:40AM -0700, Dean Gaudet wrote:
> But r->server->port may not be the port supplied in the URI. And I seem
> to recall us changing this from r->server->port to 80 ages ago ... and I
> don't recall why. I'll have to dig through the cvs history.
Of course. This routine's purpose is to _find_ the server. The initial
setting of r->server is just a sensible default.
But how about using
unsigned port = ... ? ... : ntohs(r->connection.local_addr.sin_port);
This would use the REAL port the request came in on. There are only few
places in apache where it is used (check_default_server() uses it).
Martin
--
| S I E M E N S | <Ma...@mch.sni.de> | Siemens Nixdorf
| ------------- | Voice: +49-89-636-46021 | Informationssysteme AG
| N I X D O R F | FAX: +49-89-636-44994 | 81730 Munich, Germany
~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
Re: [PATCH] PR#1049: name-based, multi-port servers don't work
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 16 Sep 1997, Martin Kraemer wrote:
> On Fri, Sep 12, 1997 at 09:06:40PM -0700, Dean Gaudet wrote:
> > In 1.3 with this port test, name-based vhosts can only work if they are on
> > the same port as the main server, requests on other ports always go to
> > the main server (or a _default_).
>
> +1 on the patch, but:
> It's even worse than that. The implicit assumption "if no port is given
> then use port 80" in check_hostalias() is wrong because in
> check_fulluri(), the lines
> /* Find the port */
> host = getword_nc(r->pool, &name, ':');
> ...
> r->hostname = pstrdup(r->pool, host);
> strip off the port part already before assigning it to r->hostname.
> Because check_fulluri() is the first thing that's called after the
> request line has been read, the r->hostname therefore doesn't have a
> port any more, and the assumption to use 80 is wrong.
>
> Shouldn't check_hostalias() therefore read (pseudo-patch):
>
> static void check_hostalias(request_rec *r)
> {
> const char *hostname = r->hostname;
> char *host = getword(r->pool, &hostname, ':'); /* Get rid of port */
> - unsigned port = (*hostname) ? atoi(hostname) : 80;
> + unsigned port = (*hostname) ? atoi(hostname) : r->server->port;
But r->server->port may not be the port supplied in the URI. And I seem
to recall us changing this from r->server->port to 80 ages ago ... and I
don't recall why. I'll have to dig through the cvs history.
> server_rec *s;
> int l;
> server_rec_chain *src;
>
> - if (port && (port != r->server->port))
> - return;
>
> (BTW: I have added an experimental uri-parsing addition to parse_uri()
> which chops the absoluteURI into all of its components and stores them
> in the request structure for later use by http_protocol and other
> modules. With such an information, it becomes much easier to get the
> information we want:
>
> static void check_hostalias(request_rec *r)
> {
> char *host = r->parsed_uri.hostname;
> unsigned port = r->parsed_uri.has_port ? r->parsed_uri.port : DEFAULT_PORT;
> server_rec *s;
>
> I haven't tested it thoroughly enough yet to offer it as a patch.
> I hope I'll find the time to do so soon.)
This would be the correct solution. Send it along when you've had a
chance to test please.
Dean
Re: [PATCH] PR#1049: name-based, multi-port servers don't work
Posted by Martin Kraemer <Ma...@mch.sni.de>.
On Fri, Sep 12, 1997 at 09:06:40PM -0700, Dean Gaudet wrote:
> In 1.3 with this port test, name-based vhosts can only work if they are on
> the same port as the main server, requests on other ports always go to
> the main server (or a _default_).
+1 on the patch, but:
It's even worse than that. The implicit assumption "if no port is given
then use port 80" in check_hostalias() is wrong because in
check_fulluri(), the lines
/* Find the port */
host = getword_nc(r->pool, &name, ':');
...
r->hostname = pstrdup(r->pool, host);
strip off the port part already before assigning it to r->hostname.
Because check_fulluri() is the first thing that's called after the
request line has been read, the r->hostname therefore doesn't have a
port any more, and the assumption to use 80 is wrong.
Shouldn't check_hostalias() therefore read (pseudo-patch):
static void check_hostalias(request_rec *r)
{
const char *hostname = r->hostname;
char *host = getword(r->pool, &hostname, ':'); /* Get rid of port */
- unsigned port = (*hostname) ? atoi(hostname) : 80;
+ unsigned port = (*hostname) ? atoi(hostname) : r->server->port;
server_rec *s;
int l;
server_rec_chain *src;
- if (port && (port != r->server->port))
- return;
(BTW: I have added an experimental uri-parsing addition to parse_uri()
which chops the absoluteURI into all of its components and stores them
in the request structure for later use by http_protocol and other
modules. With such an information, it becomes much easier to get the
information we want:
static void check_hostalias(request_rec *r)
{
char *host = r->parsed_uri.hostname;
unsigned port = r->parsed_uri.has_port ? r->parsed_uri.port : DEFAULT_PORT;
server_rec *s;
I haven't tested it thoroughly enough yet to offer it as a patch.
I hope I'll find the time to do so soon.)
Martin
--
| S I E M E N S | <Ma...@mch.sni.de> | Siemens Nixdorf
| ------------- | Voice: +49-89-636-46021 | Informationssysteme AG
| N I X D O R F | FAX: +49-89-636-44994 | 81730 Munich, Germany
~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
Re: [PATCH] PR#1049: name-based, multi-port servers don't work
Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 12 Sep 1997, Dean Gaudet wrote:
> Yeah yeah I said I gave up on this code. I lied. Under discussion is
> the port test at the top of check_hostaliases.
>
> In 1.3 with this port test, name-based vhosts can only work if they are on
> the same port as the main server, requests on other ports always go to
> the main server (or a _default_). Without the test, name-based vhosts
> work on any port (but still must have the same ip as the main server).
> The main server will still pick up all otherwise unmatched hits (unless
> there's a _default_ host on that port). Removing the test is a good thing
> IMHO.
>
> It's pretty much the same in 1.2 with some weirdness due to the weirdness of
> * and _default_ handling. Removing the test is a good thing still. So I'm
> proposing it for 1.2 as well.
>
> Note that there are port tests further on to make sure the server selected
> really should live on the port claimed by the Host: header.
Here is this patch again updated for the post-indent http_protocol.c.
Dean
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_protocol.c,v
retrieving revision 1.162
diff -u -r1.162 http_protocol.c
--- http_protocol.c 1997/09/14 10:04:58 1.162
+++ http_protocol.c 1997/09/16 03:33:01
@@ -766,9 +766,6 @@
int l;
server_rec_chain *src;
- if (port && (port != r->server->port))
- return;
-
l = strlen(host) - 1;
if ((host[l]) == '.') {
host[l] = '\0';