{"thread":{"id":"16843","subject":"[PATCH] handle_remote_ls_ctx can parsing href starting at http://","startedAt":"2008-12-23T08:31:15Z","lastAt":"2009-01-02T07:26:45Z","messageCount":7,"participants":["Kirill A. Korinskiy","Junio C Hamano","Mike Hommey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"98593","messageId":"1230021075-10113-1-git-send-email-catap@catap.ru","threadId":"16843","inReplyTo":"7v3aghnv1t.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] handle_remote_ls_ctx can parsing href starting at http://","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2008-12-23T08:31:15Z","receivedAt":"2008-12-23T08:31:15Z","isPatch":true,"sender":{"key":"catap@catap.ru","avatar":"https://gravatar.com/avatar/ea0ab2c29579606bd684eccdf786c666f4425bedc4e599b698a175f12737b1c5?d=mp&s=160"},"body":"The program call remote_ls() to get remote objects over http;\nhandle_remote_ls_ctx() is used to parse it's response to populated\n\"struct remote_ls_ctx\" that is returned from remote_ls().\n\nThe handle_remote_ls_ctx() function assumed that the server will\nreturned local path in href field, but RFC 4918 demand of support full\nURI (http://localhost/repo.git for example).\n\nThis resulted in push failure (git-http-push ask server\nPROPFIND /repo.git/alhost:8080/repo.git/refs/) when a server returned\nfull URI.\n\nSigned-off-by: Kirill A. Korinskiy \n---\n http-push.c | 25 +++++++++++++++++++------\n 1 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..a4b7d08663504a57008f66a39fffe293f62c1d08 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -87,6 +87,7 @@ static struct object_list *objects;\n struct repo\n {\n \tchar *url;\n+\tchar *path;\n \tint path_len;\n \tint has_info_refs;\n \tint can_update_info_refs;\n@@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t\tls->userFunc(ls);\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n-\t\t\tls->dentry_name = xmalloc(strlen(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 += remote->path_len;\n+\t\t\t}\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, ctx->cdata + remote->path_len);\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@@ -2206,10 +2217,11 @@ int main(int argc, char **argv)\n \t\tif (!remote->url) {\n \t\t\tchar *path = strstr(arg, \"//\");\n \t\t\tremote->url = arg;\n+\t\t\tremote->path_len = strlen(arg);\n \t\t\tif (path) {\n-\t\t\t\tpath = strchr(path+2, '/');\n-\t\t\t\tif (path)\n-\t\t\t\t\tremote->path_len = strlen(path);\n+\t\t\t\tremote->path = strchr(path+2, '/');\n+\t\t\t\tif (remote->path)\n+\t\t\t\t\tremote->path_len = strlen(remote->path);\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n@@ -2238,8 +2250,9 @@ int main(int argc, char **argv)\n \t\trewritten_url = xmalloc(strlen(remote->url)+2);\n \t\tstrcpy(rewritten_url, remote->url);\n \t\tstrcat(rewritten_url, \"/\");\n+\t\tremote->path = rewritten_url + (remote->path - remote->url);\n+\t\tremote->path_len++;\n \t\tremote->url = rewritten_url;\n-\t\t++remote->path_len;\n \t}\n \n \t/* Verify DAV compliance/lock support */\n-- \n1.5.6.5\n"},{"id":"98683","messageId":"7vmyekag6p.fsf@gitster.siamese.dyndns.org","threadId":"16843","inReplyTo":"1230021075-10113-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] handle_remote_ls_ctx can parsing href starting at http://","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-25T07:04:46Z","receivedAt":"2008-12-25T07:04:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Korinskiy\" writes:\n\n> The program call remote_ls() to get remote objects over http;\n> handle_remote_ls_ctx() is used to parse it's response to populated\n> \"struct remote_ls_ctx\" that is returned from remote_ls().\n>\n> The handle_remote_ls_ctx() function assumed that the server will\n> returned local path in href field, but RFC 4918 demand of support full\n> URI (http://localhost/repo.git for example).\n>\n> This resulted in push failure (git-http-push ask server\n> PROPFIND /repo.git/alhost:8080/repo.git/refs/) when a server returned\n> full URI.\n\nThanks.\n\nDo you mean PROPFIND was made to that garbage with :8080 in it when the\nserver returned a full URI http://localhost/repo.git as in the example in\nthe previous paragraph, or are you using a different example here?\n\nI am contemplating of munging your commit log message like this...\n\ncommit e1f33efe07b9a520505fccd71bea1292fc9448dd\nAuthor: Kirill A. Korinskiy \nDate: Tue Dec 23 11:31:15 2008 +0300\n\n http-push: support full URI in handle_remote_ls_ctx()\n \n The program calls remote_ls() to get list of files from the server over\n HTTP; handle_remote_ls_ctx() is used to parse its response to populate\n \"struct remote_ls_ctx\" that is returned from remote_ls().\n \n The handle_remote_ls_ctx() function assumed that the server returns a\n local path in href field, but RFC 4918 (14.7) demand of support full URI\n (e.g. \"http://localhost:8080/repo.git\").\n \n This resulted in push failure (e.g. git-http-push issues a PROPFIND\n request to \"/repo.git/alhost:8080/repo.git/refs/\" to the server).\n \n Signed-off-by: Kirill A. Korinskiy \n Signed-off-by: Junio C Hamano \n"},{"id":"98856","messageId":"1230517935-11299-1-git-send-email-catap@catap.ru","threadId":"16843","inReplyTo":"7vmyekag6p.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] http-push: support full URI in handle_remote_ls_ctx()","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2008-12-29T02:32:15Z","receivedAt":"2008-12-29T02:32:15Z","isPatch":true,"sender":{"key":"catap@catap.ru","avatar":"https://gravatar.com/avatar/ea0ab2c29579606bd684eccdf786c666f4425bedc4e599b698a175f12737b1c5?d=mp&s=160"},"body":"The program calls remote_ls() to get list of files from the server\nover HTTP; handle_remote_ls_ctx() is used to parse its response to\npopulate \"struct remote_ls_ctx\" that is returned from remote_ls().\n\nThe handle_remote_ls_ctx() function assumed that the server returns a\nlocal path in href field, but RFC 4918 (14.7) demand of support full\nURI (e.g. \"http://localhost:8080/repo.git\").\n\nThis resulted in push failure (e.g. git-http-push issues a PROPFIND\nrequest to \"/repo.git/alhost:8080/repo.git/refs/\" to the server).\n\nSigned-off-by: Kirill A. Korinskiy \n---\n http-push.c | 25 +++++++++++++++++++------\n 1 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..a4b7d08663504a57008f66a39fffe293f62c1d08 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -87,6 +87,7 @@ static struct object_list *objects;\n struct repo\n {\n \tchar *url;\n+\tchar *path;\n \tint path_len;\n \tint has_info_refs;\n \tint can_update_info_refs;\n@@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t\tls->userFunc(ls);\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n-\t\t\tls->dentry_name = xmalloc(strlen(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 += remote->path_len;\n+\t\t\t}\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, ctx->cdata + remote->path_len);\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@@ -2206,10 +2217,11 @@ int main(int argc, char **argv)\n \t\tif (!remote->url) {\n \t\t\tchar *path = strstr(arg, \"//\");\n \t\t\tremote->url = arg;\n+\t\t\tremote->path_len = strlen(arg);\n \t\t\tif (path) {\n-\t\t\t\tpath = strchr(path+2, '/');\n-\t\t\t\tif (path)\n-\t\t\t\t\tremote->path_len = strlen(path);\n+\t\t\t\tremote->path = strchr(path+2, '/');\n+\t\t\t\tif (remote->path)\n+\t\t\t\t\tremote->path_len = strlen(remote->path);\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n@@ -2238,8 +2250,9 @@ int main(int argc, char **argv)\n \t\trewritten_url = xmalloc(strlen(remote->url)+2);\n \t\tstrcpy(rewritten_url, remote->url);\n \t\tstrcat(rewritten_url, \"/\");\n+\t\tremote->path = rewritten_url + (remote->path - remote->url);\n+\t\tremote->path_len++;\n \t\tremote->url = rewritten_url;\n-\t\t++remote->path_len;\n \t}\n \n \t/* Verify DAV compliance/lock support */\n-- \n1.5.6.5\n"},{"id":"98864","messageId":"20081229071710.GA19175@glandium.org","threadId":"16843","inReplyTo":"1230517935-11299-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-12-29T07:17:10Z","receivedAt":"2008-12-29T07:17:10Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, Dec 29, 2008 at 05:32:15AM +0300, Kirill A. Korinskiy wrote:\n> The program calls remote_ls() to get list of files from the server\n> over HTTP; handle_remote_ls_ctx() is used to parse its response to\n> populate \"struct remote_ls_ctx\" that is returned from remote_ls().\n> \n> The handle_remote_ls_ctx() function assumed that the server returns a\n> local path in href field, but RFC 4918 (14.7) demand of support full\n> URI (e.g. \"http://localhost:8080/repo.git\").\n> \n> This resulted in push failure (e.g. git-http-push issues a PROPFIND\n> request to \"/repo.git/alhost:8080/repo.git/refs/\" to the server).\n> \n> Signed-off-by: Kirill A. Korinskiy \n> ---\n> http-push.c | 25 +++++++++++++++++++------\n> 1 files changed, 19 insertions(+), 6 deletions(-)\n> \n> diff --git a/http-push.c b/http-push.c\n> index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..a4b7d08663504a57008f66a39fffe293f62c1d08 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -87,6 +87,7 @@ static struct object_list *objects;\n> struct repo\n> {\n> \tchar *url;\n> +\tchar *path;\n> \tint path_len;\n> \tint has_info_refs;\n> \tint can_update_info_refs;\n> @@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n> \t\t\t\tls->userFunc(ls);\n> \t\t\t}\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n> -\t\t\tls->dentry_name = xmalloc(strlen(ctx->cdata) -\n> +\t\t\tchar *path = ctx->cdata;\n> +\t\t\tif (*ctx->cdata == 'h') {\n> +\t\t\t\tpath = strstr(path, \"//\");\n\nI would make that a \"://\"\n\n> +\t\t\t\tif (path) {\n> +\t\t\t\t\tpath = strchr(path+2, '/');\n\nand s/2/3/, accordingly.\n\nBut I realize the existing code already does something like what your\nare doing...\n\nMike\n"},{"id":"98868","messageId":"7viqp3e5ta.fsf@gitster.siamese.dyndns.org","threadId":"16843","inReplyTo":"1230517935-11299-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-29T08:36:33Z","receivedAt":"2008-12-29T08:36:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; already applied ;-)\n"},{"id":"99147","messageId":"1230879195-8567-1-git-send-email-catap@catap.ru","threadId":"16843","inReplyTo":"20081229071710.GA19175@glandium.org","subject":"[PATCH] http-push: support full URI in handle_remote_ls_ctx()","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2009-01-02T06:53:15Z","receivedAt":"2009-01-02T06:53:15Z","isPatch":true,"sender":{"key":"catap@catap.ru","avatar":"https://gravatar.com/avatar/ea0ab2c29579606bd684eccdf786c666f4425bedc4e599b698a175f12737b1c5?d=mp&s=160"},"body":"The program calls remote_ls() to get list of files from the server\nover HTTP; handle_remote_ls_ctx() is used to parse its response to\npopulate \"struct remote_ls_ctx\" that is returned from remote_ls().\n\nThe handle_remote_ls_ctx() function assumed that the server returns a\nlocal path in href field, but RFC 4918 (14.7) demand of support full\nURI (e.g. \"http://localhost:8080/repo.git\").\n\nThis resulted in push failure (e.g. git-http-push issues a PROPFIND\nrequest to \"/repo.git/alhost:8080/repo.git/refs/\" to the server).\n\nSigned-off-by: Kirill A. Korinskiy \n---\n http-push.c | 25 +++++++++++++++++++------\n 1 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..d1749fe2ffd6a59a4eea514997a62b5d7c80b438 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -87,6 +87,7 @@ static struct object_list *objects;\n struct repo\n {\n \tchar *url;\n+\tchar *path;\n \tint path_len;\n \tint has_info_refs;\n \tint can_update_info_refs;\n@@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t\tls->userFunc(ls);\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n-\t\t\tls->dentry_name = xmalloc(strlen(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+3, '/');\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (path) {\n+\t\t\t\tpath += remote->path_len;\n+\t\t\t}\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, ctx->cdata + remote->path_len);\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@@ -2206,10 +2217,11 @@ int main(int argc, char **argv)\n \t\tif (!remote->url) {\n \t\t\tchar *path = strstr(arg, \"//\");\n \t\t\tremote->url = arg;\n+\t\t\tremote->path_len = strlen(arg);\n \t\t\tif (path) {\n-\t\t\t\tpath = strchr(path+2, '/');\n-\t\t\t\tif (path)\n-\t\t\t\t\tremote->path_len = strlen(path);\n+\t\t\t\tremote->path = strchr(path+2, '/');\n+\t\t\t\tif (remote->path)\n+\t\t\t\t\tremote->path_len = strlen(remote->path);\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n@@ -2238,8 +2250,9 @@ int main(int argc, char **argv)\n \t\trewritten_url = xmalloc(strlen(remote->url)+2);\n \t\tstrcpy(rewritten_url, remote->url);\n \t\tstrcat(rewritten_url, \"/\");\n+\t\tremote->path = rewritten_url + (remote->path - remote->url);\n+\t\tremote->path_len++;\n \t\tremote->url = rewritten_url;\n-\t\t++remote->path_len;\n \t}\n \n \t/* Verify DAV compliance/lock support */\n-- \n1.5.6.5\n"},{"id":"99149","messageId":"7v7i5edv7u.fsf@gitster.siamese.dyndns.org","threadId":"16843","inReplyTo":"1230879195-8567-1-git-send-email-catap@catap.ru","subject":"Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-02T07:26:45Z","receivedAt":"2009-01-02T07:26:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kirill A. Korinskiy\" writes:\n\n> @@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n> \t\t\t\tls->userFunc(ls);\n> \t\t\t}\n> \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {\n> -\t\t\tls->dentry_name = xmalloc(strlen(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+3, '/');\n> +\t\t\t\t}\n> +\t\t\t}\n\nIs this \"://\" (and +3) the only change from the previous one that has\nalready been queued? I didn't have a problem with the old \"//\" one.\n\nThe check to see if it begins with 'h' bothers me much much more.\n\nIf you want to be defensively tight, you should be checking if it begins\nwith either \"http://\" or \"https://\", the only two protocols you are\nprepared to handle, and nothing else, so that you won't trigger this\ncodepath when the other end gave you \"hqrt://..\", on the basis that your\ncode won't know if hqrt:// protocol works the same way as http and https.\n\nOn the other hand, if you want to be optimistically loose, expecting\nwhatever people would implement that can be handled with the existing DAV\ncode would behave the same way as http and https, you shouldn't be\nlimiting yourself to an unknown protocol name that happens to begin with\nan 'h', only accepting \"hqrt://\" but not \"ittp://\" URLs.\n\nYour \"first byte of the protocol name must be 'h'\" does not do either.\n"}]}