threads / patch / 18825

patchhttp-push.c: DAV must support olny http and https scheme

Subject: [PATCH] http-push.c: DAV must support olny http and https scheme

## tl;dr

7 messages between Apr 10, 2009 and Apr 13, 2009. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Kirill A. Korinskiy· Apr 10, 2009, 13:44 UTC · lore

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>
---
 http-push.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

Show changes to http-push.c +10 −9

diff --git a/http-push.c b/http-push.c
index feeb340..48c9a04 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);
+			if (!strcmp(ctx->cdata, "http://")) {
+				path = strchr(path + sizeof("http://") - 1, '/');
+			} else if (!strcmp(ctx->cdata, "https://")) {
+				path = strchr(path + sizeof("https://") - 1, '/');
 			}
+
+			path += remote->path_len;
+
+			ls->dentry_name = xmalloc(strlen(path) -
+						  remote->path_len + 1);
+			strcpy(ls->dentry_name, path + remote->path_len);
 		} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
 			ls->dentry_flags |= IS_DIR;
 		}

-- 
1.6.2

Junio C Hamano· Apr 12, 2009, 08:48 UTC · re: Kirill A. Korinskiy · lore

Re: [PATCH] http-push.c: DAV must support olny http and https scheme

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

Show 33 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>
> ---
>  http-push.c |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index feeb340..48c9a04 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);
> +			if (!strcmp(ctx->cdata, "http://")) {
> +				path = strchr(path + sizeof("http://") - 1, '/');
> +			} else if (!strcmp(ctx->cdata, "https://")) {
> +				path = strchr(path + sizeof("https://") - 1, '/');
>  			}
> +
> +			path += remote->path_len;

http-push.c: In function 'handle_remote_ls_ctx': http-push.c:1466: error: 'remote' undeclared (first use in this function) http-push.c:1466: error: (Each undeclared identifier is reported only once http-push.c:1466: error: for each function it appears in.)

Ah, crap.

Show 8 quoted lines

> +			ls->dentry_name = xmalloc(strlen(path) -
> +						  remote->path_len + 1);
> +			strcpy(ls->dentry_name, path + remote->path_len);
>  		} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
>  			ls->dentry_flags |= IS_DIR;
>  		}
> -- 
> 1.6.2

Mike Hommey· Apr 12, 2009, 09:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] http-push.c: DAV must support olny http and https scheme

On Sun, Apr 12, 2009 at 01:48:05AM -0700, Junio C Hamano wrote:

Show 42 quoted lines

> "Kirill A. Korinskiy" <catap@catap.ru> writes:
> 
> > 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>
> > ---
> >  http-push.c |   19 ++++++++++---------
> >  1 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/http-push.c b/http-push.c
> > index feeb340..48c9a04 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);
> > +			if (!strcmp(ctx->cdata, "http://")) {
> > +				path = strchr(path + sizeof("http://") - 1, '/');
> > +			} else if (!strcmp(ctx->cdata, "https://")) {
> > +				path = strchr(path + sizeof("https://") - 1, '/');
> >  			}
> > +
> > +			path += remote->path_len;
> 
> http-push.c: In function 'handle_remote_ls_ctx':
> http-push.c:1466: error: 'remote' undeclared (first use in this function)
> http-push.c:1466: error: (Each undeclared identifier is reported only once
> http-push.c:1466: error: for each function it appears in.)
> 
> Ah, crap.

s/remote/repo/. He must have done his patch before 7b5201a and rebased afterwards without checking.

Mike

Junio C Hamano· Apr 12, 2009, 18:45 UTC · re: Mike Hommey · lore

Re: [PATCH] http-push.c: DAV must support olny http and https scheme

Mike Hommey <mh@glandium.org> writes:

Show 9 quoted lines

>> http-push.c: In function 'handle_remote_ls_ctx':
>> http-push.c:1466: error: 'remote' undeclared (first use in this function)
>> http-push.c:1466: error: (Each undeclared identifier is reported only once
>> http-push.c:1466: error: for each function it appears in.)
>> 
>> Ah, crap.
>
> s/remote/repo/. He must have done his patch before 7b5201a and rebased
> afterwards without checking.

Oh, I know that.

The "crap" was about "sent without checking" part.

Kirill A. Korinskiy· Apr 12, 2009, 18:49 UTC · re: Junio C Hamano · lore

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>
---
 http-push.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

Show changes to http-push.c +10 −9

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);
+			if (!strcmp(ctx->cdata, "http://")) {
+				path = strchr(path + sizeof("http://") - 1, '/');
+			} else if (!strcmp(ctx->cdata, "https://")) {
+				path = strchr(path + sizeof("https://") - 1, '/');
 			}
+
+			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

Junio C Hamano· Apr 12, 2009, 19:33 UTC · re: Kirill A. Korinskiy · lore

Re: [PATCH] http-push.c: DAV must support olny http and https scheme

"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

Matthieu Moy· Apr 13, 2009, 16:44 UTC · re: Kirill A. Korinskiy · lore

Re: [PATCH] http-push.c: DAV must support olny http and https scheme

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

> Subject: [PATCH] http-push.c: DAV must support olny http and https scheme

^^

s/olny/only/ if it's not too late.

-- 
Matthieu