Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()

[Junio C Hamano <gitster@pobox.com>]

"Kirill A. Korinskiy" <catap@catap.ru> writes:

> @@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
>  				ls->userFunc(ls);
>  			}
>  		} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> -			ls->dentry_name = xmalloc(strlen(ctx->cdata) -
> +			char *path = ctx->cdata;
> +			if (*ctx->cdata == 'h') {
> +				path = strstr(path, "://");
> +				if (path) {
> +					path = strchr(path+3, '/');
> +				}
> +			}

Is this "://" (and +3) the only change from the previous one that has
already been queued?  I didn't have a problem with the old "//" one.

The check to see if it begins with 'h' bothers me much much more.

If you want to be defensively tight, you should be checking if it begins
with either "http://" or "https://", the only two protocols you are
prepared to handle, and nothing else, so that you won't trigger this
codepath when the other end gave you "hqrt://..", on the basis that your
code won't know if hqrt:// protocol works the same way as http and https.

On the other hand, if you want to be optimistically loose, expecting
whatever people would implement that can be handled with the existing DAV
code would behave the same way as http and https, you shouldn't be
limiting yourself to an unknown protocol name that happens to begin with
an 'h', only accepting "hqrt://" but not "ittp://" URLs.

Your "first byte of the protocol name must be 'h'" does not do either.