Hi,
On 08/22/2011 07:05 PM CEST +02:00, Eric Covener wrote:
>> Do you agree that this is something that needs to be fixed?
>> If yes I could start to work on a patch...
>
> I was skeptical but it certainly looks busted for non-wildcard NVH'es
> (that can match the strcmp with the VH addr) like you've explained.
>
> I suggest opening a bugzilla ticket to track.
I've just opened ticket #51709.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51709
Please find attached my patch for check_hostalias(). I tried to stick to
the idea to do the ServerName and ServerAlias check only once for each
server. Also for this reason the result is in the end merely a rewrite
of this function. I hope though that it is clear enough how it is
intended to work.
However, I believe the fix is yet incomplete. The function
ap_matches_request_vhost() used by modules like mod_proxy seems to
implement the virtual host check also in the wrong order. From reading
the comments within that function, apparently someone else wondered
about my issue in the past too:
/* search all the <VirtualHost> values */
/* XXX: If this is a NameVirtualHost then we may not be doing the
* Right Thing. consider:
*
* NameVirtualHost 10.1.1.1
* <VirtualHost 10.1.1.1>
* ServerName v1
* </VirtualHost>
* <VirtualHost 10.1.1.1>
* ServerName v2
* </VirtualHost>
*
* Suppose r->server is v2, and we're asked to match "10.1.1.1".
* We'll say "yup it's v2", when really it isn't...
* if a request came in for 10.1.1.1 it would really go to v1.
*/
I'll follow up with a patch for this function later.
Regards,
Micha
diff -ur httpd-2.2.19.orig//server/vhost.c httpd-2.2.19.patched//server/vhost.c
--- httpd-2.2.19.orig//server/vhost.c 2010-08-10 21:11:40.000000000 +0200
+++ httpd-2.2.19.patched//server/vhost.c 2011-08-23 10:31:49.827506728 +0200
@@ -872,9 +872,11 @@
const char *host = r->hostname;
apr_port_t port;
server_rec *s;
+ server_rec *virthost_s;
server_rec *last_s;
name_chain *src;
+ virthost_s = NULL;
last_s = NULL;
port = r->connection->local_addr->port;
@@ -901,23 +903,34 @@
s = src->server;
- /* does it match the virthost from the sar? */
- if (!strcasecmp(host, sar->virthost)) {
- goto found;
- }
-
- if (s == last_s) {
- /* we've already done ServerName and ServerAlias checks for this
- * vhost
- */
- continue;
+ /* If we still need to do ServerName and ServerAlias checks for this
+ * server, do them now.
+ */
+ if (s != last_s) {
+ /* does it match any ServerName or ServerAlias directive? */
+ if (matches_aliases(s, host)) {
+ goto found;
+ }
}
last_s = s;
- if (matches_aliases(s, host)) {
- goto found;
+ /* Fallback: does it match the virthost from the sar? */
+ if (!strcasecmp(host, sar->virthost)) {
+ /* only the first match is used */
+ if (virthost_s == NULL) {
+ virthost_s = s;
+ }
}
}
+
+ /* If ServerName and ServerAlias check failed, we end up here. If it
+ * matches a VirtualHost, virthost_s is set. Use that as fallback
+ */
+ if (virthost_s) {
+ s = virthost_s;
+ goto found;
+ }
+
return;
found:
