Merge pull request #5108 from libgit2/ethomson/urlparse_empty_port

Handle URLs with a colon after host but no port

5 files changed, 53 insertions, 8 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 007963c10..b7f2bc746 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -60,10 +60,10 @@ OPTION(USE_HTTPS			"Enable HTTPS support. Can be set to a specific backend" ON)
 OPTION(USE_GSSAPI			"Link with libgssapi for SPNEGO auth"			OFF)
 OPTION(USE_STANDALONE_FUZZERS		"Enable standalone fuzzers (compatible with gcc)"	OFF)
 OPTION(VALGRIND				"Configure build for valgrind"				OFF)
-OPTION(USE_EXT_HTTP_PARSER		"Use system HTTP_Parser if available"			 ON)
 OPTION(DEBUG_POOL			"Enable debug pool allocator"				OFF)
 OPTION(ENABLE_WERROR			"Enable compilation with -Werror"			OFF)
 OPTION(USE_BUNDLED_ZLIB    		"Use the bundled version of zlib"			OFF)
+   SET(USE_HTTP_PARSER			"" CACHE STRING "Specifies the HTTP Parser implementation; either system or builtin.")
 OPTION(DEPRECATE_HARD			"Do not include deprecated functions in the library"	OFF)
    SET(REGEX_BACKEND			"" CACHE STRING "Regular expression implementation. One of regcomp_l, pcre2, pcre, regcomp, or builtin.")
 
diff --git a/deps/http-parser/http_parser.c b/deps/http-parser/http_parser.c
index 27bdd2081..cc53a5215 100644
--- a/deps/http-parser/http_parser.c
+++ b/deps/http-parser/http_parser.c
@@ -2047,7 +2047,6 @@ http_parse_host(const char * buf, struct http_parser_url *u, int found_at) {
     case s_http_host_start:
     case s_http_host_v6_start:
     case s_http_host_v6:
-    case s_http_host_port_start:
     case s_http_userinfo:
     case s_http_userinfo_start:
       return 1;
diff --git a/docs/changelog.md b/docs/changelog.md
index 666eecd17..75f839212 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -1,3 +1,20 @@
+v0.28 + 1
+---------
+
+### Breaking CMake configuration changes
+
+* The CMake option to use a system http-parser library, instead of the
+  bundled dependency, has changed.  This is due to a deficiency in
+  http-parser that we have fixed in our implementation.  The bundled
+  library is now the default, but if you wish to force the use of the
+  system http-parser implementation despite incompatibilities, you can
+  specify `-DUSE_HTTP_PARSER=system` to CMake.
+
+### Changes or improvements
+
+* libgit2 can now correctly cope with URLs where the host contains a colon
+  but a port is not specified.  (eg `http://example.com:/repo.git`).
+
 v0.28
 -----
 
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 647bb097a..5a0cf347c 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -340,12 +340,17 @@ ELSE()
 ENDIF()
 
 # Optional external dependency: http-parser
-FIND_PACKAGE(HTTP_Parser)
-IF (USE_EXT_HTTP_PARSER AND HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUAL 2)
-	LIST(APPEND LIBGIT2_SYSTEM_INCLUDES ${HTTP_PARSER_INCLUDE_DIRS})
-	LIST(APPEND LIBGIT2_LIBS ${HTTP_PARSER_LIBRARIES})
-	LIST(APPEND LIBGIT2_PC_LIBS "-lhttp_parser")
-	ADD_FEATURE_INFO(http-parser ON "http-parser support")
+IF(USE_HTTP_PARSER STREQUAL "system")
+	FIND_PACKAGE(HTTP_Parser)
+
+	IF (HTTP_PARSER_FOUND AND HTTP_PARSER_VERSION_MAJOR EQUAL 2)
+		LIST(APPEND LIBGIT2_SYSTEM_INCLUDES ${HTTP_PARSER_INCLUDE_DIRS})
+		LIST(APPEND LIBGIT2_LIBS ${HTTP_PARSER_LIBRARIES})
+		LIST(APPEND LIBGIT2_PC_LIBS "-lhttp_parser")
+		ADD_FEATURE_INFO(http-parser ON "http-parser support (system)")
+	ELSE()
+		MESSAGE(FATAL_ERROR "http-parser support was requested but not found")
+	ENDIF()
 ELSE()
 	MESSAGE(STATUS "http-parser version 2 was not found or disabled; using bundled 3rd-party sources.")
 	ADD_SUBDIRECTORY("${libgit2_SOURCE_DIR}/deps/http-parser" "${libgit2_BINARY_DIR}/deps/http-parser")
diff --git a/tests/network/urlparse.c b/tests/network/urlparse.c
index c2362e628..15707885a 100644
--- a/tests/network/urlparse.c
+++ b/tests/network/urlparse.c
@@ -61,6 +61,18 @@ void test_network_urlparse__implied_root_custom_port(void)
 	cl_assert_equal_i(git_net_url_is_default_port(&conndata), 0);
 }
 
+void test_network_urlparse__implied_root_empty_port(void)
+{
+	cl_git_pass(git_net_url_parse(&conndata, "http://example.com:"));
+	cl_assert_equal_s(conndata.scheme, "http");
+	cl_assert_equal_s(conndata.host, "example.com");
+	cl_assert_equal_s(conndata.port, "80");
+	cl_assert_equal_s(conndata.path, "/");
+	cl_assert_equal_p(conndata.username, NULL);
+	cl_assert_equal_p(conndata.password, NULL);
+	cl_assert_equal_i(git_net_url_is_default_port(&conndata), 1);
+}
+
 void test_network_urlparse__encoded_password(void)
 {
 	cl_git_pass(git_net_url_parse(&conndata,
@@ -115,6 +127,18 @@ void test_network_urlparse__port(void)
 	cl_assert_equal_i(git_net_url_is_default_port(&conndata), 0);
 }
 
+void test_network_urlparse__empty_port(void)
+{
+	cl_git_pass(git_net_url_parse(&conndata, "http://example.com:/resource"));
+	cl_assert_equal_s(conndata.scheme, "http");
+	cl_assert_equal_s(conndata.host, "example.com");
+	cl_assert_equal_s(conndata.port, "80");
+	cl_assert_equal_s(conndata.path, "/resource");
+	cl_assert_equal_p(conndata.username, NULL);
+	cl_assert_equal_p(conndata.password, NULL);
+	cl_assert_equal_i(git_net_url_is_default_port(&conndata), 1);
+}
+
 void test_network_urlparse__user_port(void)
 {
 	/* user@hostname.tld:port/resource */