diff --git a/src/rebar_git_resource.erl b/src/rebar_git_resource.erl index 808f1f8f..4a37a02b 100644 --- a/src/rebar_git_resource.erl +++ b/src/rebar_git_resource.erl @@ -100,13 +100,6 @@ compare_url(Dir, Url) -> ?DEBUG("Comparing git url ~p with ~p", [ParsedUrl, ParsedCurrentUrl]), ParsedCurrentUrl =:= ParsedUrl. --ifdef (OTP_RELEASE). - -if(?OTP_RELEASE >= 23). - -compile({nowarn_deprecated_function, [{http_uri, parse, 2}, - {http_uri, scheme_defaults, 0}]}). - -endif. --endif. - parse_git_url(Url) -> %% Checks for standard scp style git remote case re:run(Url, ?SCP_PATTERN, [{capture, [host, path], list}, unicode]) of @@ -116,7 +109,7 @@ parse_git_url(Url) -> parse_git_url(not_scp, Url) end. parse_git_url(not_scp, Url) -> - UriOpts = [{scheme_defaults, [{git, 9418} | http_uri:scheme_defaults()]}], + UriOpts = [{scheme_defaults, [{git, 9418} | rebar_uri:scheme_defaults()]}], case rebar_uri:parse(Url, UriOpts) of #{path := Path, host := Host} -> {ok, {Host, filename:rootname(Path, ".git")}}; diff --git a/src/rebar_uri.erl b/src/rebar_uri.erl index 3a6382b1..a52f0e02 100644 --- a/src/rebar_uri.erl +++ b/src/rebar_uri.erl @@ -2,26 +2,33 @@ -module(rebar_uri). -export([ - parse/1, + parse/1, parse/2, scheme_defaults/0, append_path/2 ]). --import(rebar_utils, [to_list/1]). - --ifdef (OTP_RELEASE). +-ifdef(OTP_RELEASE). -spec parse(URIString) -> URIMap when URIString :: uri_string:uri_string(), URIMap :: uri_string:uri_map() | uri_string:error(). parse(URIString) -> - uri_string:parse(URIString). + parse(URIString, []). + +parse(URIString, URIOpts) -> + case uri_string:parse(URIString) of + #{path := ""} = Map -> apply_opts(Map#{path => "/"}, URIOpts); + Map -> apply_opts(Map, URIOpts) + end. -else. -spec parse(URIString) -> URIMap when URIString :: iodata(), URIMap :: map() | {error, atom(), term()}. parse(URIString) -> - case http_uri:parse(URIString) of + parse(URIString, []). + +parse(URIString, URIOpts) -> + case http_uri:parse(URIString, URIOpts) of {error, Reason} -> %% no additional parser/term info available to us, %% e.g. see what uri_string returns in @@ -46,11 +53,11 @@ parse(URIString) -> -endif. %% OTP 21+ --ifdef (OTP_RELEASE). +-ifdef(OTP_RELEASE). append_path(Url, ExtraPath) -> case parse(Url) of #{path := Path} = Map -> - FullPath = filename:join(Path, ExtraPath), + FullPath = join(Path, ExtraPath), {ok, uri_string:recompose(maps:update(path, FullPath, Map))}; _ -> error @@ -58,14 +65,66 @@ append_path(Url, ExtraPath) -> -else. append_path(Url, ExtraPath) -> case parse(Url) of - #{scheme := Scheme, userinfo := UserInfo, host := Host, port := Port, path := Path, query := Query} -> + #{scheme := Scheme, userinfo := UserInfo, host := Host, + port := Port, path := Path, query := Query} -> + ListScheme = rebar_utils:to_list(Scheme), PrefixedQuery = case Query of [] -> []; Other -> lists:append(["?", Other]) end, - {ok, lists:append([to_list(Scheme), "://", UserInfo, Host, ":", to_list(Port), - filename:join(Path, ExtraPath), PrefixedQuery])}; + NormPath = case Path of + "" -> "/"; + _ -> Path + end, + {ok, maybe_port( + Url, lists:append([ListScheme, "://", UserInfo, Host]), + [$: | rebar_utils:to_list(Port)], + lists:append([join(NormPath, ExtraPath), PrefixedQuery]) + )}; _ -> error end. -endif. + +%% OTP 21+ +-ifdef(OTP_RELEASE). +scheme_defaults() -> + %% no scheme defaults here; just custom ones + []. +-else. +scheme_defaults() -> + http_uri:scheme_defaults(). +-endif. + +join(URI, "") -> URI; +join(URI, "/") -> URI; +join("/", [$/|_] = Path) -> Path; +join("/", Path) -> [$/ | Path]; +join("", [$/|_] = Path) -> Path; +join("", Path) -> [$/ | Path]; +join([H|T], Path) -> [H | join(T, Path)]. + + +-ifdef(OTP_RELEASE). +apply_opts(Map = #{port := _}, _) -> + Map; +apply_opts(Map = #{scheme := Scheme}, URIOpts) -> + SchemeDefaults = proplists:get_value(scheme_defaults, URIOpts, []), + %% Here is the funky bit: don't add the port number if it's in a default + %% to maintain proper default behaviour. + try lists:keyfind(list_to_existing_atom(Scheme), 1, SchemeDefaults) of + {_, Port} -> + Map#{port => Port}; + false -> + Map + catch + error:badarg -> % not an existing atom, not in the list + Map + end. +-else. +maybe_port(Url, Host, Port, PathQ) -> + case lists:prefix(Host ++ Port, Url) of + true -> Host ++ Port ++ PathQ; % port was explicit + false -> Host ++ PathQ % port was implicit + end. +-endif. diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl index 6be50f14..0933bb6c 100644 --- a/src/rebar_utils.erl +++ b/src/rebar_utils.erl @@ -64,6 +64,7 @@ tup_find/2, line_count/1, set_httpc_options/0, + url_append_path/2, escape_chars/1, escape_double_quotes/1, escape_double_quotes_weak/1, @@ -938,6 +939,9 @@ normalise_proxy(Scheme, URI) -> _ -> URI end. +url_append_path(Url, ExtraPath) -> + rebar_uri:append_path(Url, ExtraPath). + %% escape\ as\ a\ shell\? escape_chars(Str) when is_atom(Str) -> escape_chars(atom_to_list(Str)); diff --git a/test/rebar_uri_SUITE.erl b/test/rebar_uri_SUITE.erl index 4db93690..a7fde27b 100644 --- a/test/rebar_uri_SUITE.erl +++ b/test/rebar_uri_SUITE.erl @@ -9,20 +9,20 @@ -include_lib("kernel/include/file.hrl"). all() -> - [parse]. + [parse, append_path]. parse(_Config) -> #{scheme := Scheme, host := Host, path := Path} = rebar_uri:parse("https://repo.hex.pm"), ?assertEqual("https", Scheme), ?assertEqual("repo.hex.pm", Host), - ?assert(lists:member(Path, ["", "/"])), + ?assertEqual(Path, "/"), % Normalize on OTP-23 behaviour. #{scheme := Scheme2, host := Host2, port := Port2, path := Path2, query := Query2} = rebar_uri:parse("https://repo.hex.pm:443?foo=bar"), ?assertEqual("https", Scheme2), ?assertEqual("repo.hex.pm", Host2), ?assertEqual(443, Port2), - ?assert(lists:member(Path2, ["", "/"])), + ?assertEqual(Path2, "/"), % Normalize on old http_uri behaviour ?assertEqual("foo=bar", Query2), #{scheme := Scheme3, host := Host3, path := Path3, query := Query3} = @@ -30,19 +30,21 @@ parse(_Config) -> ?assertEqual("https", Scheme3), ?assertEqual("repo.hex.pm", Host3), ?assertEqual("/over/here", Path3), - ?assertEqual("foo=bar", Query3). + ?assertEqual("foo=bar", Query3), + + %% override default port and get it parsed as such + ?assertMatch(#{port := 1337}, + rebar_uri:parse("https://repo.hex.pm/", + [{scheme_defaults, [{https,1337}]}])), + ok. append_path(_Config) -> - %% OTP version differences - {ok, Val1} = rebar_utils:append_path("https://repo.hex.pm", "/repos/org"), - ?assert(lists:member(Val1, [ - "https://repo.hex.pm/repos/org", - "https://repo.hex.pm:443/repos/org" - ])), - {ok, Val2} = rebar_utils:append_path("https://repo.hex.pm?foo=bar", "/repos/org"), - ?assert(lists:member(Val2, [ - "https://repo.hex.pm/repos/org?foo=bar", - "https://repo.hex.pm:443/repos/org?foo=bar" - ])), + %% Default port for the proto is omitted if not mentioned originally + {ok, Val1} = rebar_uri:append_path("https://repo.hex.pm/", "/repos/org"), + ?assertEqual("https://repo.hex.pm/repos/org", Val1), + %% QS elements come after the path + {ok, Val2} = rebar_uri:append_path("https://repo.hex.pm?foo=bar", "/repos/org"), + ?assertEqual("https://repo.hex.pm/repos/org?foo=bar", Val2), + %% If the port is explicitly mentioned, keep it. ?assertEqual({ok, "https://repo.hex.pm:443/repos/org?foo=bar"}, - rebar_utils:append_path("https://repo.hex.pm:443?foo=bar", "/repos/org")). + rebar_uri:append_path("https://repo.hex.pm:443?foo=bar", "/repos/org")).