From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 19 Nov 2010 18:33:39 GMT
Subject: Re: [PATCH 5/6] http-push: check path length before using it
Message-ID: <20101119183339.GB26187@burratino>
In-Reply-To: <1290170790-2200-6-git-send-email-rctay89@gmail.com>

Tay Ray Chuan wrote:

> We use path_len to skip the base url/path, but we do not know for sure
> if path does indeed contain the base url/path. Check if this is so.
[...]
> --- a/http-push.c
> +++ b/http-push.c
> @@ -1116,8 +1116,14 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
>  				}
>  			}
>  			if (path) {
> -				path += repo->path_len;
> -				ls->dentry_name = xstrdup(path);
> +				if (strncmp(path,
> +					    (repo->path ? repo->path : repo->url),
> +					    repo->path_len) == 0) {

Nits: the usual idiom is

 if (!strncmp(path, ...

to make it is obvious from looking at the left side that this is a
test for equality.  The parentheses around the second parameter are
not needed here...


> +					path += repo->path_len;
> +					ls->dentry_name = xstrdup(path);
> +				} else
> +					error("Parsed path '%s' does not match url: '%s'\n",
> +					      path, (repo->path ? repo->path : repo->url));

... or here.  So perhaps:

			if (path) {
				const char *url = repo->path;
				if (!url)
					url = repo->url;
				if (strncmp(path, url, repo->path_len)) {
					error("Parsed path '%s' does not match url: '%s'\n",
						path, url);
				} else {
					path += ...

By the way, is the error behavior correct?  This prints a message with
"error: " and then continues anyway.