{"thread":{"id":"35430","subject":"[PATCH v7 09/10] connect.c: Refactor url parsing","startedAt":"2013-11-28T19:50:03Z","lastAt":"2013-11-28T19:50:03Z","messageCount":1,"participants":["Torsten Bögershausen"],"isPatch":true,"patchVersion":7,"patchTotal":10},"messages":[{"id":"231260","messageId":"201311282050.04266.tboegi@web.de","threadId":"35430","inReplyTo":null,"subject":"[PATCH v7 09/10] connect.c: Refactor url parsing","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-28T19:50:03Z","receivedAt":"2013-11-28T19:50:03Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Make the function is_local() from transport.c public, rename it into\nurl_is_local_not_ssh() and use it in both transport.c and connect.c\n\nUse a protocol \"local\" for URLs for the local file system.\n\nOne note about using file:// under Windows:\nThe (absolute) path on Unix like system typically starts with \"/\".\nWhen the host is empty, it can be omitted, so that a shell scriptlet\nurl=file://$pwd\nwill give a URL like \"file:///home/user/repo\".\n\nWindows does not have the same concept of a root directory located in \"/\".\nWhen parsing the URL allow \"file://C:/user/repo\"\n(even if RFC1738 indicates that \"file:///C:/user/repo\" should be used).\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n connect.c             | 57 +++++++++++++++++++++++++++------------------------\n connect.h             |  1 +\n t/t5500-fetch-pack.sh |  7 +++++++\n t/t5601-clone.sh      |  8 ++++++++\n transport.c           | 12 ++---------\n 5 files changed, 48 insertions(+), 37 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 7874530..04093c4 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -232,14 +232,24 @@ int server_supports(const char *feature)\n \n enum protocol {\n \tPROTO_LOCAL = 1,\n+\tPROTO_FILE,\n \tPROTO_SSH,\n \tPROTO_GIT\n };\n \n+int url_is_local_not_ssh(const char *url)\n+{\n+\tconst char *colon = strchr(url, ':');\n+\tconst char *slash = strchr(url, '/');\n+\treturn !colon || (slash && slash < colon) ||\n+\t\thas_dos_drive_prefix(url);\n+}\n+\n static const char *prot_name(enum protocol protocol)\n {\n \tswitch (protocol) {\n \t\tcase PROTO_LOCAL:\n+\t\tcase PROTO_FILE:\n \t\t\treturn \"file\";\n \t\tcase PROTO_SSH:\n \t\t\treturn \"ssh\";\n@@ -261,7 +271,7 @@ static enum protocol get_protocol(const char *name)\n \tif (!strcmp(name, \"ssh+git\"))\n \t\treturn PROTO_SSH;\n \tif (!strcmp(name, \"file\"))\n-\t\treturn PROTO_LOCAL;\n+\t\treturn PROTO_FILE;\n \tdie(\"I don't handle protocol '%s'\", name);\n }\n \n@@ -564,9 +574,8 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \tchar *url;\n \tchar *host, *path;\n \tchar *end;\n-\tint separator;\n+\tint separator = '/';\n \tenum protocol protocol = PROTO_LOCAL;\n-\tint free_path = 0;\n \n \tif (is_url(url_orig))\n \t\turl = url_decode(url_orig);\n@@ -578,10 +587,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t\t*host = '\\0';\n \t\tprotocol = get_protocol(url);\n \t\thost += 3;\n-\t\tseparator = '/';\n \t} else {\n \t\thost = url;\n-\t\tseparator = ':';\n+\t\tif (!url_is_local_not_ssh(url)) {\n+\t\t\tprotocol = PROTO_SSH;\n+\t\t\tseparator = ':';\n+\t\t}\n \t}\n \n \t/*\n@@ -597,17 +608,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t} else\n \t\tend = host;\n \n-\tpath = strchr(end, separator);\n-\tif (path && !has_dos_drive_prefix(end)) {\n-\t\tif (separator == ':') {\n-\t\t\tif (host != url || path < strchrnul(host, '/')) {\n-\t\t\t\tprotocol = PROTO_SSH;\n-\t\t\t\t*path++ = '\\0';\n-\t\t\t} else /* '/' in the host part, assume local path */\n-\t\t\t\tpath = end;\n-\t\t}\n-\t} else\n+\tif (protocol == PROTO_LOCAL)\n \t\tpath = end;\n+\telse if (protocol == PROTO_FILE && has_dos_drive_prefix(end))\n+\t\tpath = end; /* \"file://$(pwd)\" may be \"file://C:/projects/repo\" */\n+\telse\n+\t\tpath = strchr(end, separator);\n \n \tif (!path || !*path)\n \t\tdie(\"No path specified. See 'man git-pull' for valid url syntax\");\n@@ -616,23 +622,20 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t * null-terminate hostname and point path to ~ for URL's like this:\n \t *    ssh://host.xz/~user/repo\n \t */\n-\tif (protocol != PROTO_LOCAL) {\n-\t\tchar *ptr = path;\n+\n+\tend = path; /* Need to \\0 terminate host here */\n+\tif (separator == ':')\n+\t\tpath++; /* path starts after ':' */\n+\tif (protocol == PROTO_GIT || protocol == PROTO_SSH) {\n \t\tif (path[1] == '~')\n \t\t\tpath++;\n-\t\telse {\n-\t\t\tpath = xstrdup(ptr);\n-\t\t\tfree_path = 1;\n-\t\t}\n-\n-\t\t*ptr = '\\0';\n \t}\n \n+\tpath = xstrdup(path);\n+\t*end = '\\0';\n+\n \t*ret_host = xstrdup(host);\n-\tif (free_path)\n-\t\t*ret_path = path;\n-\telse\n-\t\t*ret_path = xstrdup(path);\n+\t*ret_path = path;\n \tfree(url);\n \treturn protocol;\n }\ndiff --git a/connect.h b/connect.h\nindex 527d58a..c41a685 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -9,5 +9,6 @@ extern int git_connection_is_socket(struct child_process *conn);\n extern int server_supports(const char *feature);\n extern int parse_feature_request(const char *features, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\n+extern int url_is_local_not_ssh(const char *url);\n \n #endif\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex f4a2a38..5ad3505 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -612,4 +612,11 @@ do\n \tdone\n done\n \n+test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' '\n+\tcheck_prot_path file://c:/repo file c:/repo\n+'\n+test_expect_success MINGW 'fetch-pack --diag-url c:repo' '\n+\tcheck_prot_path c:repo file c:repo\n+'\n+\n test_done\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 53a1de9..62fbd7e 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -362,6 +362,14 @@ do\n \t\ttest_clone_url [::1]:$repo ::1 $repo\n \t'\n done\n+#home directory\n+test_expect_success \"clone host:/~repo\" '\n+\ttest_clone_url host:/~repo host \"~repo\"\n+'\n+\n+test_expect_success \"clone [::1]:/~repo\" '\n+\ttest_clone_url [::1]:/~repo ::1 \"~repo\"\n+'\n \n # Corner cases\n for url in foo/bar:baz [foo]bar/baz:qux [foo/bar]:baz\ndiff --git a/transport.c b/transport.c\nindex 7202b77..5485e2a 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -885,14 +885,6 @@ void transport_take_over(struct transport *transport,\n \ttransport->cannot_reuse = 1;\n }\n \n-static int is_local(const char *url)\n-{\n-\tconst char *colon = strchr(url, ':');\n-\tconst char *slash = strchr(url, '/');\n-\treturn !colon || (slash && slash < colon) ||\n-\t\thas_dos_drive_prefix(url);\n-}\n-\n static int is_file(const char *url)\n {\n \tstruct stat buf;\n@@ -941,7 +933,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->fetch = fetch_objs_via_rsync;\n \t\tret->push = rsync_transport_push;\n \t\tret->smart_options = NULL;\n-\t} else if (is_local(url) && is_file(url) && is_bundle(url, 1)) {\n+\t} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {\n \t\tstruct bundle_transport_data *data = xcalloc(1, sizeof(*data));\n \t\tret->data = data;\n \t\tret->get_refs_list = get_refs_from_bundle;\n@@ -1297,7 +1289,7 @@ char *transport_anonymize_url(const char *url)\n \tsize_t anon_len, prefix_len = 0;\n \n \tanon_part = strchr(url, '@');\n-\tif (is_local(url) || !anon_part)\n+\tif (url_is_local_not_ssh(url) || !anon_part)\n \t\tgoto literal_copy;\n \n \tanon_len = strlen(++anon_part);\n-- \n1.8.5.rc0.23.gaa27064\n"}]}