From 83aae9baeed07fd1c3a7d55df20129c4480292ac Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Thu, 27 Feb 2020 09:50:24 +0200 Subject: [PATCH] Replace xmerl with erlsom Also includes small syntax fixes to sys.config, as well as one compilation warning removal. --- apps/epp_proxy/src/epp_xml.erl | 45 +++++++++++++------------- apps/epp_proxy/test/epp_util_SUITE.erl | 1 - apps/epp_proxy/test/epp_xml_SUITE.erl | 22 +++++++------ config/docker.config | 1 + config/sys.config | 3 +- config/test.config | 1 + rebar.config | 5 +-- rebar.lock | 2 ++ 8 files changed, 43 insertions(+), 37 deletions(-) diff --git a/apps/epp_proxy/src/epp_xml.erl b/apps/epp_proxy/src/epp_xml.erl index 0a98f3c..e98804d 100644 --- a/apps/epp_proxy/src/epp_xml.erl +++ b/apps/epp_proxy/src/epp_xml.erl @@ -2,27 +2,25 @@ -export([find_cltrid/1, get_command/1, parse/1]). --include_lib("xmerl/include/xmerl.hrl"). +%% We are only interested in start element of a kind. The list produced +%% by this will need reversing after it is complete. +%% This parsing is naive, expects command/hello element to come right +%% after epp, but this should be everything we need for the purpose. +-define(PARSER_FUN, + fun (Event, Acc) -> + case Event of + {startElement, _, Name, _, _} -> [Name | Acc]; + _ -> Acc + end + end). -%% Get command from an xmlElement. Otherwise return undefined. -get_command(Record) - when is_record(Record, xmlElement) -> - case xmerl_xpath:string("name(/epp/*[1])", Record) of - {xmlObj, string, "hello"} -> "hello"; - {xmlObj, string, "command"} -> get_command1(Record); - {xmlObj, string, []} -> undefined - end; +%% Get command a list of elements found by erlsom. +%% Otherwise return undefined. +get_command(["epp", "command", Command | _Rest]) -> + Command; +get_command(["epp", "hello" | _Rest]) -> "hello"; get_command(_) -> undefined. -get_command1(Record) - when is_record(Record, xmlElement) -> - case xmerl_xpath:string("name(/epp/command/*[1])", - Record) - of - {xmlObj, string, []} -> undefined; - {xmlObj, string, Command} -> Command - end. - %% xml_erl expects a list of characters, so let's give it to it. %% Otherwise return an error tuple. parse(Text) when is_list(Text) -> parse_list(Text); @@ -30,13 +28,14 @@ parse(Text) when is_binary(Text) -> List = binary_to_list(Text), parse_list(List); parse(_) -> {error, {fatal, {expected_binary_or_list}}}. -%% Parse a record that came from the wire and return a xmlElement record. parse_list(List) when is_list(List) -> - try xmerl_scan:string(List, [{quiet, true}]) of - {Record, []} when is_record(Record, xmlElement) -> - {ok, Record} + try erlsom:parse_sax(List, [], ?PARSER_FUN) of + {ok, Result, _} -> + ProperResult = lists:reverse(Result), {ok, ProperResult} catch - exit:X -> {error, X} + {error, Error} -> {error, Error}; + error:Error -> {error, Error}; + Error -> {error, Error} end. %% The idea is that even when XML command is invalid, diff --git a/apps/epp_proxy/test/epp_util_SUITE.erl b/apps/epp_proxy/test/epp_util_SUITE.erl index 2968f6f..fe8a80a 100644 --- a/apps/epp_proxy/test/epp_util_SUITE.erl +++ b/apps/epp_proxy/test/epp_util_SUITE.erl @@ -74,6 +74,5 @@ readable_ip_test_case(_Config) -> path_for_file_test_case(_Config) -> AbsoluteFilename = "/usr/bin", AbsoluteFilename = epp_util:path_for_file(AbsoluteFilename), - RelativeFilename = "usr/bin", true = (AbsoluteFilename =:= epp_util:path_for_file(AbsoluteFilename)), ok. diff --git a/apps/epp_proxy/test/epp_xml_SUITE.erl b/apps/epp_proxy/test/epp_xml_SUITE.erl index 9341817..daa68fe 100644 --- a/apps/epp_proxy/test/epp_xml_SUITE.erl +++ b/apps/epp_proxy/test/epp_xml_SUITE.erl @@ -1,9 +1,7 @@ -module(epp_xml_SUITE). -include_lib("common_test/include/ct.hrl"). - -%% This is required for parse tests. --include_lib("xmerl/include/xmerl.hrl"). +-include_lib("stdlib/include/assert.hrl"). -define(sampleCommandList, " @@ -59,24 +57,28 @@ parse_not_a_list_or_binary_test_case(_Config) -> parse_sample_valid_xml_list_test_case(_Config) -> Input = ?sampleCommandList, - {ok, Record} = epp_xml:parse(Input), - true = is_record(Record, xmlElement), + {ok, Result} = epp_xml:parse(Input), + ?assertEqual(["epp", "command", "login", "clID", "pw", "clTRID"], + Result), ok. parse_sample_valid_xml_binary_test_case(_Config) -> Input = list_to_binary(?sampleCommandList), - {ok, Record} = epp_xml:parse(Input), - true = is_record(Record, xmlElement), + {ok, Result} = epp_xml:parse(Input), + ?assertEqual(["epp", "command", "login", "clID", "pw", "clTRID"], + Result), ok. parse_sample_invalid_xml_list_test_case(_Config) -> Input = "Some text", - {error, {fatal, _Details}} = epp_xml:parse(Input), + ExpectedResult = {error, "Malformed: Illegal character in prolog"}, + ?assertEqual(ExpectedResult, epp_xml:parse(Input)), ok. parse_sample_invalid_xml_binary_test_case(_Config) -> - Input = list_to_binary("Some text"), - {error, {fatal, _Details}} = epp_xml:parse(Input), + Input = <<"\n">>, + ExpectedResult = {error, {badmatch, []}}, + ?assertEqual(ExpectedResult, epp_xml:parse(Input)), ok. %% find_cltrid diff --git a/config/docker.config b/config/docker.config index fa8eee7..a8f8935 100644 --- a/config/docker.config +++ b/config/docker.config @@ -1,3 +1,4 @@ +%% -*- erlang -*- [ {epp_proxy, [ {dev_mode, true}, diff --git a/config/sys.config b/config/sys.config index f895a3d..c2f3ae4 100644 --- a/config/sys.config +++ b/config/sys.config @@ -1,3 +1,4 @@ +%% -*- erlang -*- [ {epp_proxy, [ %% Enables or disable TCP connections without TLS (true/false) @@ -9,7 +10,7 @@ {tls_port, 700}, %% When set to true, you can connect to EPP over HTTPS endpoints without %% verifying their TLS certificates. - {insecure, false} + {insecure, false}, %% URL of EPP endpoints. Can be pointed at a web server (Apache/NGINX) %% Can contain port (https://some-host:3000/epp/session) %% Honors the prepended protocol (http / https). diff --git a/config/test.config b/config/test.config index 61c6588..61b7a2c 100644 --- a/config/test.config +++ b/config/test.config @@ -1,3 +1,4 @@ +%% -*- erlang -*- [ {epp_proxy, [{dev_mode, true}, {tcp_port, 1180}, diff --git a/rebar.config b/rebar.config index 857d2bf..3f6979f 100644 --- a/rebar.config +++ b/rebar.config @@ -3,7 +3,8 @@ {deps, [{hackney, "1.15.1"}, {syslog, {git, "https://github.com/Vagabond/erlang-syslog.git", {branch, "master"}}}, {lager, "3.7.0"}, - {lager_syslog, {git, "https://github.com/erlang-lager/lager_syslog.git"}}]}. + {lager_syslog, {git, "https://github.com/erlang-lager/lager_syslog.git"}}, + {erlsom, "1.5.0"}]}. {relx, [{release, {epp_proxy, "git"}, [epp_proxy, @@ -11,7 +12,7 @@ lager_syslog, hackney, sasl, - xmerl]}, + erlsom]}, {sys_config, "./config/sys.config"}, {dev_mode, false}, diff --git a/rebar.lock b/rebar.lock index dd1633b..d34177f 100644 --- a/rebar.lock +++ b/rebar.lock @@ -1,5 +1,6 @@ {"1.1.0", [{<<"certifi">>,{pkg,<<"certifi">>,<<"2.5.1">>},1}, + {<<"erlsom">>,{pkg,<<"erlsom">>,<<"1.5.0">>},0}, {<<"goldrush">>,{pkg,<<"goldrush">>,<<"0.1.9">>},1}, {<<"hackney">>,{pkg,<<"hackney">>,<<"1.15.1">>},0}, {<<"idna">>,{pkg,<<"idna">>,<<"6.0.0">>},1}, @@ -20,6 +21,7 @@ [ {pkg_hash,[ {<<"certifi">>, <<"867CE347F7C7D78563450A18A6A28A8090331E77FA02380B4A21962A65D36EE5">>}, + {<<"erlsom">>, <<"C5A5CDD0EE0E8DCA62BCC4B13FF08DA24FDEFC16CCD8B25282A2FDA2BA1BE24A">>}, {<<"goldrush">>, <<"F06E5D5F1277DA5C413E84D5A2924174182FB108DABB39D5EC548B27424CD106">>}, {<<"hackney">>, <<"9F8F471C844B8CE395F7B6D8398139E26DDCA9EBC171A8B91342EE15A19963F4">>}, {<<"idna">>, <<"689C46CBCDF3524C44D5F3DDE8001F364CD7608A99556D8FBD8239A5798D4C10">>},