Merge branch 'goertzenator/snmpstring/PR-1100/OTP-13706' into maint

* goertzenator/snmpstring/PR-1100/OTP-13706:
  add snmp_agent_conf_test to modules.mk
  Fix ERL-164 for agent, vacm, and usm config files
  snmp tests demonstrating ERL-164, no string escaping for SNMP config files

4 files changed, 222 insertions, 9 deletions

diff --git a/lib/snmp/src/agent/snmpa_conf.erl b/lib/snmp/src/agent/snmpa_conf.erl
index 94325a8ed6..fc5116dac9 100644
--- a/lib/snmp/src/agent/snmpa_conf.erl
+++ b/lib/snmp/src/agent/snmpa_conf.erl
@@ -154,7 +154,7 @@ do_write_agent_conf(Fd, {intAgentMaxPacketSize = Tag, Val} ) ->
 do_write_agent_conf(Fd, {snmpEngineMaxMessageSize = Tag, Val} ) ->
     io:format(Fd, "{~w, ~w}.~n", [Tag, Val]);
 do_write_agent_conf(Fd, {snmpEngineID = Tag, Val} ) ->
-    io:format(Fd, "{~w, \"~s\"}.~n", [Tag, Val]);
+    io:format(Fd, "{~w, ~p}.~n", [Tag, Val]);
 do_write_agent_conf(_Fd, Crap) ->
     error({bad_agent_config, Crap}).
 
@@ -758,9 +758,9 @@ do_write_usm_conf(
    PrivP, PrivKeyC, OwnPrivKeyC,
    Public, AuthKey, PrivKey}) ->
     io:format(Fd, "{", []),
-    io:format(Fd, "\"~s\", ", [EngineID]),
-    io:format(Fd, "\"~s\", ", [UserName]),
-    io:format(Fd, "\"~s\", ", [SecName]),
+    io:format(Fd, "~p, ", [EngineID]),
+    io:format(Fd, "~p, ", [UserName]),
+    io:format(Fd, "~p, ", [SecName]),
     io:format(Fd, "~w, ",     [Clone]),
     io:format(Fd, "~w, ",     [AuthP]),
     do_write_usm2(Fd, AuthKeyC, ", "),
@@ -859,15 +859,15 @@ do_write_vacm_conf(
   {vacmSecurityToGroup,
    SecModel, SecName, GroupName}) ->
     io:format(
-      Fd, "{vacmSecurityToGroup, ~w, \"~s\", \"~s\"}.~n",
+      Fd, "{vacmSecurityToGroup, ~w, ~p, ~p}.~n",
       [SecModel, SecName, GroupName]);
 do_write_vacm_conf(
   Fd,
   {vacmAccess,
    GroupName, Prefix, SecModel, SecLevel, Match, RV, WV, NV}) ->
     io:format(
-      Fd, "{vacmAccess, \"~s\", \"~s\", ~w, ~w, ~w, "
-      "\"~s\", \"~s\", \"~s\"}.~n",
+      Fd, "{vacmAccess, ~p, ~p, ~w, ~w, ~w, "
+      "~p, ~p, ~p}.~n",
       [GroupName, Prefix, SecModel, SecLevel,
        Match, RV, WV, NV]);
 do_write_vacm_conf(
@@ -875,7 +875,7 @@ do_write_vacm_conf(
   {vacmViewTreeFamily,
    ViewIndex, ViewSubtree, ViewStatus, ViewMask}) ->
     io:format(
-      Fd, "{vacmViewTreeFamily, \"~s\", ~w, ~w, ~w}.~n",
+      Fd, "{vacmViewTreeFamily, ~p, ~w, ~w, ~w}.~n",
       [ViewIndex, ViewSubtree, ViewStatus, ViewMask]);
 do_write_vacm_conf(_Fd, Crap) ->
     error({bad_vacm_config, Crap}).
diff --git a/lib/snmp/test/modules.mk b/lib/snmp/test/modules.mk
index 87539f88f7..0f54e67c65 100644
--- a/lib/snmp/test/modules.mk
+++ b/lib/snmp/test/modules.mk
@@ -31,6 +31,7 @@ SUITE_MODULES = \
 	snmp_agent_mibs_test \
 	snmp_agent_nfilter_test \
 	snmp_agent_test \
+	snmp_agent_conf_test \
 	snmp_agent_test_lib \
 	snmp_manager_config_test \
 	snmp_manager_user \
diff --git a/lib/snmp/test/snmp_SUITE.erl b/lib/snmp/test/snmp_SUITE.erl
index 3b9219739b..05bd86253b 100644
--- a/lib/snmp/test/snmp_SUITE.erl
+++ b/lib/snmp/test/snmp_SUITE.erl
@@ -81,7 +81,8 @@ groups() ->
 		     {group, note_store_test}]},
      {agent, [],    [{group, mibs_test}, 
 		     {group, nfilter_test},
-		     {group, agent_test},
+             {group, agent_test},
+             {group, agent_conf_test},
 		     {group, snmpnet_test}]},
      {manager, [],  [{group, manager_config_test},
 		     {group, manager_user_test}, 
@@ -97,6 +98,7 @@ groups() ->
      {mibs_test,           [], [{snmp_agent_mibs_test,     all}]},
      {nfilter_test,        [], [{snmp_agent_nfilter_test,  all}]},
      {agent_test,          [], [{snmp_agent_test,          all}]},
+     {agent_conf_test,     [], [{snmp_agent_conf_test,     all}]},
      {snmpnet_test,        [], [{snmp_to_snmpnet_SUITE,    all}]},
      {manager_config_test, [], [{snmp_manager_config_test, all}]},
      {manager_user_test,   [], [{snmp_manager_user_test,   all}]},
diff --git a/lib/snmp/test/snmp_agent_conf_test.erl b/lib/snmp/test/snmp_agent_conf_test.erl
new file mode 100644
index 0000000000..0a22bd47d1
--- /dev/null
+++ b/lib/snmp/test/snmp_agent_conf_test.erl
@@ -0,0 +1,210 @@
+%%
+%% %CopyrightBegin%
+%%
+%% Copyright Ericsson AB 2003-2016. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%
+%% %CopyrightEnd%
+%%
+
+-module(snmp_agent_conf_test).
+
+%%----------------------------------------------------------------------
+%% Include files
+%%----------------------------------------------------------------------
+
+%-include_lib("test_server/include/test_server.hrl").
+%-include("snmp_test_lib.hrl").
+-include_lib("common_test/include/ct.hrl").
+
+-export([
+    all/0,
+    groups/0,
+    init_per_suite/1,
+    end_per_suite/1,
+
+    check_agent/1,
+    check_usm/1,
+    check_vacm/1
+    ]).
+
+
+all() -> [
+    check_agent,
+    check_usm,
+    check_vacm
+    ].
+
+
+groups() ->
+    [].
+
+
+init_per_suite(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    PrivSubdir = filename:join(PrivDir, "snmp_agent_conf_test"),
+    ok = filelib:ensure_dir(filename:join(PrivSubdir, "dummy")),
+    [{priv_subdir, PrivSubdir} | Config].
+
+end_per_suite(_Config) ->
+    ok.
+
+%%======================================================================
+%% Test data
+%%======================================================================
+
+engine_ids() -> [
+    "plain eid",
+    "here\"eid",
+    "comes\neid",
+    "trouble\0eid",
+    binary_to_list(<<"中国引擎标识符"/utf8>>)
+].
+
+snmp_admin_strings() -> [
+    "plain string",
+    "heres\"eid",
+    "trouble\neid",
+    binary_to_list(<<"中国引擎标识符"/utf8>>)
+].
+
+
+%%======================================================================
+%% Test functions
+%%======================================================================
+
+
+check_agent(Config) ->
+    Dir = ?config(priv_subdir, Config),
+    lists:foreach(
+        fun(EngineId) -> check_agent_by_engineid(Dir, EngineId) end,
+        engine_ids()
+    ),
+    ok.
+
+check_agent_by_engineid(Dir, EngineId) ->
+    WEntries = [
+        snmpa_conf:agent_entry(intAgentIpAddress, {0,0,0,0}),
+        snmpa_conf:agent_entry(intAgentUDPPort, 161),
+        snmpa_conf:agent_entry(snmpEngineMaxMessageSize, 484),
+        snmpa_conf:agent_entry(snmpEngineID, EngineId)
+    ],
+
+    ok = snmpa_conf:write_agent_config(Dir, WEntries),
+    {ok, REntries} = snmpa_conf:read_agent_config(Dir),
+
+    true = is_subset(WEntries, REntries),
+    ok.
+
+%%======================================================================
+
+check_usm(Config) ->
+    Dir = ?config(priv_subdir, Config),
+    EngineId = hd(engine_ids()),
+    UserName = hd(snmp_admin_strings()),
+    SecName  = hd(snmp_admin_strings()),
+
+    %% vary engine id
+    lists:foreach(
+        fun(EngineId_) -> check_usm_by_params(Dir, EngineId_, UserName, SecName) end,
+        engine_ids()
+    ),
+
+    %% vary user name
+    lists:foreach(
+        fun(UserName_) -> check_usm_by_params(Dir, EngineId, UserName_, SecName) end,
+        snmp_admin_strings()
+    ),
+
+    %% vary sec name
+    lists:foreach(
+        fun(SecName_) -> check_usm_by_params(Dir, EngineId, UserName, SecName_) end,
+        snmp_admin_strings()
+    ),
+
+    ok.
+
+check_usm_by_params(Dir, EngineId, UserName, SecName) ->
+    WEntries = [
+        snmpa_conf:usm_entry(
+            EngineId,
+            UserName,
+            SecName,
+            zeroDotZero,
+            usmNoAuthProtocol, % authproto
+            "", "",
+            usmNoPrivProtocol, % privproto
+            "", "", "",
+            [], %AuthKey
+            [])  %PrivKey
+    ],
+
+    ok = snmpa_conf:write_usm_config(Dir, WEntries),
+    {ok, REntries} = snmpa_conf:read_usm_config(Dir),
+
+    true = is_subset(WEntries, REntries),
+    ok.
+
+%%======================================================================
+
+check_vacm(Config) ->
+    Dir = ?config(priv_subdir, Config),
+
+    %% vary sec name
+    lists:foreach(
+        fun(SecName_) -> check_vacm_by_params(Dir, SecName_) end,
+        snmp_admin_strings()
+    ),
+
+    ok.
+
+
+check_vacm_by_params(Dir, SecName) ->
+    WEntries = [
+        %%                      SecModel, SecName, GroupName
+        snmpa_conf:vacm_s2g_entry(usm, SecName,  SecName),
+        %%                     GroupName,Prefix,SecModel,SecLevel,Match,ReadView,WriteView,NotifyView
+        snmpa_conf:vacm_acc_entry(SecName,  "", any, noAuthNoPriv, exact, "all", "all", "all")
+    ],
+
+    ok = snmpa_conf:write_vacm_config(Dir, WEntries),
+    {ok, REntries} = snmpa_conf:read_vacm_config(Dir),
+
+    true = is_subset(WEntries, REntries),
+    ok.
+
+
+
+%%======================================================================
+
+
+%% additional tests needed:
+% check_context()
+% check_community()
+% check_standard()
+% check_target_addr()
+% check_target_params()
+% check_notify()
+
+
+%%======================================================================
+%% Local utility functions
+%%======================================================================
+
+is_subset(List1, List2) ->
+    io:format("Check ~p is subset of ~p\n", [List1, List2]),
+    sets:is_subset(
+        sets:from_list(List1),
+        sets:from_list(List2)
+    ).