{"thread":{"id":"18825","subject":"[PATCH] http-push.c: DAV must support olny http and https scheme","startedAt":"2009-04-10T13:44:20Z","lastAt":"2009-04-13T16:44:28Z","messageCount":7,"participants":["Kirill A. Korinskiy","Junio C Hamano","Mike Hommey","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"111008","messageId":"1239371060-3978-1-git-send-email-catap@catap.ru","threadId":"18825","inReplyTo":null,"subject":"[PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2009-04-10T13:44:20Z","receivedAt":"2009-04-10T13:44:20Z","isPatch":true,"sender":{"key":"catap@catap.ru","avatar":"https://gravatar.com/avatar/ea0ab2c29579606bd684eccdf786c666f4425bedc4e599b698a175f12737b1c5?d=mp&s=160"},"body":"If the response from remote web-server have scp or other not http-like\nscheme http-push can't go to change url, because DAV must work only\nover HTTP (http and https scheme).\n\nSigned-off-by: Kirill A. Korinskiy \n---\n http-push.c | 19 ++++++++++---------\n 1 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex feeb340..48c9a04 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n \t\t\tchar *path = ctx->cdata;\n-\t\t\tif (*ctx->cdata == 'h') {\n-\t\t\t\tpath = strstr(path, \"//\");\n-\t\t\t\tif (path) {\n-\t\t\t\t\tpath = strchr(path+2, '/');\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tif (path) {\n-\t\t\t\tpath += repo->path_len;\n-\t\t\t\tls->dentry_name = xstrdup(path);\n+\t\t\tif (!strcmp(ctx->cdata, \"http://\")) {\n+\t\t\t\tpath = strchr(path + sizeof(\"http://\") - 1, '/');\n+\t\t\t} else if (!strcmp(ctx->cdata, \"https://\")) {\n+\t\t\t\tpath = strchr(path + sizeof(\"https://\") - 1, '/');\n \t\t\t}\n+\n+\t\t\tpath += remote->path_len;\n+\n+\t\t\tls->dentry_name = xmalloc(strlen(path) -\n+\t\t\t\t\t\t remote->path_len + 1);\n+\t\t\tstrcpy(ls->dentry_name, path + remote->path_len);\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n \t\t\tls->dentry_flags |= IS_DIR;\n \t\t}\n-- \n1.6.2\n"},{"id":"111124","messageId":"7vd4bi9s1m.fsf@gitster.siamese.dyndns.org","threadId":"18825","inReplyTo":"1239371060-3978-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-12T08:48:05Z","receivedAt":"2009-04-12T08:48:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Korinskiy\" writes:\n\n> If the response from remote web-server have scp or other not http-like\n> scheme http-push can't go to change url, because DAV must work only\n> over HTTP (http and https scheme).\n>\n> Signed-off-by: Kirill A. Korinskiy \n> ---\n> http-push.c | 19 ++++++++++---------\n> 1 files changed, 10 insertions(+), 9 deletions(-)\n>\n> diff --git a/http-push.c b/http-push.c\n> index feeb340..48c9a04 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n> \t\t\t}\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n> \t\t\tchar *path = ctx->cdata;\n> -\t\t\tif (*ctx->cdata == 'h') {\n> -\t\t\t\tpath = strstr(path, \"//\");\n> -\t\t\t\tif (path) {\n> -\t\t\t\t\tpath = strchr(path+2, '/');\n> -\t\t\t\t}\n> -\t\t\t}\n> -\t\t\tif (path) {\n> -\t\t\t\tpath += repo->path_len;\n> -\t\t\t\tls->dentry_name = xstrdup(path);\n> +\t\t\tif (!strcmp(ctx->cdata, \"http://\")) {\n> +\t\t\t\tpath = strchr(path + sizeof(\"http://\") - 1, '/');\n> +\t\t\t} else if (!strcmp(ctx->cdata, \"https://\")) {\n> +\t\t\t\tpath = strchr(path + sizeof(\"https://\") - 1, '/');\n> \t\t\t}\n> +\n> +\t\t\tpath += remote->path_len;\n\nhttp-push.c: In function 'handle_remote_ls_ctx':\nhttp-push.c:1466: error: 'remote' undeclared (first use in this function)\nhttp-push.c:1466: error: (Each undeclared identifier is reported only once\nhttp-push.c:1466: error: for each function it appears in.)\n\nAh, crap.\n\n> +\t\t\tls->dentry_name = xmalloc(strlen(path) -\n> +\t\t\t\t\t\t remote->path_len + 1);\n> +\t\t\tstrcpy(ls->dentry_name, path + remote->path_len);\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n> \t\t\tls->dentry_flags |= IS_DIR;\n> \t\t}\n> -- \n> 1.6.2\n"},{"id":"111125","messageId":"20090412090053.GA9920@glandium.org","threadId":"18825","inReplyTo":"7vd4bi9s1m.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-04-12T09:00:53Z","receivedAt":"2009-04-12T09:00:53Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Apr 12, 2009 at 01:48:05AM -0700, Junio C Hamano wrote:\n> \"Kirill A. Korinskiy\" writes:\n> \n> > If the response from remote web-server have scp or other not http-like\n> > scheme http-push can't go to change url, because DAV must work only\n> > over HTTP (http and https scheme).\n> >\n> > Signed-off-by: Kirill A. Korinskiy \n> > ---\n> > http-push.c | 19 ++++++++++---------\n> > 1 files changed, 10 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/http-push.c b/http-push.c\n> > index feeb340..48c9a04 100644\n> > --- a/http-push.c\n> > +++ b/http-push.c\n> > @@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n> > \t\t\t}\n> > \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n> > \t\t\tchar *path = ctx->cdata;\n> > -\t\t\tif (*ctx->cdata == 'h') {\n> > -\t\t\t\tpath = strstr(path, \"//\");\n> > -\t\t\t\tif (path) {\n> > -\t\t\t\t\tpath = strchr(path+2, '/');\n> > -\t\t\t\t}\n> > -\t\t\t}\n> > -\t\t\tif (path) {\n> > -\t\t\t\tpath += repo->path_len;\n> > -\t\t\t\tls->dentry_name = xstrdup(path);\n> > +\t\t\tif (!strcmp(ctx->cdata, \"http://\")) {\n> > +\t\t\t\tpath = strchr(path + sizeof(\"http://\") - 1, '/');\n> > +\t\t\t} else if (!strcmp(ctx->cdata, \"https://\")) {\n> > +\t\t\t\tpath = strchr(path + sizeof(\"https://\") - 1, '/');\n> > \t\t\t}\n> > +\n> > +\t\t\tpath += remote->path_len;\n> \n> http-push.c: In function 'handle_remote_ls_ctx':\n> http-push.c:1466: error: 'remote' undeclared (first use in this function)\n> http-push.c:1466: error: (Each undeclared identifier is reported only once\n> http-push.c:1466: error: for each function it appears in.)\n> \n> Ah, crap.\n\ns/remote/repo/. He must have done his patch before 7b5201a and rebased\nafterwards without checking.\n\nMike\n"},{"id":"111134","messageId":"7vprfh7lua.fsf@gitster.siamese.dyndns.org","threadId":"18825","inReplyTo":"20090412090053.GA9920@glandium.org","subject":"Re: [PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-12T18:45:01Z","receivedAt":"2009-04-12T18:45:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n>> http-push.c: In function 'handle_remote_ls_ctx':\n>> http-push.c:1466: error: 'remote' undeclared (first use in this function)\n>> http-push.c:1466: error: (Each undeclared identifier is reported only once\n>> http-push.c:1466: error: for each function it appears in.)\n>> \n>> Ah, crap.\n>\n> s/remote/repo/. He must have done his patch before 7b5201a and rebased\n> afterwards without checking.\n\nOh, I know that.\n\nThe \"crap\" was about \"sent without checking\" part.\n"},{"id":"111136","messageId":"1239562146-32133-1-git-send-email-catap@catap.ru","threadId":"18825","inReplyTo":"7vd4bi9s1m.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2009-04-12T18:49:06Z","receivedAt":"2009-04-12T18:49:06Z","isPatch":true,"sender":{"key":"catap@catap.ru","avatar":"https://gravatar.com/avatar/ea0ab2c29579606bd684eccdf786c666f4425bedc4e599b698a175f12737b1c5?d=mp&s=160"},"body":"If the response from remote web-server have scp or other not http-like\nscheme http-push can't go to change url, because DAV must work only\nover HTTP (http and https scheme).\n\nSigned-off-by: Kirill A. Korinskiy \n---\n http-push.c | 19 ++++++++++---------\n 1 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex feeb340..cce9ead 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n \t\t\tchar *path = ctx->cdata;\n-\t\t\tif (*ctx->cdata == 'h') {\n-\t\t\t\tpath = strstr(path, \"//\");\n-\t\t\t\tif (path) {\n-\t\t\t\t\tpath = strchr(path+2, '/');\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tif (path) {\n-\t\t\t\tpath += repo->path_len;\n-\t\t\t\tls->dentry_name = xstrdup(path);\n+\t\t\tif (!strcmp(ctx->cdata, \"http://\")) {\n+\t\t\t\tpath = strchr(path + sizeof(\"http://\") - 1, '/');\n+\t\t\t} else if (!strcmp(ctx->cdata, \"https://\")) {\n+\t\t\t\tpath = strchr(path + sizeof(\"https://\") - 1, '/');\n \t\t\t}\n+\n+\t\t\tpath += repo->path_len;\n+\n+\t\t\tls->dentry_name = xmalloc(strlen(path) -\n+\t\t\t\t\t\t repo->path_len + 1);\n+\t\t\tstrcpy(ls->dentry_name, path + repo->path_len);\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n \t\t\tls->dentry_flags |= IS_DIR;\n \t\t}\n-- \n1.6.2\n"},{"id":"111140","messageId":"7vskkd651b.fsf@gitster.siamese.dyndns.org","threadId":"18825","inReplyTo":"1239562146-32133-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-12T19:33:20Z","receivedAt":"2009-04-12T19:33:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Korinskiy\" writes:\n\n> If the response from remote web-server have scp or other not http-like\n> scheme http-push can't go to change url, because DAV must work only\n> over HTTP (http and https scheme).\n>\n> Signed-off-by: Kirill A. Korinskiy \n> ---\n\nThanks. I was wondering about a few more things about the patch and the\noriginal code.\n\n> http-push.c | 19 ++++++++++---------\n> 1 files changed, 10 insertions(+), 9 deletions(-)\n>\n> diff --git a/http-push.c b/http-push.c\n> index feeb340..cce9ead 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1457,16 +1457,17 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n> \t\t\t}\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n> \t\t\tchar *path = ctx->cdata;\n> -\t\t\tif (*ctx->cdata == 'h') {\n> -\t\t\t\tpath = strstr(path, \"//\");\n> -\t\t\t\tif (path) {\n> -\t\t\t\t\tpath = strchr(path+2, '/');\n> -\t\t\t\t}\n> -\t\t\t}\n> -\t\t\tif (path) {\n> -\t\t\t\tpath += repo->path_len;\n> -\t\t\t\tls->dentry_name = xstrdup(path);\n\nThe original protects against an unexpected ctx->cdata such as\n\n\thttp://frotz\n\nthat does not have any slash after the method:// part (in which case it\ndoes not even set ls_dentry_name.\n\n\n> +\t\t\tif (!strcmp(ctx->cdata, \"http://\")) {\n> +\t\t\t\tpath = strchr(path + sizeof(\"http://\") - 1, '/');\n> +\t\t\t} else if (!strcmp(ctx->cdata, \"https://\")) {\n> +\t\t\t\tpath = strchr(path + sizeof(\"https://\") - 1, '/');\n\nI wonder what happens if the strchr() returns NULL for such a broken\nctx->cdata. Will it make strlen(path) later to segfault?\n\nBesides, obviously this patch was never tested; you meant prefixcmp, not\nstrcmp here, so path never becomes NULL here. Oh, and instead of\ncomparing with ctx->cdata it would be easier to compare against path.\n\n> \t\t\t}\n> +\n> +\t\t\tpath += repo->path_len;\n> +\n> +\t\t\tls->dentry_name = xmalloc(strlen(path) -\n> +\t\t\t\t\t\t repo->path_len + 1);\n> +\t\t\tstrcpy(ls->dentry_name, path + repo->path_len);\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n> \t\t\tls->dentry_flags |= IS_DIR;\n> \t\t}\n> -- \n> 1.6.2\n"},{"id":"111200","messageId":"vpq8wm4tser.fsf@bauges.imag.fr","threadId":"18825","inReplyTo":"1239562146-32133-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push.c: DAV must support olny http and https scheme","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-04-13T16:44:28Z","receivedAt":"2009-04-13T16:44:28Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\"Kirill A. Korinskiy\" writes:\n\n> Subject: [PATCH] http-push.c: DAV must support olny http and https scheme\n ^^\n\ns/olny/only/ if it's not too late.\n\n-- \nMatthieu\n"}]}