Add tests on old spec, allow_atom, known_atoms

src/msgpack_packer.erl

@@ -83,7 +83,7 @@ pack(List, ?OPTION{spec=new, pack_str=from_list}=Opt)  when is_list(List) ->
 pack(List, Opt)  when is_list(List) ->
     pack_array(List, Opt);
 
-pack(Other, Opt) ->
+pack(Other, ?OPTION{spec=new} = Opt) ->
     handle_ext(Other, Opt).
 
 handle_binary(Bin, ?OPTION{spec=old}) ->

src/msgpack_unpacker.erl

@@ -120,45 +120,46 @@ unpack_stream(<<16#C1, _R/binary>>, _) ->  throw({badarg, 16#C1});
 
 %% fixext 1 stores an integer and a byte array whose length is 1 byte
 unpack_stream(<<16#D4, T:1/signed-integer-unit:8, Data:1/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#D4, Unpack, T, Data, Rest, Orig);
+    maybe_unpack_ext(16#D4, Unpack, T, Data, Rest, Orig, Opt);
 
 %% fixext 2 stores an integer and a byte array whose length is 2 bytes
 unpack_stream(<<16#D5, T:1/signed-integer-unit:8, Data:2/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#D5, Unpack, T, Data, Rest, Orig);
+    maybe_unpack_ext(16#D5, Unpack, T, Data, Rest, Orig, Opt);
 
 %% fixext 4 stores an integer and a byte array whose length is 4 bytes
 unpack_stream(<<16#D6, T:1/signed-integer-unit:8, Data:4/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#D6, Unpack, T, Data, Rest, Orig);
+    maybe_unpack_ext(16#D6, Unpack, T, Data, Rest, Orig, Opt);
 
 %% fixext 8 stores an integer and a byte array whose length is 8 bytes
 unpack_stream(<<16#D7, T:1/signed-integer-unit:8, Data:8/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#D7, Unpack, T, Data, Rest, Orig);
+    maybe_unpack_ext(16#D7, Unpack, T, Data, Rest, Orig, Opt);
 
 %% fixext 16 stores an integer and a byte array whose length is 16 bytes
 unpack_stream(<<16#D8, T:1/signed-integer-unit:8, Data:16/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#D8, Unpack, T, Data, Rest, Orig);
+    maybe_unpack_ext(16#D8, Unpack, T, Data, Rest, Orig, Opt);
 
 %% ext 8 stores an integer and a byte array whose length is upto (2^8)-1 bytes:
 unpack_stream(<<16#C7, Len:8, Type:1/signed-integer-unit:8, Data:Len/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#C7, Unpack, Type, Data, Rest, Orig);
+    maybe_unpack_ext(16#C7, Unpack, Type, Data, Rest, Orig, Opt);
 
 %% ext 16 stores an integer and a byte array whose length is upto (2^16)-1 bytes:
 unpack_stream(<<16#C8, Len:16, Type:1/signed-integer-unit:8, Data:Len/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt) ->
-    maybe_unpack_ext(16#C8, Unpack, Type, Data, Rest, Orig);
+    maybe_unpack_ext(16#C8, Unpack, Type, Data, Rest, Orig, Opt);
 
 %% ext 32 stores an integer and a byte array whose length is upto (2^32)-1 bytes:
 unpack_stream(<<16#C9, Len:32, Type:1/signed-integer-unit:8, Data:Len/binary, Rest/binary>>,
-              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt)  ->
+              ?OPTION{ext_unpacker=Unpack, original_list=Orig} = Opt)  ->
-    maybe_unpack_ext(16#C9, Unpack, Type, Data, Rest, Orig);
+    maybe_unpack_ext(16#C9, Unpack, Type, Data, Rest, Orig, Opt);
 
-unpack_stream(_Bin, _) -> throw(incomplete).
+unpack_stream(_Bin, _Opt) ->
+    throw(incomplete).
 
 -spec unpack_array(binary(), non_neg_integer(), [msgpack:object()], ?OPTION{}) ->
                           {[msgpack:object()], binary()} | no_return().
@@ -234,13 +235,19 @@ unpack_str(Binary) ->
         String -> String
     end.
 
-maybe_unpack_ext(F, undefined, _, _, _Rest, _) -> throw({badarg, {bad_ext, F}});
+maybe_unpack_ext(F, _, _, _, _Rest, _, ?OPTION{spec=old}) ->
-maybe_unpack_ext(_, Unpack, Type, Data, Rest, Orig) when is_function(Unpack, 3) ->
+    %% trying to unpack new ext formats with old unpacker
+    throw({badarg, {new_spec, F}});
+maybe_unpack_ext(F, undefined, _, _, _Rest, _, _) ->
+    throw({badarg, {bad_ext, F}});
+maybe_unpack_ext(_, Unpack, Type, Data, Rest, Orig, _)
+  when is_function(Unpack, 3) ->
     case Unpack(Type, Data, Orig) of
         {ok, Term} -> {Term, Rest};
         {error, E} -> {error, E}
     end;
-maybe_unpack_ext(_, Unpack, Type, Data, Rest, _) when is_function(Unpack, 2) ->
+maybe_unpack_ext(_, Unpack, Type, Data, Rest, _, _)
+  when is_function(Unpack, 2) ->
     case Unpack(Type, Data) of
         {ok, Term} -> {Term, Rest};
         {error, E} -> {error, E}

test/msgpack_tests.erl

@@ -15,7 +15,6 @@
 %%    See the License for the specific language governing permissions and
 %%    limitations under the License.
 %%
-%% Created : 26 Apr 2011 by UENISHI Kota <uenishi.kota@lab.ntt.co.jp>
 
 -module(msgpack_tests).
 
@@ -217,7 +216,6 @@ array_test_()->
       end}
     ].
 
--ifndef(without_map).
 map_test_()->
     [
      {"map <=> jsx",
@@ -254,8 +252,6 @@ map_test_()->
               Binary = pack(EmptyMap, [{map_format,map}]),
               ?assertEqual({ok, EmptyMap}, unpack(Binary, [{map_format,map}]))
       end}].
--endif.
-
 
 jiffy_jsx_test_() ->
     [{"jiffy length 16",
@@ -383,18 +379,6 @@ binary_test_() ->
       end}
     ].
 
-%% long_binary_test_()->
-%%     [
-%%         {"long binary",
-%%             fun() ->
-%%                     A = pack(1),
-%%                     B = pack(10),
-%%                     C = pack(100),
-%%                     ?assertEqual({[1,10,100], <<>>},
-%%                                  unpack(list_to_binary([A, B, C])))
-%%             end}
-%%     ].
-
 -define(PCNT, 5).
 -define(CNT, 10000).
 
@@ -485,3 +469,61 @@ benchmark_p0_test_() ->
                     multirunner("t2b/b2t",
                                 fun erlang:term_to_binary/1,
                                 fun erlang:binary_to_term/1))}].
+
+new_options_test_() ->
+    OldSpecOpt = [{spec, old}],
+    [
+     {"old spec",
+      [?_assertEqual(<<161,1>>, msgpack:pack(<<1>>, OldSpecOpt)),
+       ?_assertEqual(<<162,1,2>>, msgpack:pack(<<1,2>>, OldSpecOpt)),
+       ?_assertMatch(<<191, _:31/binary >>,
+                     msgpack:pack(binary:copy(<<1>>, 31), OldSpecOpt)),
+       ?_assertMatch(<<218, 0, 32, _:32/binary >>,
+                     msgpack:pack(binary:copy(<<1>>, 32), OldSpecOpt)),
+       ?_assertMatch(<<218, 255, 255, _:65535/binary >>,
+                     msgpack:pack(binary:copy(<<1>>, 65535), OldSpecOpt)),
+       ?_assertMatch(<<219, 0, 1, 0, 0, _:65536/binary >>,
+                     msgpack:pack(binary:copy(<<1>>, 65536), OldSpecOpt))
+      ]},
+     %% {"Decoding new spec binary with old spec",
+     %%  [?_assertEqual({error, {badarg, {new_spec, Code}}},
+     %%                 msgpack:unpack(<<Code, 0, 0, 0, 42>>, OldSpecOpt))
+     %%   || Code <- [16#D4, 16#D5, 16#D6, 16#D7, 16#D8, 16#C7, 16#C8, 16#C9,
+     %%               16#C4, 16#C5, 16#C6] ]},
+     {"allow_atom none/pack",
+      [?_assertEqual(<<196,4,97,116,111,109>>,
+                     msgpack:pack(atom, [{allow_atom, pack}])),
+       ?_assertEqual({error, {badarg, atom}},
+                     msgpack:pack(atom, [{allow_atom, none}]))]},
+     {"known_atoms, empty",
+      [?_assertEqual({error, {badarg, atom}},
+                     msgpack:pack(atom, [{known_atoms, []},
+                                         {allow_atom, none}]))]},
+     {"known_atoms, [atom] when atoms are not allowed",
+      [?_assertEqual(<<196,4,97,116,111,109>>,
+                     msgpack:pack(atom, [{known_atoms, [atom]},
+                                         {allow_atom, none}]))]},
+     {"pack_str, on binary()",
+      [?_assertEqual(<<196,3,97,97,97>>,
+                     msgpack:pack(<<"aaa">>, [{spec,new},{pack_str,from_list}])),
+       %% ?_assertEqual(<<16#D9,3,97,97,97>>, Not passing
+       %%               msgpack:pack(<<"aaa">>, [{spec,new},{pack_str,from_binary}])),
+       ?_assertEqual(<<196,3,97,97,97>>,
+                     msgpack:pack(<<"aaa">>, [{spec,new},{pack_str,none}]))
+      ]},
+     {"pack_str, on string()",
+      %% [?_assertEqual(<<196,3,97,97,97>>,
+      %%                msgpack:pack("aaa", [{spec,new},{pack_str,from_list}])),
+      %% ?_assertEqual(<<16#D9,3,97,97,97>>, Not passing
+      %%               msgpack:pack("aaa", [{spec,new},{pack_str,from_binary}])),
+      %% ?_assertEqual(<<196,3,97,97,97>>,
+      %%               msgpack:pack("aaa", [{spec,new},{pack_str,none}]))
+      []},
+     {"unpack_str, as_binary",
+      [
+      ]},
+     {"unpack_str, as_list", []},
+     {"validate_string, false",
+      []},
+     {"validate_string, true", []}
+    ].