{"thread":{"id":"25783","subject":"[PATCH] Normalise directory names when pushing to some WebDAV servers","startedAt":"2010-11-19T00:06:03Z","lastAt":"2010-11-20T04:32:28Z","messageCount":15,"participants":["Gabriel Corona","Tay Ray Chuan","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156146","messageId":"1290125163-31065-1-git-send-email-gabriel.corona@enst-bretagne.fr","threadId":"25783","inReplyTo":null,"subject":"[PATCH] Normalise directory names when pushing to some WebDAV servers","fromName":"Gabriel Corona","fromEmail":"gabriel.corona@enst-bretagne.fr","sentAt":"2010-11-19T00:06:03Z","receivedAt":"2010-11-19T00:06:03Z","isPatch":true,"sender":{"key":"gabriel.corona@enst-bretagne.fr","avatar":null},"body":"Fix a bug when pushing to WebDAV servers which do not use a trailing\nslash for collection names. The previous implementation fails to see\nthat the requested resource \"refs/\" is the same resource as \"refs\"\nand loads every reference twice (once for refs/ and once for refs).\n\nThis implementation normalises every collection name by appending a\ntrailing slash if necessary.\n\nThis can be tested with old versions of Apache (such as the WebDAV\nserver of GMX, Apache 2.0.63).\n---\n http-push.c |   13 +++++++++++++\n 1 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex c9bcd11..aeb37ab 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1083,6 +1083,18 @@ static void process_ls_ref(struct remote_ls_ctx *ls)\n \t\tone_remote_ref(ls->dentry_name);\n }\n \n+static void normalize_dirname(char** name) {\n+\tchar* res;\n+\tint len = strlen(*name);\n+\tif((*name)[len-1]=='/') return;\n+\tres = malloc(len+2);\n+\tstrcpy(res, *name);\n+\tres[len]='/';\n+\tres[len+1]='\\0';\n+\tfree(*name);\n+\t*name = res;\n+}\n+\n static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n {\n \tstruct remote_ls_ctx *ls = (struct remote_ls_ctx *)ctx->userData;\n@@ -1090,6 +1102,7 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \tif (tag_closed) {\n \t\tif (!strcmp(ctx->name, DAV_PROPFIND_RESP) && ls->dentry_name) {\n \t\t\tif (ls->dentry_flags & IS_DIR) {\n+\t\t\t\tnormalize_dirname(&ls->dentry_name);\n \t\t\t\tif (ls->flags & PROCESS_DIRS) {\n \t\t\t\t\tls->userFunc(ls);\n \t\t\t\t}\n-- \n1.7.2.3\n"},{"id":"156150","messageId":"AANLkTikM8mNv+GiVyDWEJxsf0o3FYaaJj+jdYyiuXnov@mail.gmail.com","threadId":"25783","inReplyTo":"1290125163-31065-1-git-send-email-gabriel.corona@enst-bretagne.fr","subject":"Re: [PATCH] Normalise directory names when pushing to some WebDAV servers","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T04:02:38Z","receivedAt":"2010-11-19T04:02:38Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Fri, Nov 19, 2010 at 8:06 AM, Gabriel Corona\n<gabriel.corona@enst-bretagne.fr> wrote:\n>  http-push.c |   13 +++++++++++++\n>  1 files changed, 13 insertions(+), 0 deletions(-)\n>\n> diff --git a/http-push.c b/http-push.c\n> index c9bcd11..aeb37ab 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1083,6 +1083,18 @@ static void process_ls_ref(struct remote_ls_ctx *ls)\n>                one_remote_ref(ls->dentry_name);\n>  }\n>\n> +static void normalize_dirname(char** name) {\n> +       char* res;\n> +       int len = strlen(*name);\n> +       if((*name)[len-1]=='/') return;\n> +       res = malloc(len+2);\n> +       strcpy(res, *name);\n> +       res[len]='/';\n> +       res[len+1]='\\0';\n> +       free(*name);\n> +       *name = res;\n> +}\n> +\n>  static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n>  {\n>        struct remote_ls_ctx *ls = (struct remote_ls_ctx *)ctx->userData;\n> @@ -1090,6 +1102,7 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n>        if (tag_closed) {\n>                if (!strcmp(ctx->name, DAV_PROPFIND_RESP) && ls->dentry_name) {\n>                        if (ls->dentry_flags & IS_DIR) {\n> +                               normalize_dirname(&ls->dentry_name);\n>                                if (ls->flags & PROCESS_DIRS) {\n>                                        ls->userFunc(ls);\n>                                }\n> --\n\nHmm, we already have the logic to do this in end_url_with_slash(). It\nworks on strbufs though. How about this?\n\n-->8--\n\ndiff --git a/http-push.c b/http-push.c\nindex c9bcd11..645c2b5 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1086,10 +1086,15 @@ static void process_ls_ref(struct remote_ls_ctx *ls)\n static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n {\n        struct remote_ls_ctx *ls = (struct remote_ls_ctx *)ctx->userData;\n+       struct strbuf buf = STRBUF_INIT;\n\n        if (tag_closed) {\n                if (!strcmp(ctx->name, DAV_PROPFIND_RESP) && ls->dentry_name) {\n                        if (ls->dentry_flags & IS_DIR) {\n+                               end_url_with_slash(&buf, ls->dentry_name);\n+                               free(ls->dentry_name);\n+                               ls->dentry_name = strbuf_detach(&buf, NULL);\n+\n                                if (ls->flags & PROCESS_DIRS) {\n                                        ls->userFunc(ls);\n                                }\n\n-- \nCheers,\nRay Chuan\n"},{"id":"156151","messageId":"20101119051047.GA4201@burratino","threadId":"25783","inReplyTo":"AANLkTikM8mNv+GiVyDWEJxsf0o3FYaaJj+jdYyiuXnov@mail.gmail.com","subject":"Re: [PATCH] Normalise directory names when pushing to some WebDAV servers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-19T05:10:47Z","receivedAt":"2010-11-19T05:10:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1086,10 +1086,15 @@ static void process_ls_ref(struct remote_ls_ctx *ls)\n>  static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n>  {\n>         struct remote_ls_ctx *ls = (struct remote_ls_ctx *)ctx->userData;\n> +       struct strbuf buf = STRBUF_INIT;\n> \n>         if (tag_closed) {\n>                 if (!strcmp(ctx->name, DAV_PROPFIND_RESP) && ls->dentry_name) {\n>                         if (ls->dentry_flags & IS_DIR) {\n> +                               end_url_with_slash(&buf, ls->dentry_name);\n> +                               free(ls->dentry_name);\n> +                               ls->dentry_name = strbuf_detach(&buf, NULL);\n> +\n>                                 if (ls->flags & PROCESS_DIRS) {\n\nNit: might make sense to narrow the scope of buf, to avoid tempting\npeople to reuse or release it.\n\nThanks,\nJonathan\n"},{"id":"156172","messageId":"1290170790-2200-1-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"20101119051047.GA4201@burratino","subject":"[PATCH 0/6] http (dumb): end urls with slash","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:24Z","receivedAt":"2010-11-19T12:46:24Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This patch series contains Gabriel's fix, as well as some related work I did in\nthe area while looking through the code paths.\n\nContents:\n--\n[PATCH 1/6] shift end_url_with_slash() from http.[ch] to url.[ch]\n[PATCH 2/6] url: add str wrapper for end_url_with_slash()\n[PATCH 3/6] http-push: Normalise directory names when pushing to some WebDAV servers\n[PATCH 4/6] http-backend: use end_url_with_slash()\n[PATCH 5/6] http-push: check path length before using it\n[PATCH 6/6] http-push: add trailing slash at arg-parse time, instead of later on\n\n--\n1.7.3.67.g2a10b\n"},{"id":"156173","messageId":"1290170790-2200-2-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/6] shift end_url_with_slash() from http.[ch] to url.[ch]","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:25Z","receivedAt":"2010-11-19T12:46:25Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This allows non-http/curl users to access it too (eg. http-backend.c).\n\nUpdate include headers in end_url_with_slash() users too.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Makefile |    2 +-\n http.c   |    8 +-------\n http.h   |    1 -\n url.c    |    7 +++++++\n url.h    |    2 ++\n 5 files changed, 11 insertions(+), 9 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ca1d1fe..e02b0f4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1922,7 +1922,7 @@ builtin/tar-tree.o archive-tar.o: tar.h\n builtin/pack-objects.o: thread-utils.h\n connect.o transport.o http-backend.o: url.h\n http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h\n-http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h\n+http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h\n \n xdiff-interface.o $(XDIFF_OBJS): \\\n \txdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \\\ndiff --git a/http.c b/http.c\nindex 17bcf19..310dd86 100644\n--- a/http.c\n+++ b/http.c\n@@ -2,6 +2,7 @@\n #include \"pack.h\"\n #include \"sideband.h\"\n #include \"run-command.h\"\n+#include \"url.h\"\n \n int data_received;\n int active_requests;\n@@ -733,13 +734,6 @@ static inline int hex(int v)\n \t\treturn 'A' + v - 10;\n }\n \n-void end_url_with_slash(struct strbuf *buf, const char *url)\n-{\n-\tstrbuf_addstr(buf, url);\n-\tif (buf->len && buf->buf[buf->len - 1] != '/')\n-\t\tstrbuf_addstr(buf, \"/\");\n-}\n-\n static char *quote_ref_url(const char *base, const char *ref)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/http.h b/http.h\nindex 173f74c..10de0a5 100644\n--- a/http.h\n+++ b/http.h\n@@ -117,7 +117,6 @@ extern void append_remote_object_url(struct strbuf *buf, const char *url,\n \t\t\t\t     int only_two_digit_prefix);\n extern char *get_remote_object_url(const char *url, const char *hex,\n \t\t\t\t   int only_two_digit_prefix);\n-extern void end_url_with_slash(struct strbuf *buf, const char *url);\n \n /* Options for http_request_*() */\n #define HTTP_NO_CACHE\t\t1\ndiff --git a/url.c b/url.c\nindex cd8f74f..7cebc64 100644\n--- a/url.c\n+++ b/url.c\n@@ -125,3 +125,10 @@ char *url_decode_parameter_value(const char **query)\n \tstruct strbuf out = STRBUF_INIT;\n \treturn url_decode_internal(query, \"&\", &out, 1);\n }\n+\n+void end_url_with_slash(struct strbuf *buf, const char *url)\n+{\n+\tstrbuf_addstr(buf, url);\n+\tif (buf->len && buf->buf[buf->len - 1] != '/')\n+\t\tstrbuf_addstr(buf, \"/\");\n+}\ndiff --git a/url.h b/url.h\nindex 15817f8..8cb74d4 100644\n--- a/url.h\n+++ b/url.h\n@@ -7,4 +7,6 @@ extern char *url_decode(const char *url);\n extern char *url_decode_parameter_name(const char **query);\n extern char *url_decode_parameter_value(const char **query);\n \n+extern void end_url_with_slash(struct strbuf *buf, const char *url);\n+\n #endif /* URL_H */\n-- \n1.7.3.67.g2a10b\n"},{"id":"156174","messageId":"1290170790-2200-3-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/6] url: add str wrapper for end_url_with_slash()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:26Z","receivedAt":"2010-11-19T12:46:26Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n url.c |    8 ++++++++\n url.h |    1 +\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/url.c b/url.c\nindex 7cebc64..138396c 100644\n--- a/url.c\n+++ b/url.c\n@@ -132,3 +132,11 @@ void end_url_with_slash(struct strbuf *buf, const char *url)\n \tif (buf->len && buf->buf[buf->len - 1] != '/')\n \t\tstrbuf_addstr(buf, \"/\");\n }\n+\n+void str_end_url_with_slash(const char *url, char **dest) {\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tend_url_with_slash(&buf, url);\n+\tif (dest)\n+\t\tfree(*dest);\n+\t*dest = strbuf_detach(&buf, NULL);\n+}\ndiff --git a/url.h b/url.h\nindex 8cb74d4..7100e32 100644\n--- a/url.h\n+++ b/url.h\n@@ -8,5 +8,6 @@ extern char *url_decode_parameter_name(const char **query);\n extern char *url_decode_parameter_value(const char **query);\n \n extern void end_url_with_slash(struct strbuf *buf, const char *url);\n+extern void str_end_url_with_slash(const char *url, char **dest);\n \n #endif /* URL_H */\n-- \n1.7.3.67.g2a10b\n"},{"id":"156177","messageId":"1290170790-2200-4-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-3-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/6] http-push: Normalise directory names when pushing to some WebDAV servers","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:27Z","receivedAt":"2010-11-19T12:46:27Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Fix a bug when pushing to WebDAV servers which do not use a trailing\nslash for collection names. The previous implementation fails to see\nthat the requested resource \"refs/\" is the same resource as \"refs\"\nand loads every reference twice (once for refs/ and once for refs).\n\nThis implementation normalises every collection name by appending a\ntrailing slash if necessary.\n\nThis can be tested with old versions of Apache (such as the WebDAV\nserver of GMX, Apache 2.0.63).\n\nBased-on-patch-by: Gabriel Corona <gabriel.corona@enst-bretagne.fr>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex c9bcd11..565e580 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1090,6 +1090,10 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \tif (tag_closed) {\n \t\tif (!strcmp(ctx->name, DAV_PROPFIND_RESP) && ls->dentry_name) {\n \t\t\tif (ls->dentry_flags & IS_DIR) {\n+\n+\t\t\t\t/* ensure collection names end with slash */\n+\t\t\t\tstr_end_url_with_slash(ls->dentry_name, &ls->dentry_name);\n+\n \t\t\t\tif (ls->flags & PROCESS_DIRS) {\n \t\t\t\t\tls->userFunc(ls);\n \t\t\t\t}\n-- \n1.7.3.67.g2a10b\n"},{"id":"156175","messageId":"1290170790-2200-5-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-4-git-send-email-rctay89@gmail.com","subject":"[PATCH 4/6] http-backend: use end_url_with_slash()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:28Z","receivedAt":"2010-11-19T12:46:28Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-backend.c |    4 +---\n 1 files changed, 1 insertions(+), 3 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex 14c90c2..8501504 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -510,9 +510,7 @@ static char* getdir(void)\n \t\t\tdie(\"GIT_PROJECT_ROOT is set but PATH_INFO is not\");\n \t\tif (daemon_avoid_alias(pathinfo))\n \t\t\tdie(\"'%s': aliased\", pathinfo);\n-\t\tstrbuf_addstr(&buf, root);\n-\t\tif (buf.buf[buf.len - 1] != '/')\n-\t\t\tstrbuf_addch(&buf, '/');\n+\t\tend_url_with_slash(&buf, root);\n \t\tif (pathinfo[0] == '/')\n \t\t\tpathinfo++;\n \t\tstrbuf_addstr(&buf, pathinfo);\n-- \n1.7.3.67.g2a10b\n"},{"id":"156176","messageId":"1290170790-2200-6-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-5-git-send-email-rctay89@gmail.com","subject":"[PATCH 5/6] http-push: check path length before using it","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:29Z","receivedAt":"2010-11-19T12:46:29Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"We use path_len to skip the base url/path, but we do not know for sure\nif path does indeed contain the base url/path. Check if this is so.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c |   10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 565e580..1c34b00 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1116,8 +1116,14 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\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\t\tif (strncmp(path,\n+\t\t\t\t\t    (repo->path ? repo->path : repo->url),\n+\t\t\t\t\t    repo->path_len) == 0) {\n+\t\t\t\t\tpath += repo->path_len;\n+\t\t\t\t\tls->dentry_name = xstrdup(path);\n+\t\t\t\t} else\n+\t\t\t\t\terror(\"Parsed path '%s' does not match url: '%s'\\n\",\n+\t\t\t\t\t      path, (repo->path ? repo->path : repo->url));\n \t\t\t}\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n \t\t\tls->dentry_flags |= IS_DIR;\n-- \n1.7.3.67.g2a10b\n"},{"id":"156178","messageId":"1290170790-2200-7-git-send-email-rctay89@gmail.com","threadId":"25783","inReplyTo":"1290170790-2200-6-git-send-email-rctay89@gmail.com","subject":"[PATCH 6/6] http-push: add trailing slash at arg-parse time, instead of later on","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-19T12:46:30Z","receivedAt":"2010-11-19T12:46:30Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"That way, we don't have to update repo->path and repo->path_len again\nafter adding the trailing slash.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c |   15 ++-------------\n 1 files changed, 2 insertions(+), 13 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 1c34b00..a10891e 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1799,7 +1799,6 @@ int main(int argc, char **argv)\n \tint new_refs;\n \tstruct ref *ref, *local_refs;\n \tstruct remote *remote;\n-\tchar *rewritten_url = NULL;\n \n \tgit_extract_argv0_path(argv[0]);\n \n@@ -1845,8 +1844,8 @@ int main(int argc, char **argv)\n \t\t}\n \t\tif (!repo->url) {\n \t\t\tchar *path = strstr(arg, \"//\");\n-\t\t\trepo->url = arg;\n-\t\t\trepo->path_len = strlen(arg);\n+\t\t\tstr_end_url_with_slash(arg, &repo->url);\n+\t\t\trepo->path_len = strlen(repo->url);\n \t\t\tif (path) {\n \t\t\t\trepo->path = strchr(path+2, '/');\n \t\t\t\tif (repo->path)\n@@ -1882,15 +1881,6 @@ int main(int argc, char **argv)\n \tremote->url[remote->url_nr++] = repo->url;\n \thttp_init(remote);\n \n-\tif (repo->url && repo->url[strlen(repo->url)-1] != '/') {\n-\t\trewritten_url = xmalloc(strlen(repo->url)+2);\n-\t\tstrcpy(rewritten_url, repo->url);\n-\t\tstrcat(rewritten_url, \"/\");\n-\t\trepo->path = rewritten_url + (repo->path - repo->url);\n-\t\trepo->path_len++;\n-\t\trepo->url = rewritten_url;\n-\t}\n-\n #ifdef USE_CURL_MULTI\n \tis_running_queue = 0;\n #endif\n@@ -2098,7 +2088,6 @@ int main(int argc, char **argv)\n \t}\n \n  cleanup:\n-\tfree(rewritten_url);\n \tif (info_ref_lock)\n \t\tunlock_remote(info_ref_lock);\n \tfree(repo);\n-- \n1.7.3.67.g2a10b\n"},{"id":"156200","messageId":"20101119183339.GB26187@burratino","threadId":"25783","inReplyTo":"1290170790-2200-6-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 5/6] http-push: check path length before using it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-19T18:33:39Z","receivedAt":"2010-11-19T18:33:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tay Ray Chuan wrote:\n\n> We use path_len to skip the base url/path, but we do not know for sure\n> if path does indeed contain the base url/path. Check if this is so.\n[...]\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1116,8 +1116,14 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\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\t\tif (strncmp(path,\n> +\t\t\t\t\t    (repo->path ? repo->path : repo->url),\n> +\t\t\t\t\t    repo->path_len) == 0) {\n\nNits: the usual idiom is\n\n if (!strncmp(path, ...\n\nto make it is obvious from looking at the left side that this is a\ntest for equality.  The parentheses around the second parameter are\nnot needed here...\n\n\n> +\t\t\t\t\tpath += repo->path_len;\n> +\t\t\t\t\tls->dentry_name = xstrdup(path);\n> +\t\t\t\t} else\n> +\t\t\t\t\terror(\"Parsed path '%s' does not match url: '%s'\\n\",\n> +\t\t\t\t\t      path, (repo->path ? repo->path : repo->url));\n\n... or here.  So perhaps:\n\n\t\t\tif (path) {\n\t\t\t\tconst char *url = repo->path;\n\t\t\t\tif (!url)\n\t\t\t\t\turl = repo->url;\n\t\t\t\tif (strncmp(path, url, repo->path_len)) {\n\t\t\t\t\terror(\"Parsed path '%s' does not match url: '%s'\\n\",\n\t\t\t\t\t\tpath, url);\n\t\t\t\t} else {\n\t\t\t\t\tpath += ...\n\nBy the way, is the error behavior correct?  This prints a message with\n\"error: \" and then continues anyway.\n"},{"id":"156201","messageId":"20101119191810.GD26187@burratino","threadId":"25783","inReplyTo":"1290170790-2200-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 0/6] http (dumb): end urls with slash","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-19T19:18:10Z","receivedAt":"2010-11-19T19:18:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ray Chuan,\n\nTay Ray Chuan wrote:\n\n> This patch series contains Gabriel's fix, as well as some related work I did in\n> the area while looking through the code paths.\n\nThanks!  The code is quite readable.  I don't know enough to say\nanything about the functional changes, but I assume you have that\ncovered.\n"},{"id":"156220","messageId":"7vd3q0c4b3.fsf@alter.siamese.dyndns.org","threadId":"25783","inReplyTo":"1290170790-2200-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 0/6] http (dumb): end urls with slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-19T22:54:40Z","receivedAt":"2010-11-19T22:54:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Didn't find anything suspicious during my cursory read (except for the\nones Jonathan pointed out, which I missed), but how does this series\nrelate to your earlier commits around d8fab07 (remote-curl: ensure that\nURLs have a trailing slash, 2010-04-08)?  Same topic, tying loose ends?\nSame topic, different approach?  Unrelated changes with similar-sounding\nnames?\n"},{"id":"156246","messageId":"AANLkTikJz+SigzqFdeP+rZR3FS06U1exDb59AsRFTQAj@mail.gmail.com","threadId":"25783","inReplyTo":"20101119183339.GB26187@burratino","subject":"Re: [PATCH 5/6] http-push: check path length before using it","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-20T04:28:51Z","receivedAt":"2010-11-20T04:28:51Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, Nov 20, 2010 at 2:33 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> [snip]\n> ... or here.  So perhaps:\n>\n>                        if (path) {\n>                                const char *url = repo->path;\n>                                if (!url)\n>                                        url = repo->url;\n>                                if (strncmp(path, url, repo->path_len)) {\n>                                        error(\"Parsed path '%s' does not match url: '%s'\\n\",\n>                                                path, url);\n>                                } else {\n>                                        path += ...\n\nLooks much cleaner, thanks.\n\n> By the way, is the error behavior correct?  This prints a message with\n> \"error: \" and then continues anyway.\n\nYeah, this fits in with the exiting xml code paths - they just\nindicate errors and continue.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"156247","messageId":"AANLkTimWz4meHUb6o0K8yNMmqyqpWbKrq0OmnzWAS300@mail.gmail.com","threadId":"25783","inReplyTo":"7vd3q0c4b3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/6] http (dumb): end urls with slash","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-11-20T04:32:28Z","receivedAt":"2010-11-20T04:32:28Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, Nov 20, 2010 at 6:54 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Didn't find anything suspicious during my cursory read (except for the\n> ones Jonathan pointed out, which I missed), but how does this series\n> relate to your earlier commits around d8fab07 (remote-curl: ensure that\n> URLs have a trailing slash, 2010-04-08)?  Same topic, tying loose ends?\n> Same topic, different approach?  Unrelated changes with similar-sounding\n> names?\n\nJohnathan and Junio, thanks for taking the time.\n\nSorry for the confusion. Well, it's all about slashes, hence the\ntitle, but there are some refactors and related work in the http area\nin this series.\n\n-- \nCheers,\nRay Chuan\n"}]}