{"thread":{"id":"16806","subject":"[PATCH] handle_remote_ls_ctx can parsing href starting at http://","startedAt":"2008-12-20T11:23:33Z","lastAt":"2008-12-21T09:42:42Z","messageCount":2,"participants":["Kirill A. Korinskiy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"98428","messageId":"1229772213-11932-1-git-send-email-catap@catap.ru","threadId":"16806","inReplyTo":null,"subject":"[PATCH] handle_remote_ls_ctx can parsing href starting at http://","fromName":"Kirill A. Korinskiy","fromEmail":"catap@catap.ru","sentAt":"2008-12-20T11:23:33Z","receivedAt":"2008-12-20T11:23:33Z","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 | 16 ++++++++++------\n 1 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..abf0269e0fe20a43a6247fc7584dc2d58322a8fa 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,10 @@ 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 = strstr(ctx->cdata, remote->path);\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 +2208,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 +2241,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":"98482","messageId":"7vr641rhil.fsf@gitster.siamese.dyndns.org","threadId":"16806","inReplyTo":"1229772213-11932-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-21T09:42:42Z","receivedAt":"2008-12-21T09:42:42Z","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\nDo you mean \"the client should support both server-relative '/repo.git'\nand full 'http://localhost/repo.git'\", or \"the client should reject\n'/repo.git' and insist on full 'http://localhost/repo.git'\"? I am\nguessing the former but it is not quite clear. Where in 4918 is this\nspecified?\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\nThis is an interesting but confusing example.\n\nDo you mean the bug is:\n\n (1) the client asks PROPFIND /repo.git/;\n\n (2) the server gives http://localhost/repo.git/refs back;\n\n (3) the client incorrectly assumes that the response would start with\n /repo.git/ (e.g. \"/repo.git/refs\"), so strips 10 bytes from the\n beginning of this result and uses the remainder as the \"new\"\n information to dig deeper; i.e. \"alhost/repo.git/refs\";\n\n (4) the new part is appended to the original path and the client forms\n the next request \"PROPFIND /repo.git/alhost/repo.git/refs/\";\n\n (5) instead, the client should strip the proto://host part (if exists)\n and request \"PROPFIND /repo.git/refs/\".\n\n> @@ -1424,9 +1425,10 @@ 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 = strstr(ctx->cdata, remote->path);\n> +\t\t\tls->dentry_name = xmalloc(strlen(path) -\n> \t\t\t\t\t\t remote->path_len + 1);\n\nWhat if you are talking to http://repo.git/repo.git/? Doesn't this\nstrstr() misbehave? Instead, shouldn't you be checking if the response\nbegins with proto://host/ and stripping it iff so?\n"}]}