I guess the part that confuses me most is why is honoring the physical port number a bad thing? If you look at the implementation of ap_get_server_port() in the 2.0 branch, the function determines the port value by:
USE_CANONICAL_NAME_OFF || USE_CANONICAL_NAME_DNS 1- parsed_uri.port 2- server->port 3- ap_default_port() USE_CANONICAL_NAME_ON 1- server->port 2- local_addr->port 3- ap_default_port() Notice that in the USE_CANONICAL_NAME_ON it checks the physical port before calling ap_default_port() but USE_CANONICAL_NAME_OFF || DNS doesn't. Shouldn't the sources of port information be the same for both cases, just a different order? The patch in the 2.1 branch just fixes this. But, it was pointed out by nd that if the local_addr->port could never be 0 then the call to ap_default_port() is dead code and would never be called. But if it could be 0 then what is the difference between no port and a valid port value of 0. This is the reason why the backport proposal has stagnated in the STATUS file. I don't know what the correct answer is but I do believe that honoring the physical port number is a good thing and should be checked somehow. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com >>> [EMAIL PROTECTED] Wednesday, May 12, 2004 12:25:55 PM >>> What I've done, for the 1.3 case, is make honoring the physical port number (ala 2.1) a compile-time flag... This should hold us off until we figure out a better way to do this, so it may get backed out when that happens. In the meantime, 1.3.32-dev will operate as does 2.0, which is, I think, the implementation of least astonishment. On May 12, 2004, at 1:59 PM, Brad Nicholes wrote: > It works correctly for the NetWare SSL case that I was running into. > But I don't think it works correctly for the case that you are > describing. The patches that I added to 2.0 and 1.3 are NetWare > specific. The 2.0 patch is in mod_nw_ssl.c which implements the > default_port hook and the 1.3 patch was added to netware/os.c by > redefining ap_default_port() to ap_os_default_port(). These patches > resolve the issue of where does the port come from. Neither of them > address the issue of when or how the port value should be used. So I > believe that the example you gave of: > >> correctly handle some problems you were seeing. However, >> the 2.0 case is also required when Apache (on port 8000, eg) >> is behind a load-balancer (on port 80) and the LB splices >> the request to Apache. In this case, if Apache needs to >> create a self-ref with UCN Off, then it returns the >> hostname from Host (as it should) but assuming no port >> information it returns port 8000: >> >> LB: www.foo.com:80 >> Apache: foo1.foo.com:8000 >> >> Apache should send www.foo.com:80, but instead >> it'll send www.foo.com:8000 unless the client >> appends ':80' to the Host header :/ > > > would still be broken. In fact searching back through the mailing list > archieve for [EMAIL PROTECTED], Matthieu Estrade's issue with > ap_get_server_port() sounded similar to the example that you gave, but > it is what prompted the 2.1 patch and the backport proposal. > (http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=103798747720589&w=2 > ) > > Brad > > Brad Nicholes > Senior Software Engineer > Novell, Inc., the leading provider of Net business solutions > http://www.novell.com > >>>> [EMAIL PROTECTED] Wednesday, May 12, 2004 11:32:30 AM >>> > Do you mean that 2.0 now works correctly? In that case > maybe the short-term is to use the 2.0 method for both > 1.3 and 2.1, until we can figure out a better > method... I think the 2.0 method is likely "more > correct" than the 1.3/2.1 one, at least as a default > implementation. > > On May 12, 2004, at 1:13 PM, Brad Nicholes wrote: > >> Now I understand better, thanks. The issue that prompted me to >> implement the fixes for 2.1 and 1.3 manifested themselves primarily > on >> NetWare due to the way NetWare implements the SSL functionality >> (NetWare >> doesn't use mod_ssl). In some cases requrests on an SSL port were >> being >> redirected to port 80 rather than the port that they came from. > This >> problem has been solved in 2.x for NetWare by implementing the >> default_port hook in mod_nw_ssl and doing something similar in 1.3. >> It sounds like there are really two issues that need to be >> addressed at least for the 2.0 branch although they could be tied >> together. One issue, as you have described, is how or when to use a >> port value which UseCanonicalPort would solve. The other issue, > which >> has already been address in 1.3 and 2.1, is where to get the port > value >> from. Allowing Apache to look at the physical port would need to be >> added to 2.0 as it does in 1.3 and 2.1. >> >> Brad >> >> Brad Nicholes >> Senior Software Engineer >> Novell, Inc., the leading provider of Net business solutions >> http://www.novell.com >> >>>>> [EMAIL PROTECTED] Wednesday, May 12, 2004 7:28:24 AM >>> >> >> On May 11, 2004, at 6:18 PM, Brad Nicholes wrote: >> >>> +1 to Bill's comment. I don't quite understand what is confusing >> and >>> why we would need UseCanonicalPort. IMO, all that really needs to >> be >>> done is to fix UseCanonicalName so that it works according to the >>> documentation. As was explained previously, when UseCanonicalName >> is >>> OFF, both 1.3 and 2.1 try to pull the port information from the >> client >>> in any way that it can before defaulting to values supplied in the >>> .conf >>> file or the hard-coded standard port values. The problem with the >> 2.0 >>> tree is that it only looks for the port value as part of the URL >> before >>> defaulting to the known values. Before using known values, it >> should >>> look for the port in the connection information (ie. >>> r->connection->local_addr->port). The current result can produce >>> incorrect port information when a port value is not supplied as > part >> of >>> the URL. According to the documentation, if UseCanonicalName is > OFF >> it >>> should construct the self-referential information from the client. >> By >>> skipping the port information held in the connection record, it >> isn't >>> doing what it claims to be doing. >>> >> >> The rub is that with UCN Off, we either choose the port number >> sent within the Host header or we choose the actual physical >> port number; we *never* choose the configured or default port. >> The docs say: >> >> With UseCanonicalName off Apache will form self-referential >> URLs using the hostname and port supplied by the client if any >> are supplied (otherwise it will use the canonical name, as >> defined above). >> >> which is does not do currently but *is* a viable and required >> implementation in some cases, as you know since IIRC you >> were the one to adjust 2.1 to the current behavior to >> correctly handle some problems you were seeing. However, >> the 2.0 case is also required when Apache (on port 8000, eg) >> is behind a load-balancer (on port 80) and the LB splices >> the request to Apache. In this case, if Apache needs to >> create a self-ref with UCN Off, then it returns the >> hostname from Host (as it should) but assuming no port >> information it returns port 8000: >> >> LB: www.foo.com:80 >> Apache: foo1.foo.com:8000 >> >> Apache should send www.foo.com:80, but instead >> it'll send www.foo.com:8000 unless the client >> appends ':80' to the Host header :/ >> >> So both the 1.3/2.1 and the 2.0 methods may be required >> for different environments. Which means that at least >> there should be a 4th option (after On, Off and DNS) which >> says "Ignore physical port" or alternatively "Use physical >> port". But use_canonical_name is a bitfield of width 2, >> which doesn't give us enough room, so in order to prevent >> breaking the API (we can't expand it), we could tack another >> element to the end of core_dir_config to extend how the >> port is determined, hence UseCanonicalPort. >> >> > >
