Merge pull request #1803 from libgit2/ntk/topic/even_more_lenient_remote_parsing

Even more lenient remote parsing

4 files changed, 46 insertions, 16 deletions

diff --git a/src/remote.c b/src/remote.c
index 671075bb7..bdc8e0e67 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -233,7 +233,8 @@ static int refspec_cb(const git_config_entry *entry, void *payload)
 }
 
 static int get_optional_config(
-	git_config *config, git_buf *buf, git_config_foreach_cb cb, void *payload)
+	bool *found, git_config *config, git_buf *buf,
+	git_config_foreach_cb cb, void *payload)
 {
 	int error = 0;
 	const char *key = git_buf_cstr(buf);
@@ -246,6 +247,9 @@ static int get_optional_config(
 	else
 		error = git_config_get_string(payload, config, key);
 
+	if (found)
+		*found = !error;
+
 	if (error == GIT_ENOTFOUND) {
 		giterr_clear();
 		error = 0;
@@ -265,6 +269,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	int error = 0;
 	git_config *config;
 	struct refspec_cb_data data;
+	bool optional_setting_found = false, found;
 
 	assert(out && repo && name);
 
@@ -294,21 +299,33 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 		goto cleanup;
 	}
 
-	if ((error = git_config_get_string(&val, config, git_buf_cstr(&buf))) < 0)
+	if ((error = get_optional_config(&found, config, &buf, NULL, (void *)&val)) < 0)
 		goto cleanup;
 
+	optional_setting_found |= found;
+
 	remote->repo = repo;
-	remote->url = git__strdup(val);
-	GITERR_CHECK_ALLOC(remote->url);
+
+	if (found && strlen(val) > 0) {
+		remote->url = git__strdup(val);
+		GITERR_CHECK_ALLOC(remote->url);
+	}
 
 	val = NULL;
 	git_buf_clear(&buf);
 	git_buf_printf(&buf, "remote.%s.pushurl", name);
 
-	if ((error = get_optional_config(config, &buf, NULL, (void *)&val)) < 0)
+	if ((error = get_optional_config(&found, config, &buf, NULL, (void *)&val)) < 0)
+		goto cleanup;
+
+	optional_setting_found |= found;
+
+	if (!optional_setting_found) {
+		error = GIT_ENOTFOUND;
 		goto cleanup;
+	}
 
-	if (val) {
+	if (found && strlen(val) > 0) {
 		remote->pushurl = git__strdup(val);
 		GITERR_CHECK_ALLOC(remote->pushurl);
 	}
@@ -318,14 +335,14 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	git_buf_clear(&buf);
 	git_buf_printf(&buf, "remote.%s.fetch", name);
 
-	if ((error = get_optional_config(config, &buf, refspec_cb, &data)) < 0)
+	if ((error = get_optional_config(NULL, config, &buf, refspec_cb, &data)) < 0)
 		goto cleanup;
 
 	data.fetch = false;
 	git_buf_clear(&buf);
 	git_buf_printf(&buf, "remote.%s.push", name);
 
-	if ((error = get_optional_config(config, &buf, refspec_cb, &data)) < 0)
+	if ((error = get_optional_config(NULL, config, &buf, refspec_cb, &data)) < 0)
 		goto cleanup;
 
 	if (download_tags_value(remote, config) < 0)
@@ -534,6 +551,8 @@ const char* git_remote__urlfordirection(git_remote *remote, int direction)
 {
 	assert(remote);
 
+	assert(direction == GIT_DIRECTION_FETCH || direction == GIT_DIRECTION_PUSH);
+
 	if (direction == GIT_DIRECTION_FETCH) {
 		return remote->url;
 	}
@@ -556,8 +575,11 @@ int git_remote_connect(git_remote *remote, git_direction direction)
 	t = remote->transport;
 
 	url = git_remote__urlfordirection(remote, direction);
-	if (url == NULL )
+	if (url == NULL ) {
+		giterr_set(GITERR_INVALID,
+			"Malformed remote '%s' - missing URL", remote->name);
 		return -1;
+	}
 
 	/* A transport could have been supplied in advance with
 	 * git_remote_set_transport */
@@ -1087,10 +1109,10 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	if (git_repository_config__weakptr(&cfg, repo) < 0)
 		return -1;
 
-	if (git_vector_init(&list, 4, NULL) < 0)
+	if (git_vector_init(&list, 4, git__strcmp_cb) < 0)
 		return -1;
 
-	if (regcomp(&preg, "^remote\\.(.*)\\.url$", REG_EXTENDED) < 0) {
+	if (regcomp(&preg, "^remote\\.(.*)\\.(push)?url$", REG_EXTENDED) < 0) {
 		giterr_set(GITERR_OS, "Remote catch regex failed to compile");
 		return -1;
 	}
@@ -1115,6 +1137,8 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 		return error;
 	}
 
+	git_vector_uniq(&list, git__free);
+
 	remotes_list->strings = (char **)list.contents;
 	remotes_list->count = list.length;
 
diff --git a/src/util.h b/src/util.h
index a784390c1..bd93b46b5 100644
--- a/src/util.h
+++ b/src/util.h
@@ -82,7 +82,10 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size)
 	return new_ptr;
 }
 
-#define git__free(ptr) free(ptr)
+GIT_INLINE(void) git__free(void *ptr)
+{
+	free(ptr);
+}
 
 #define STRCMP_CASESELECT(IGNORE_CASE, STR1, STR2) \
 	((IGNORE_CASE) ? strcasecmp((STR1), (STR2)) : strcmp((STR1), (STR2)))
diff --git a/src/vector.c b/src/vector.c
index 5ba2fab18..362e7b0c0 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -220,7 +220,7 @@ void git_vector_pop(git_vector *v)
 		v->length--;
 }
 
-void git_vector_uniq(git_vector *v)
+void git_vector_uniq(git_vector *v, void  (*git_free_cb)(void *))
 {
 	git_vector_cmp cmp;
 	size_t i, j;
@@ -232,9 +232,12 @@ void git_vector_uniq(git_vector *v)
 	cmp = v->_cmp ? v->_cmp : strict_comparison;
 
 	for (i = 0, j = 1 ; j < v->length; ++j)
-		if (!cmp(v->contents[i], v->contents[j]))
+		if (!cmp(v->contents[i], v->contents[j])) {
+			if (git_free_cb)
+				git_free_cb(v->contents[i]);
+
 			v->contents[i] = v->contents[j];
-		else
+		} else
 			v->contents[++i] = v->contents[j];
 
 	v->length -= j - i - 1;
diff --git a/src/vector.h b/src/vector.h
index 1bda9c93d..cca846f43 100644
--- a/src/vector.h
+++ b/src/vector.h
@@ -71,7 +71,7 @@ int git_vector_insert_sorted(git_vector *v, void *element,
 	int (*on_dup)(void **old, void *new));
 int git_vector_remove(git_vector *v, size_t idx);
 void git_vector_pop(git_vector *v);
-void git_vector_uniq(git_vector *v);
+void git_vector_uniq(git_vector *v, void  (*git_free_cb)(void *));
 void git_vector_remove_matching(
 	git_vector *v, int (*match)(const git_vector *v, size_t idx));