Skip to content

Handle URLs with a colon after host but no port #5108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand Down
1 change: 0 additions & 1 deletion deps/http-parser/http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -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
-----

Expand Down
17 changes: 11 additions & 6 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
24 changes: 24 additions & 0 deletions tests/network/urlparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
/* [email protected]:port/resource */
Expand Down