Re: [PATCH] http-push.c: DAV must support olny http and https scheme
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 12, 2009, 19:33 UTC
- Message-ID
- <7vskkd651b.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1239562146-32133-1-git-send-email-catap@catap.ru>
"Kirill A. Korinskiy" <catap@catap.ru> writes:
Show 6 quoted lines
> If the response from remote web-server have scp or other not http-like > scheme http-push can't go to change url, because DAV must work only > over HTTP (http and https scheme). > > Signed-off-by: Kirill A. Korinskiy <catap@catap.ru> > ---
Thanks. I was wondering about a few more things about the patch and the original code.
Show 20 quoted lines
> http-push.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index feeb340..cce9ead 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
> }
> } else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> char *path = ctx->cdata;
> - if (*ctx->cdata == 'h') {
> - path = strstr(path, "//");
> - if (path) {
> - path = strchr(path+2, '/');
> - }
> - }
> - if (path) {
> - path += repo->path_len;
> - ls->dentry_name = xstrdup(path);The original protects against an unexpected ctx->cdata such as
http://frotz
that does not have any slash after the method:// part (in which case it does not even set ls_dentry_name.
> + if (!strcmp(ctx->cdata, "http://")) {
> + path = strchr(path + sizeof("http://") - 1, '/');
> + } else if (!strcmp(ctx->cdata, "https://")) {
> + path = strchr(path + sizeof("https://") - 1, '/');I wonder what happens if the strchr() returns NULL for such a broken ctx->cdata. Will it make strlen(path) later to segfault?
Besides, obviously this patch was never tested; you meant prefixcmp, not strcmp here, so path never becomes NULL here. Oh, and instead of comparing with ctx->cdata it would be easier to compare against path.
Show 12 quoted lines
> }
> +
> + path += repo->path_len;
> +
> + ls->dentry_name = xmalloc(strlen(path) -
> + repo->path_len + 1);
> + strcpy(ls->dentry_name, path + repo->path_len);
> } else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
> ls->dentry_flags |= IS_DIR;
> }
> --
> 1.6.2