Merge pull request #37 from zerth/adjust-ext-type-code-check

Adjust ext type code handling to match spec

Author: kuenishi

src/msgpack_packer.erl

@@ -374,19 +374,19 @@ pack_map(M, Opt)->
 -spec pack_ext(any(), msgpack_ext_packer(), msgpack:options()) -> {ok, binary()} | {error, any()}.
 pack_ext(Any, Packer, Opt) ->
     case Packer(Any, Opt) of
-        {ok, {Type, Data}} when 0 < Type andalso Type < 16#100 ->
+        {ok, {Type, Data}} when -16#80 =< Type andalso Type =< 16#7F ->
             Bin = case byte_size(Data) of
-                      1  -> <<16#D4, Type:8, Data/binary>>;
-                      2  -> <<16#D5, Type:8, Data/binary>>;
-                      4  -> <<16#D6, Type:8, Data/binary>>;
-                      8  -> <<16#D7, Type:8, Data/binary>>;
-                      16 -> <<16#D8, Type:8, Data/binary>>;
+                      1  -> <<16#D4, Type:1/signed-integer-unit:8, Data/binary>>;
+                      2  -> <<16#D5, Type:1/signed-integer-unit:8, Data/binary>>;
+                      4  -> <<16#D6, Type:1/signed-integer-unit:8, Data/binary>>;
+                      8  -> <<16#D7, Type:1/signed-integer-unit:8, Data/binary>>;
+                      16 -> <<16#D8, Type:1/signed-integer-unit:8, Data/binary>>;
                       Len when Len < 16#100 ->
-                          <<16#C7, Len:8, Type:8, Data/binary>>;
+                          <<16#C7, Len:8, Type:1/signed-integer-unit:8, Data/binary>>;
                       Len when Len < 16#10000 ->
-                          <<16#C8, Len:16, Type:8, Data/binary>>;
+                          <<16#C8, Len:16, Type:1/signed-integer-unit:8, Data/binary>>;
                       Len when Len < 16#100000000 ->
-                          <<16#C9, Len:32, Type:8, Data/binary>>
+                          <<16#C9, Len:32, Type:1/signed-integer-unit:8, Data/binary>>
                   end,
             {ok, Bin};
         {error, E} ->

src/msgpack_term.erl

@@ -21,7 +21,7 @@
          pack_ext/2, unpack_ext/3]).
 -behaviour(msgpack_ext).
 
--define(ERLANG_TERM, 131).
+-define(ERLANG_TERM, 127).
 -define(TERM_OPTION, [{enable_str,true},{ext,?MODULE},{allow_atom,none}]).
 
 %% @doc experimental

src/msgpack_unpacker.erl

@@ -126,42 +126,42 @@ unpack_stream(<<16#C1, _R/binary>>, _) ->  throw({badarg, 16#C1});
 %% for extention types
 
 %% fixext 1 stores an integer and a byte array whose length is 1 byte
-unpack_stream(<<16#D4, T:8, Data:1/binary, Rest/binary>>,
+unpack_stream(<<16#D4, T:1/signed-integer-unit:8, Data:1/binary, Rest/binary>>,
               ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
     maybe_unpack_ext(16#D4, Unpack, T, Data, Rest, Orig);
 
 %% fixext 2 stores an integer and a byte array whose length is 2 bytes
-unpack_stream(<<16#D5, T:8, Data:2/binary, Rest/binary>>,
+unpack_stream(<<16#D5, T:1/signed-integer-unit:8, Data:2/binary, Rest/binary>>,
               ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
     maybe_unpack_ext(16#D5, Unpack, T, Data, Rest, Orig);
 
 %% fixext 4 stores an integer and a byte array whose length is 4 bytes
-unpack_stream(<<16#D6, T:8, Data:4/binary, Rest/binary>>,
+unpack_stream(<<16#D6, T:1/signed-integer-unit:8, Data:4/binary, Rest/binary>>,
               ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
     maybe_unpack_ext(16#D6, Unpack, T, Data, Rest, Orig);
 
 %% fixext 8 stores an integer and a byte array whose length is 8 bytes
-unpack_stream(<<16#D7, T:8, Data:8/binary, Rest/binary>>,
+unpack_stream(<<16#D7, T:1/signed-integer-unit:8, Data:8/binary, Rest/binary>>,
               ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
     maybe_unpack_ext(16#D7, Unpack, T, Data, Rest, Orig);
 
 %% fixext 16 stores an integer and a byte array whose length is 16 bytes
-unpack_stream(<<16#D8, T:8, Data:16/binary, Rest/binary>>,
+unpack_stream(<<16#D8, T:1/signed-integer-unit:8, Data:16/binary, Rest/binary>>,
               ?OPTION{ext_unpacker=Unpack, original_list=Orig} = _Opt) ->
     maybe_unpack_ext(16#D8, Unpack, T, Data, Rest, Orig);
 
 %% ext 8 stores an integer and a byte array whose length is upto (2^8)-1 bytes:
-unpack_stream(<<16#C7, Len:8, Type:8, Data:Len/binary, Rest/binary>>,
+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) ->
     maybe_unpack_ext(16#C7, Unpack, Type, Data, Rest, Orig);
 
 %% ext 16 stores an integer and a byte array whose length is upto (2^16)-1 bytes:
-unpack_stream(<<16#C8, Len:16, Type:8, Data:Len/binary, Rest/binary>>,
+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) ->
     maybe_unpack_ext(16#C8, Unpack, Type, Data, Rest, Orig);
 
 %% ext 32 stores an integer and a byte array whose length is upto (2^32)-1 bytes:
-unpack_stream(<<16#C9, Len:32, Type:8, Data:Len/binary, Rest/binary>>,
+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)  ->
     maybe_unpack_ext(16#C9, Unpack, Type, Data, Rest, Orig);
 

test/msgpack_ext_example_tests.erl

@@ -70,3 +70,33 @@ behaviour_test() ->
     Opt = [{ext, ?MODULE}],
     Term = {native, {self(), make_ref(), foobar, fun() -> ok end}},
     {ok, Term} = msgpack:unpack(msgpack:pack(Term, Opt), Opt).
+
+
+ext_typecode_range_test() ->
+    %% typecode range from msgpack spec. [-128,-1] is the "reserved"
+    %% range, [0,127] is the "user-defined" range.
+    TypecodeMin = -128,
+    TypecodeMax = 127,
+    Packer = fun ({thing, N}, _) ->
+                     {ok, {N, msgpack:pack(N)}}
+             end,
+    Unpacker = fun(N, Bin, _) ->
+                       Result = msgpack:unpack(Bin),
+                       ?assertEqual({ok, N}, Result),
+                       Result
+               end,
+    Opt = [{ext,{Packer,Unpacker}}],
+    %% it should be possible to use an uncontroversial ext type code:
+    Enc = msgpack:pack({thing,1}, Opt),
+    ?assertMatch({ok, 1}, msgpack:unpack(Enc, Opt)),
+    %% it should be possible to use ext typecodes covering the entire
+    %% range specified in the msgpack specification:
+    [begin
+         Encoded = msgpack:pack({thing, N}, Opt),
+         Result = msgpack:unpack(Encoded, Opt),
+         ?assertMatch({ok, N}, Result)
+     end || N <- lists:seq(TypecodeMin,TypecodeMax)],
+    %% using codes outside the allowed range should fail:
+    [?assertError({case_clause, _}, msgpack:pack({thing, N}, Opt))
+     || N <- [-129, 128]],
+    ok.