From c752f3bec524a6801e49a19a7e40f0b736803ccb Mon Sep 17 00:00:00 2001 From: Zaiming Shi Date: Sat, 6 Mar 2021 06:35:02 +0100 Subject: [PATCH] Refactor http lib add uri parse (#4292) * feat(http_lib): Add uri parse to emqx_http_lib * fix(webhook): call emqx_http_lib to parse uri * fix(auth-http): Call emqx_http_lib to parse uri * fix(rule-engine): call emqx_http_lib to parse uri --- .../emqx_auth_http/src/emqx_auth_http_app.erl | 24 ++------ apps/emqx_rule_engine/src/emqx_rule_utils.erl | 19 ++---- .../src/emqx_web_hook_actions.erl | 34 +++-------- apps/emqx_web_hook/src/emqx_web_hook_app.erl | 24 ++------ src/emqx_http_lib.erl | 60 ++++++++++++++++++- test/emqx_http_lib_tests.erl | 28 +++++++++ 6 files changed, 112 insertions(+), 77 deletions(-) diff --git a/apps/emqx_auth_http/src/emqx_auth_http_app.erl b/apps/emqx_auth_http/src/emqx_auth_http_app.erl index 89f42a2cd..acbb67bf4 100644 --- a/apps/emqx_auth_http/src/emqx_auth_http_app.erl +++ b/apps/emqx_auth_http/src/emqx_auth_http_app.erl @@ -53,19 +53,16 @@ translate_env(EnvName) -> {ok, PoolSize} = application:get_env(?APP, pool_size), {ok, ConnectTimeout} = application:get_env(?APP, connect_timeout), URL = proplists:get_value(url, Req), - #{host := Host0, - path := Path0, - scheme := Scheme} = URIMap = uri_string:parse(add_default_scheme(uri_string:normalize(URL))), - Port = maps:get(port, URIMap, case Scheme of - "https" -> 443; - "http" -> 80 - end), + {ok, #{host := Host0, + path := Path0, + port := Port, + scheme := Scheme}} = emqx_http_lib:uri_parse(URL), Path = path(Path0), {Inet, Host} = parse_host(Host0), MoreOpts = case Scheme of - "http" -> + http -> [{transport_opts, [Inet]}]; - "https" -> + https -> CACertFile = application:get_env(?APP, cacertfile, undefined), CertFile = application:get_env(?APP, certfile, undefined), KeyFile = application:get_env(?APP, keyfile, undefined), @@ -158,15 +155,6 @@ ensure_content_type_header(Method, Headers) ensure_content_type_header(_Method, Headers) -> lists:keydelete("content-type", 1, Headers). -add_default_scheme(URL) when is_list(URL) -> - binary_to_list(add_default_scheme(list_to_binary(URL))); -add_default_scheme(<<"http://", _/binary>> = URL) -> - URL; -add_default_scheme(<<"https://", _/binary>> = URL) -> - URL; -add_default_scheme(URL) -> - <<"http://", URL/binary>>. - path("") -> "/"; path(Path) -> diff --git a/apps/emqx_rule_engine/src/emqx_rule_utils.erl b/apps/emqx_rule_engine/src/emqx_rule_utils.erl index 409095e02..e178fc9d2 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_utils.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_utils.erl @@ -202,15 +202,11 @@ http_connectivity(Url) -> -spec(http_connectivity(uri_string(), integer()) -> ok | {error, Reason :: term()}). http_connectivity(Url, Timeout) -> - case uri_string:parse(uri_string:normalize(Url)) of - {error, Reason, _} -> - {error, Reason}; - #{host := Host, port := Port} -> + case emqx_http_lib:uri_parse(Url) of + {ok, #{host := Host, port := Port}} -> tcp_connectivity(str(Host), Port, Timeout); - #{host := Host, scheme := Scheme} -> - tcp_connectivity(str(Host), default_port(Scheme), Timeout); - _ -> - {error, {invalid_url, Url}} + {error, Reason} -> + {error, Reason} end. -spec tcp_connectivity(Host :: inet:socket_address() | inet:hostname(), @@ -229,13 +225,6 @@ tcp_connectivity(Host, Port, Timeout) -> {error, Reason} -> {error, Reason} end. -default_port("http") -> 80; -default_port("https") -> 443; -default_port(<<"http">>) -> 80; -default_port(<<"https">>) -> 443; -default_port(Scheme) -> throw({bad_scheme, Scheme}). - - unwrap(<<"${", Val/binary>>) -> binary:part(Val, {0, byte_size(Val)-1}). diff --git a/apps/emqx_web_hook/src/emqx_web_hook_actions.erl b/apps/emqx_web_hook/src/emqx_web_hook_actions.erl index e5aa25c3a..4e63b54df 100644 --- a/apps/emqx_web_hook/src/emqx_web_hook_actions.erl +++ b/apps/emqx_web_hook/src/emqx_web_hook_actions.erl @@ -281,7 +281,7 @@ create_req(_, Path, Headers, Body) -> parse_action_params(Params = #{<<"url">> := URL}) -> try - #{path := CommonPath} = uri_string:parse(URL), + {ok, #{path := CommonPath}} = emqx_http_lib:uri_parse(URL), Method = method(maps:get(<<"method">>, Params, <<"POST">>)), Headers = headers(maps:get(<<"headers">>, Params, undefined)), NHeaders = ensure_content_type_header(Headers, Method), @@ -318,31 +318,19 @@ str(Str) when is_list(Str) -> Str; str(Atom) when is_atom(Atom) -> atom_to_list(Atom); str(Bin) when is_binary(Bin) -> binary_to_list(Bin). -add_default_scheme(<<"http://", _/binary>> = URL) -> - URL; -add_default_scheme(<<"https://", _/binary>> = URL) -> - URL; -add_default_scheme(URL) -> - <<"http://", URL/binary>>. - pool_opts(Params = #{<<"url">> := URL}, ResId) -> - #{host := Host0, scheme := Scheme} = URIMap = - uri_string:parse(binary_to_list(add_default_scheme(URL))), - DefaultPort = case is_https(Scheme) of - true -> 443; - false -> 80 - end, - Port = maps:get(port, URIMap, DefaultPort), + {ok, #{host := Host0, + port := Port, + scheme := Scheme}} = emqx_http_lib:uri_parse(URL), PoolSize = maps:get(<<"pool_size">>, Params, 32), ConnectTimeout = cuttlefish_duration:parse(str(maps:get(<<"connect_timeout">>, Params, <<"5s">>))), {Inet, Host} = parse_host(Host0), - TransportOpts = - case is_https(Scheme) of - true -> [Inet | get_ssl_opts(Params, ResId)]; - false -> [Inet] - end, - Opts = case is_https(Scheme) of + TransportOpts = case Scheme =:= https of + true -> [Inet | get_ssl_opts(Params, ResId)]; + false -> [Inet] + end, + Opts = case Scheme =:= https of true -> [{transport_opts, TransportOpts}, {transport, ssl}]; false -> [{transport_opts, TransportOpts}] end, @@ -357,10 +345,6 @@ pool_opts(Params = #{<<"url">> := URL}, ResId) -> pool_name(ResId) -> list_to_atom("webhook:" ++ str(ResId)). -is_https(Scheme) when is_list(Scheme) -> is_https(list_to_binary(Scheme)); -is_https(<<"https", _/binary>>) -> true; -is_https(_) -> false. - get_ssl_opts(Opts, ResId) -> Dir = filename:join([emqx:get_env(data_dir), "rule", ResId]), [{ssl, true}, {ssl_opts, emqx_plugin_libs_ssl:save_files_return_opts(Opts, Dir)}]. diff --git a/apps/emqx_web_hook/src/emqx_web_hook_app.erl b/apps/emqx_web_hook/src/emqx_web_hook_app.erl index 54ac9c317..67775e00f 100644 --- a/apps/emqx_web_hook/src/emqx_web_hook_app.erl +++ b/apps/emqx_web_hook/src/emqx_web_hook_app.erl @@ -39,31 +39,19 @@ stop(_State) -> emqx_web_hook:unload(), ehttpc_sup:stop_pool(?APP). -add_default_scheme(URL) when is_list(URL) -> - binary_to_list(add_default_scheme(list_to_binary(URL))); -add_default_scheme(<<"http://", _/binary>> = URL) -> - URL; -add_default_scheme(<<"https://", _/binary>> = URL) -> - URL; -add_default_scheme(URL) -> - <<"http://", URL/binary>>. - translate_env() -> {ok, URL} = application:get_env(?APP, url), - #{host := Host0, - path := Path0, - scheme := Scheme} = URIMap = uri_string:parse(add_default_scheme(uri_string:normalize(URL))), - Port = maps:get(port, URIMap, case Scheme of - "https" -> 443; - "http" -> 80 - end), + {ok, #{host := Host0, + path := Path0, + port := Port, + scheme := Scheme}} = emqx_http_lib:uri_parse(URL), Path = path(Path0), {Inet, Host} = parse_host(Host0), PoolSize = application:get_env(?APP, pool_size, 32), MoreOpts = case Scheme of - "http" -> + http -> [{transport_opts, [Inet]}]; - "https" -> + https -> CACertFile = application:get_env(?APP, cacertfile, undefined), CertFile = application:get_env(?APP, certfile, undefined), KeyFile = application:get_env(?APP, keyfile, undefined), diff --git a/src/emqx_http_lib.erl b/src/emqx_http_lib.erl index 695879236..195656622 100644 --- a/src/emqx_http_lib.erl +++ b/src/emqx_http_lib.erl @@ -16,7 +16,20 @@ -module(emqx_http_lib). --export([uri_encode/1, uri_decode/1]). +-export([ uri_encode/1 + , uri_decode/1 + , uri_parse/1 + ]). + +-export_type([uri_map/0]). + +-type uri_map() :: #{scheme := http | https, + host := unicode:chardata(), + port := non_neg_integer(), + path => unicode:chardata(), + query => unicode:chardata(), + fragment => unicode:chardata(), + userinfo => unicode:chardata()}. %% @doc Decode percent-encoded URI. %% This is copied from http_uri.erl which has been deprecated since OTP-23 @@ -35,6 +48,51 @@ uri_decode(<<>>) -> uri_encode(URI) when is_binary(URI) -> << <<(uri_encode_binary(Char))/binary>> || <> <= URI >>. +%% @doc Parse URI into a map as uri_string:uri_map(), but with two fields +%% normalised: (1): port number is never 'undefined', default ports are used +%% if missing. (2): scheme is always atom. +-spec uri_parse(string() | binary()) -> {ok, uri_map()} | {error, any()}. +uri_parse(URI) -> + try + {ok, do_parse(uri_string:normalize(URI))} + catch + throw : Reason -> + {error, Reason} + end. + +do_parse({error, Reason, Which}) -> throw({Reason, Which}); +do_parse(URI) -> + %% ensure we return string() instead of binary() in uri_map() values. + Map = uri_string:parse(unicode:characters_to_list(URI)), + case maps:is_key(scheme, Map) of + true -> + normalise_parse_result(Map); + false -> + %% missing scheme, add "http://" and try again + Map2 = uri_string:parse(unicode:characters_to_list(["http://", URI])), + normalise_parse_result(Map2) + end. + +normalise_parse_result(#{host := _, scheme := Scheme0} = Map) -> + Scheme = atom_scheme(Scheme0), + DefaultPort = case https =:= Scheme of + true -> 443; + false -> 80 + end, + Port = case maps:get(port, Map, undefined) of + N when is_number(N) -> N; + _ -> DefaultPort + end, + Map#{ scheme => Scheme + , port => Port + }. + +%% NOTE: so far we only support http schemes. +atom_scheme(Scheme) when is_list(Scheme) -> atom_scheme(list_to_binary(Scheme)); +atom_scheme(<<"https">>) -> https; +atom_scheme(<<"http">>) -> http; +atom_scheme(Other) -> throw({unsupported_scheme, Other}). + uri_encode_binary(Char) -> case reserved(Char) of true -> diff --git a/test/emqx_http_lib_tests.erl b/test/emqx_http_lib_tests.erl index 78a647417..b5c50e7c9 100644 --- a/test/emqx_http_lib_tests.erl +++ b/test/emqx_http_lib_tests.erl @@ -44,3 +44,31 @@ test_prop_uri(URI) -> Decoded2 = uri_string:percent_decode(Encoded), ?assertEqual(URI, Decoded2), true. + +uri_parse_test_() -> + [ {"default port http", + fun() -> ?assertMatch({ok, #{port := 80, scheme := http, host := "localhost"}}, + emqx_http_lib:uri_parse("localhost")) + end + } + , {"default port https", + fun() -> ?assertMatch({ok, #{port := 443, scheme := https}}, + emqx_http_lib:uri_parse("https://localhost")) + end + } + , {"bad url", + fun() -> ?assertMatch({error, {invalid_uri, _}}, + emqx_http_lib:uri_parse("https://localhost:notnumber")) + end + } + , {"normalise", + fun() -> ?assertMatch({ok, #{scheme := https}}, + emqx_http_lib:uri_parse("HTTPS://127.0.0.1")) + end + } + , {"unsupported_scheme", + fun() -> ?assertEqual({error, {unsupported_scheme, <<"wss">>}}, + emqx_http_lib:uri_parse("wss://127.0.0.1")) + end + } + ].