On 03.10.2009 16:54, [email protected] wrote:
> Author: minfrin
> Date: Sat Oct 3 14:54:00 2009
> New Revision: 821333
>
> URL: http://svn.apache.org/viewvc?rev=821333&view=rev
> Log:
> mod_cache: Fix uri_meets_conditions() so that CacheEnable will
> match by scheme, or by a wildcarded hostname.
> PR: 40169
> Submitted by: Ryan Pendergast <rpender us.ibm.com>
> Reviewed by: Graham Leggett
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/mod_cache.xml
> httpd/httpd/trunk/modules/cache/cache_util.c
>
> Modified: httpd/httpd/trunk/modules/cache/cache_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=821333&r1=821332&r2=821333&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/cache_util.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_util.c Sat Oct 3 14:54:00 2009
> @@ -26,42 +26,79 @@
>
> /* Determine if "url" matches the hostname, scheme and port and path
> * in "filter". All but the path comparisons are case-insensitive.
> + * Note: the 's' parameters is not used currently, but needed for
> + * logging during debugging.
It is not even used for logging. Why is it present?
Please remove it or actually do logging.
> */
> -static int uri_meets_conditions(apr_uri_t filter, int pathlen, apr_uri_t url)
> -{
> - /* Compare the hostnames */
> - if(filter.hostname) {
> - if (!url.hostname) {
> +static int uri_meets_conditions(const server_rec * const s,
> + const apr_uri_t filter, const int pathlen, const apr_uri_t url) {
> + (void) s;
What is this?
> +
> + /* Scheme, hostname port and local part. The filter URI and the
> + * URI we test may have the following shapes:
> + * /<path>
> + * <scheme>[:://<hostname>[:<port>][/<path>]]
> + * That is, if there is no scheme then there must be only the path,
> + * and we check only the path; if there is a scheme, we check the
> + * scheme for equality, and then if present we match the hostname,
> + * and then if present match the port, and finally the path if any.
> + *
> + * Note that this means that "/<path>" only matches local paths,
> + * and to match proxied paths one *must* specify the scheme.
> + */
> +
> + /* Is the filter is just for a local path or a proxy URI? */
> + if (!filter.scheme) {
> + if (url.scheme || url.hostname) {
> return 0;
> }
> - else if (strcasecmp(filter.hostname, url.hostname)) {
> + } else {
Style.
> + /* The URI scheme must be present and identical except for case. */
> + if (!url.scheme || strcasecmp(filter.scheme, url.scheme)) {
> return 0;
> }
> - }
>
> - /* Compare the schemes */
> - if(filter.scheme) {
> - if (!url.scheme) {
> - return 0;
> - }
> - else if (strcasecmp(filter.scheme, url.scheme)) {
> - return 0;
> + /* If the filter hostname is null or empty it matches any hostname,
> + * if it begins with a "*" it matches the _end_ of the URI hostname
> + * excluding the "*", if it begins with a "." it matches the _end_
> + * of the URI * hostname including the ".", otherwise it must match
> + * the URI hostname exactly. */
> +
> + if (filter.hostname && filter.hostname[0]) {
> + if (filter.hostname[0] == '.') {
> + const size_t fhostlen = strlen(filter.hostname);
> + const size_t uhostlen = url.hostname ? strlen(url.hostname)
> : 0;
> +
> + if (fhostlen > uhostlen || strcasecmp(filter.hostname,
> + url.hostname + uhostlen - fhostlen)) {
> + return 0;
> + }
> + } else if (filter.hostname[0] == '*') {
Style.
> + const size_t fhostlen = strlen(filter.hostname + 1);
> + const size_t uhostlen = url.hostname ? strlen(url.hostname)
> : 0;
> +
> + if (fhostlen > uhostlen || strcasecmp(filter.hostname + 1,
> + url.hostname + uhostlen - fhostlen)) {
> + return 0;
> + }
> + } else if (!url.hostname || strcasecmp(filter.hostname,
> url.hostname)) {
Style.
> + return 0;
> + }
> }
> - }
>
> - /* Compare the ports */
> - if(filter.port_str) {
> - if (url.port_str && filter.port != url.port) {
> - return 0;
> - }
> - /* NOTE: ap_port_of_scheme will return 0 if given NULL input */
> - else if (filter.port != apr_uri_port_of_scheme(url.scheme)) {
> - return 0;
> - }
> - }
> - else if(url.port_str && filter.scheme) {
> - if (apr_uri_port_of_scheme(filter.scheme) == url.port) {
> - return 0;
> + /* If the filter port is empty it matches any URL port.
> + * If the filter or URL port are missing, or the URL port is
> + * empty, they default to the port for their scheme. */
> +
> + if (!(filter.port_str && !filter.port_str[0])) {
> + /* NOTE: ap_port_of_scheme will return 0 if given NULL input */
> + const unsigned fport = filter.port_str ? filter.port
> + : apr_uri_port_of_scheme(filter.scheme);
> + const unsigned uport = (url.port_str && url.port_str[0])
> + ? url.port : apr_uri_port_of_scheme(url.scheme);
IMHO we cannot be sure that url.scheme != NULL here.
> +
> + if (fport != uport) {
> + return 0;
> + }
> }
> }
>
> @@ -71,8 +108,7 @@
> if (!url.path) {
> if (*filter.path == '/' && pathlen == 1) {
> return 1;
> - }
> - else {
> + } else {
Please fix your editor to avoid these kind of whitespace changes
that as in case even violate our style guide.
Regards
Rüdiger
