You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Randy Terbush <ra...@covalent.net> on 1998/02/08 22:00:48 UTC
[PATCH] setting port in construct_url()
I believe that the following is the most logical fix to the
problems we have started seeing redirecting to port 0. This
seems to solve it in the case that I found where this breaks.
I would appreciate a sanity check before commiting this. This
patche also deals with a potentially similar problem in
get_server_port().
Index: src/main/http_core.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/http_core.c,v
retrieving revision 1.156
diff -c -r1.156 http_core.c
*** http_core.c 1998/02/02 22:33:32 1.156
--- http_core.c 1998/02/08 20:52:51
***************
*** 595,608 ****
API_EXPORT(unsigned) get_server_port(const request_rec *r)
{
core_dir_config *d =
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
if (d->use_canonical_name & 1) {
! return r->server->port;
}
return r->hostname ? ntohs(r->connection->local_addr.sin_port)
! : r->server->port;
}
API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
--- 595,611 ----
API_EXPORT(unsigned) get_server_port(const request_rec *r)
{
+ unsigned port;
core_dir_config *d =
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
+ port = r->server->port ? r->server->port : default_port(r);
+
if (d->use_canonical_name & 1) {
! return port;
}
return r->hostname ? ntohs(r->connection->local_addr.sin_port)
! : port;
}
API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
***************
*** 614,625 ****
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
if (d->use_canonical_name & 1) {
! port = r->server->port;
host = r->server->server_hostname;
}
else {
! port = r->hostname ? ntohs(r->connection->local_addr.sin_port)
! : r->server->port;
host = r->hostname ? r->hostname : r->server->server_hostname;
}
if (is_default_port(port, r)) {
--- 617,633 ----
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
if (d->use_canonical_name & 1) {
! port = r->server->port ? r->server->port : default_port(r);
host = r->server->server_hostname;
}
else {
! if (r->hostname)
! port = ntohs(r->connection->local_addr.sin_port);
! else if (r->server->port)
! port = r->server->port;
! else
! port = default_port(r);
!
host = r->hostname ? r->hostname : r->server->server_hostname;
}
if (is_default_port(port, r)) {
Re: [PATCH] setting port in construct_url()
Posted by Randy Terbush <ra...@covalent.net>.
For arguments sake, lets say that the request is an FTP request.
Granted we don't support it yet, but it would be nice to start
thinking in terms of multiple protocol support.
I'll commit the patch.
Dean Gaudet <dg...@arctic.org> wrote:
> Well, s->port has been initialized to 80 for ... uh... a long time. I'm
> not certain right at this moment if http_vhost.c has any assumptions based
> on that. So you may be changing other unrelated behaviour.
>
> Remember we only support HTTP, not HTTPS... because the US gov't is lame.
> So "the current request protocol" is always HTTP.
>
> At any rate, my vhtest suite doesn't pick up any problems with the port ==
> 0, but that's probably because all the tests use explicit ports.
>
> +1 on this patch, we'll find out soon enough if port == 0 breaks folks.
>
> Dean
>
> On Sun, 8 Feb 1998, Randy Terbush wrote:
>
> > I think that it's a wrong assumption to initialize server->port to 80.
> > After looking at this for a bit, it seems that the right thing to do
> > is to make sure that the port has been initialized and if not,
> > set it to the default for the current request protocol.
> >
> > Dean Gaudet <dg...@arctic.org> wrote:
> > > But I thought you got rid of the s->port = 0 initialization. That alone
> > > would seem to fix everything. If for some reason we need s->port = 0 then
> > > the below is fine, but we'll have to document this as an API change and
> > > ask all module authors to start using get_server_port().
> > >
> > > If we go that route then I suggest we rename the server_rec.port element
> > > so that old modules won't compile (and do the wrong thing).
> > >
> > > Dean
> > >
> > > On Sun, 8 Feb 1998, Randy Terbush wrote:
> > >
> > > >
> > > > I believe that the following is the most logical fix to the
> > > > problems we have started seeing redirecting to port 0. This
> > > > seems to solve it in the case that I found where this breaks.
> > > > I would appreciate a sanity check before commiting this. This
> > > > patche also deals with a potentially similar problem in
> > > > get_server_port().
> > > >
> > > >
> > > > Index: src/main/http_core.c
> > > > ===================================================================
> > > > RCS file: /export/home/cvs/apache-1.3/src/main/http_core.c,v
> > > > retrieving revision 1.156
> > > > diff -c -r1.156 http_core.c
> > > > *** http_core.c 1998/02/02 22:33:32 1.156
> > > > --- http_core.c 1998/02/08 20:52:51
> > > > ***************
> > > > *** 595,608 ****
> > > >
> > > > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > > > {
> > > > core_dir_config *d =
> > > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > > >
> > > > if (d->use_canonical_name & 1) {
> > > > ! return r->server->port;
> > > > }
> > > > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > > ! : r->server->port;
> > > > }
> > > >
> > > > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > > > --- 595,611 ----
> > > >
> > > > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > > > {
> > > > + unsigned port;
> > > > core_dir_config *d =
> > > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > > >
> > > > + port = r->server->port ? r->server->port : default_port(r);
> > > > +
> > > > if (d->use_canonical_name & 1) {
> > > > ! return port;
> > > > }
> > > > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > > ! : port;
> > > > }
> > > >
> > > > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > > > ***************
> > > > *** 614,625 ****
> > > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > > >
> > > > if (d->use_canonical_name & 1) {
> > > > ! port = r->server->port;
> > > > host = r->server->server_hostname;
> > > > }
> > > > else {
> > > > ! port = r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > > ! : r->server->port;
> > > > host = r->hostname ? r->hostname : r->server->server_hostname;
> > > > }
> > > > if (is_default_port(port, r)) {
> > > > --- 617,633 ----
> > > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > > >
> > > > if (d->use_canonical_name & 1) {
> > > > ! port = r->server->port ? r->server->port : default_port(r);
> > > > host = r->server->server_hostname;
> > > > }
> > > > else {
> > > > ! if (r->hostname)
> > > > ! port = ntohs(r->connection->local_addr.sin_port);
> > > > ! else if (r->server->port)
> > > > ! port = r->server->port;
> > > > ! else
> > > > ! port = default_port(r);
> > > > !
> > > > host = r->hostname ? r->hostname : r->server->server_hostname;
> > > > }
> > > > if (is_default_port(port, r)) {
> > > >
> >
Re: [PATCH] setting port in construct_url()
Posted by Dean Gaudet <dg...@arctic.org>.
Well, s->port has been initialized to 80 for ... uh... a long time. I'm
not certain right at this moment if http_vhost.c has any assumptions based
on that. So you may be changing other unrelated behaviour.
Remember we only support HTTP, not HTTPS... because the US gov't is lame.
So "the current request protocol" is always HTTP.
At any rate, my vhtest suite doesn't pick up any problems with the port ==
0, but that's probably because all the tests use explicit ports.
+1 on this patch, we'll find out soon enough if port == 0 breaks folks.
Dean
On Sun, 8 Feb 1998, Randy Terbush wrote:
> I think that it's a wrong assumption to initialize server->port to 80.
> After looking at this for a bit, it seems that the right thing to do
> is to make sure that the port has been initialized and if not,
> set it to the default for the current request protocol.
>
> Dean Gaudet <dg...@arctic.org> wrote:
> > But I thought you got rid of the s->port = 0 initialization. That alone
> > would seem to fix everything. If for some reason we need s->port = 0 then
> > the below is fine, but we'll have to document this as an API change and
> > ask all module authors to start using get_server_port().
> >
> > If we go that route then I suggest we rename the server_rec.port element
> > so that old modules won't compile (and do the wrong thing).
> >
> > Dean
> >
> > On Sun, 8 Feb 1998, Randy Terbush wrote:
> >
> > >
> > > I believe that the following is the most logical fix to the
> > > problems we have started seeing redirecting to port 0. This
> > > seems to solve it in the case that I found where this breaks.
> > > I would appreciate a sanity check before commiting this. This
> > > patche also deals with a potentially similar problem in
> > > get_server_port().
> > >
> > >
> > > Index: src/main/http_core.c
> > > ===================================================================
> > > RCS file: /export/home/cvs/apache-1.3/src/main/http_core.c,v
> > > retrieving revision 1.156
> > > diff -c -r1.156 http_core.c
> > > *** http_core.c 1998/02/02 22:33:32 1.156
> > > --- http_core.c 1998/02/08 20:52:51
> > > ***************
> > > *** 595,608 ****
> > >
> > > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > > {
> > > core_dir_config *d =
> > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > >
> > > if (d->use_canonical_name & 1) {
> > > ! return r->server->port;
> > > }
> > > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > ! : r->server->port;
> > > }
> > >
> > > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > > --- 595,611 ----
> > >
> > > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > > {
> > > + unsigned port;
> > > core_dir_config *d =
> > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > >
> > > + port = r->server->port ? r->server->port : default_port(r);
> > > +
> > > if (d->use_canonical_name & 1) {
> > > ! return port;
> > > }
> > > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > ! : port;
> > > }
> > >
> > > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > > ***************
> > > *** 614,625 ****
> > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > >
> > > if (d->use_canonical_name & 1) {
> > > ! port = r->server->port;
> > > host = r->server->server_hostname;
> > > }
> > > else {
> > > ! port = r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > > ! : r->server->port;
> > > host = r->hostname ? r->hostname : r->server->server_hostname;
> > > }
> > > if (is_default_port(port, r)) {
> > > --- 617,633 ----
> > > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> > >
> > > if (d->use_canonical_name & 1) {
> > > ! port = r->server->port ? r->server->port : default_port(r);
> > > host = r->server->server_hostname;
> > > }
> > > else {
> > > ! if (r->hostname)
> > > ! port = ntohs(r->connection->local_addr.sin_port);
> > > ! else if (r->server->port)
> > > ! port = r->server->port;
> > > ! else
> > > ! port = default_port(r);
> > > !
> > > host = r->hostname ? r->hostname : r->server->server_hostname;
> > > }
> > > if (is_default_port(port, r)) {
> > >
>
Re: [PATCH] setting port in construct_url()
Posted by Randy Terbush <ra...@covalent.net>.
I think that it's a wrong assumption to initialize server->port to 80.
After looking at this for a bit, it seems that the right thing to do
is to make sure that the port has been initialized and if not,
set it to the default for the current request protocol.
Dean Gaudet <dg...@arctic.org> wrote:
> But I thought you got rid of the s->port = 0 initialization. That alone
> would seem to fix everything. If for some reason we need s->port = 0 then
> the below is fine, but we'll have to document this as an API change and
> ask all module authors to start using get_server_port().
>
> If we go that route then I suggest we rename the server_rec.port element
> so that old modules won't compile (and do the wrong thing).
>
> Dean
>
> On Sun, 8 Feb 1998, Randy Terbush wrote:
>
> >
> > I believe that the following is the most logical fix to the
> > problems we have started seeing redirecting to port 0. This
> > seems to solve it in the case that I found where this breaks.
> > I would appreciate a sanity check before commiting this. This
> > patche also deals with a potentially similar problem in
> > get_server_port().
> >
> >
> > Index: src/main/http_core.c
> > ===================================================================
> > RCS file: /export/home/cvs/apache-1.3/src/main/http_core.c,v
> > retrieving revision 1.156
> > diff -c -r1.156 http_core.c
> > *** http_core.c 1998/02/02 22:33:32 1.156
> > --- http_core.c 1998/02/08 20:52:51
> > ***************
> > *** 595,608 ****
> >
> > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > {
> > core_dir_config *d =
> > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> >
> > if (d->use_canonical_name & 1) {
> > ! return r->server->port;
> > }
> > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > ! : r->server->port;
> > }
> >
> > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > --- 595,611 ----
> >
> > API_EXPORT(unsigned) get_server_port(const request_rec *r)
> > {
> > + unsigned port;
> > core_dir_config *d =
> > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> >
> > + port = r->server->port ? r->server->port : default_port(r);
> > +
> > if (d->use_canonical_name & 1) {
> > ! return port;
> > }
> > return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > ! : port;
> > }
> >
> > API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> > ***************
> > *** 614,625 ****
> > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> >
> > if (d->use_canonical_name & 1) {
> > ! port = r->server->port;
> > host = r->server->server_hostname;
> > }
> > else {
> > ! port = r->hostname ? ntohs(r->connection->local_addr.sin_port)
> > ! : r->server->port;
> > host = r->hostname ? r->hostname : r->server->server_hostname;
> > }
> > if (is_default_port(port, r)) {
> > --- 617,633 ----
> > (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
> >
> > if (d->use_canonical_name & 1) {
> > ! port = r->server->port ? r->server->port : default_port(r);
> > host = r->server->server_hostname;
> > }
> > else {
> > ! if (r->hostname)
> > ! port = ntohs(r->connection->local_addr.sin_port);
> > ! else if (r->server->port)
> > ! port = r->server->port;
> > ! else
> > ! port = default_port(r);
> > !
> > host = r->hostname ? r->hostname : r->server->server_hostname;
> > }
> > if (is_default_port(port, r)) {
> >
Re: [PATCH] setting port in construct_url()
Posted by Dean Gaudet <dg...@arctic.org>.
But I thought you got rid of the s->port = 0 initialization. That alone
would seem to fix everything. If for some reason we need s->port = 0 then
the below is fine, but we'll have to document this as an API change and
ask all module authors to start using get_server_port().
If we go that route then I suggest we rename the server_rec.port element
so that old modules won't compile (and do the wrong thing).
Dean
On Sun, 8 Feb 1998, Randy Terbush wrote:
>
> I believe that the following is the most logical fix to the
> problems we have started seeing redirecting to port 0. This
> seems to solve it in the case that I found where this breaks.
> I would appreciate a sanity check before commiting this. This
> patche also deals with a potentially similar problem in
> get_server_port().
>
>
> Index: src/main/http_core.c
> ===================================================================
> RCS file: /export/home/cvs/apache-1.3/src/main/http_core.c,v
> retrieving revision 1.156
> diff -c -r1.156 http_core.c
> *** http_core.c 1998/02/02 22:33:32 1.156
> --- http_core.c 1998/02/08 20:52:51
> ***************
> *** 595,608 ****
>
> API_EXPORT(unsigned) get_server_port(const request_rec *r)
> {
> core_dir_config *d =
> (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
>
> if (d->use_canonical_name & 1) {
> ! return r->server->port;
> }
> return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> ! : r->server->port;
> }
>
> API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> --- 595,611 ----
>
> API_EXPORT(unsigned) get_server_port(const request_rec *r)
> {
> + unsigned port;
> core_dir_config *d =
> (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
>
> + port = r->server->port ? r->server->port : default_port(r);
> +
> if (d->use_canonical_name & 1) {
> ! return port;
> }
> return r->hostname ? ntohs(r->connection->local_addr.sin_port)
> ! : port;
> }
>
> API_EXPORT(char *) construct_url(pool *p, const char *uri, const request_rec *r)
> ***************
> *** 614,625 ****
> (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
>
> if (d->use_canonical_name & 1) {
> ! port = r->server->port;
> host = r->server->server_hostname;
> }
> else {
> ! port = r->hostname ? ntohs(r->connection->local_addr.sin_port)
> ! : r->server->port;
> host = r->hostname ? r->hostname : r->server->server_hostname;
> }
> if (is_default_port(port, r)) {
> --- 617,633 ----
> (core_dir_config *)get_module_config(r->per_dir_config, &core_module);
>
> if (d->use_canonical_name & 1) {
> ! port = r->server->port ? r->server->port : default_port(r);
> host = r->server->server_hostname;
> }
> else {
> ! if (r->hostname)
> ! port = ntohs(r->connection->local_addr.sin_port);
> ! else if (r->server->port)
> ! port = r->server->port;
> ! else
> ! port = default_port(r);
> !
> host = r->hostname ? r->hostname : r->server->server_hostname;
> }
> if (is_default_port(port, r)) {
>