modcluster / mod_proxy_cluster

mod_cluster is an intelligent native Apache httpd-based and pure-Java Undertow-based load-balancer
https://www.modcluster.io
Apache License 2.0
7 stars 15 forks source link

Fix condition in find_node_context_host #269

Closed jajik closed 2 months ago

jajik commented 2 months ago

I believe the current condition is incorrect, because in case of balancer-s->name is shorter than the prefix, it gets included for node->mess.balancer even though they don't match.

In combination with #267 it prevents 404 when the node is up but without app (yet).

jajik commented 2 months ago

Should we have if (strlen(balancer->s->name) <= BALANCER_PREFIX_LENGTH) { continue; } if (strcasecmp(&balancer->s->name[BALANCER_PREFIX_LENGTH], node->mess.balancer) != 0) { continue; } The test for strlen() is to check that the strcasecmp is not comparing somewhere outside the balancer->s->name... Is that even possible?

The proposed change is equivalent to the two ifs you wrote if I read that correctly.

For the second part, I don't think it's possible because BALANCER_PREFIX_LENGTH is smaller than the array size. However, I thought that the strlen should prevent reading invalid balancer names (e.g. if we have a valid balancer name for which we change the first char to \0, we would catch it with the strlen but wouldn't without it).