THRIFT-4266 Erlang library throws during skipping fields of composite type (maps, lists, structs, sets)
Client: Erlang
Patch: David Hull <david.hull@openx.com>
This closes #1316
diff --git a/.gitignore b/.gitignore
index 00cf8bb..bdb84ea 100644
--- a/.gitignore
+++ b/.gitignore
@@ -207,13 +207,15 @@
/lib/delphi/test/typeregistry/*.identcache
/lib/delphi/test/typeregistry/*.local
/lib/delphi/test/typeregistry/*.dcu
-/lib/erl/.generated
/lib/erl/.eunit
-/lib/erl/ebin
+/lib/erl/.generated
+/lib/erl/.rebar/
/lib/erl/deps/
+/lib/erl/ebin
/lib/erl/src/thrift.app.src
-/lib/erl/test/*.hrl
/lib/erl/test/*.beam
+/lib/erl/test/*.hrl
+/lib/erl/test/Thrift_omit_without.thrift
/lib/haxe/test/bin
/lib/hs/dist
/lib/java/build
diff --git a/lib/erl/Makefile.am b/lib/erl/Makefile.am
index 92e2204..20aadeb 100644
--- a/lib/erl/Makefile.am
+++ b/lib/erl/Makefile.am
@@ -18,7 +18,9 @@
#
THRIFT = ../../compiler/cpp/thrift
+THRIFT_OMIT_FILE = test/Thrift_omit_without.thrift
THRIFT_FILES = $(wildcard test/*.thrift) \
+ $(THRIFT_OMIT_FILE) \
../../test/ConstantsDemo.thrift \
../../test/NameConflictTest.thrift \
../../test/ThriftTest.thrift
@@ -33,6 +35,10 @@
ERL_FLAG_LEGACY = erl:legacynames
ERL_FLAG_MAPS = erl:maps
endif
+
+$(THRIFT_OMIT_FILE): test/Thrift_omit_with.thrift
+ grep -v omit $< >$@
+
.generated: $(THRIFT) $(THRIFT_FILES)
for f in $(THRIFT_FILES) ; do \
$(THRIFT) --gen $(ERL_FLAG) -o test $$f ; \
@@ -65,6 +71,7 @@
clean:
rm -f .generated
rm -rf test/gen-erl/
+ rm -f $(THRIFT_OMIT_FILE)
$(REBAR) clean
maintainer-clean-local:
diff --git a/lib/erl/include/thrift_protocol.hrl b/lib/erl/include/thrift_protocol.hrl
index f85f455..bc0acc8 100644
--- a/lib/erl/include/thrift_protocol.hrl
+++ b/lib/erl/include/thrift_protocol.hrl
@@ -20,12 +20,12 @@
-ifndef(THRIFT_PROTOCOL_INCLUDED).
-define(THRIFT_PROTOCOL_INCLUDED, true).
--record(protocol_message_begin, {name, type, seqid}).
--record(protocol_struct_begin, {name}).
--record(protocol_field_begin, {name, type, id}).
--record(protocol_map_begin, {ktype, vtype, size}).
--record(protocol_list_begin, {etype, size}).
--record(protocol_set_begin, {etype, size}).
+-record(protocol_message_begin, {name :: string(), type :: integer(), seqid :: integer()}).
+-record(protocol_struct_begin, {name :: string()}).
+-record(protocol_field_begin, {name :: string(), type :: integer(), id :: integer()}).
+-record(protocol_map_begin, {ktype :: integer(), vtype :: integer(), size :: integer()}).
+-record(protocol_list_begin, {etype :: integer(), size :: integer()}).
+-record(protocol_set_begin, {etype :: integer(), size :: integer()}).
-type tprot_header_val() :: #protocol_message_begin{}
| #protocol_struct_begin{}
diff --git a/lib/erl/src/thrift_protocol.erl b/lib/erl/src/thrift_protocol.erl
index dc3bfef..2fe10d6 100644
--- a/lib/erl/src/thrift_protocol.erl
+++ b/lib/erl/src/thrift_protocol.erl
@@ -219,12 +219,11 @@
end.
skip_field(FType, IProto0, SDict, RTuple) ->
- FTypeAtom = thrift_protocol:typeid_to_atom(FType),
- {IProto1, ok} = thrift_protocol:skip(IProto0, FTypeAtom),
+ {IProto1, ok} = skip(IProto0, typeid_to_atom(FType)),
{IProto2, ok} = read(IProto1, field_end),
read_struct_loop(IProto2, SDict, RTuple).
--spec skip(#protocol{}, any()) -> {#protocol{}, ok}.
+-spec skip(#protocol{}, atom()) -> {#protocol{}, ok}.
skip(Proto0, struct) ->
{Proto1, ok} = read(Proto0, struct_begin),
@@ -261,7 +260,7 @@
?tType_STOP ->
{Proto1, ok};
_Else ->
- {Proto2, ok} = skip(Proto1, Type),
+ {Proto2, ok} = skip(Proto1, typeid_to_atom(Type)),
{Proto3, ok} = read(Proto2, field_end),
skip_struct_loop(Proto3)
end.
@@ -271,8 +270,8 @@
size = Size}) ->
case Size of
N when N > 0 ->
- {Proto1, ok} = skip(Proto0, Ktype),
- {Proto2, ok} = skip(Proto1, Vtype),
+ {Proto1, ok} = skip(Proto0, typeid_to_atom(Ktype)),
+ {Proto2, ok} = skip(Proto1, typeid_to_atom(Vtype)),
skip_map_loop(Proto2,
Map#protocol_map_begin{size = Size - 1});
0 -> {Proto0, ok}
@@ -282,7 +281,7 @@
size = Size}) ->
case Size of
N when N > 0 ->
- {Proto1, ok} = skip(Proto0, Etype),
+ {Proto1, ok} = skip(Proto0, typeid_to_atom(Etype)),
skip_set_loop(Proto1,
Map#protocol_set_begin{size = Size - 1});
0 -> {Proto0, ok}
@@ -292,7 +291,7 @@
size = Size}) ->
case Size of
N when N > 0 ->
- {Proto1, ok} = skip(Proto0, Etype),
+ {Proto1, ok} = skip(Proto0, typeid_to_atom(Etype)),
skip_list_loop(Proto1,
Map#protocol_list_begin{size = Size - 1});
0 -> {Proto0, ok}
diff --git a/lib/erl/test/Thrift_omit_with.thrift b/lib/erl/test/Thrift_omit_with.thrift
new file mode 100644
index 0000000..8bffc7c
--- /dev/null
+++ b/lib/erl/test/Thrift_omit_with.thrift
@@ -0,0 +1,22 @@
+struct test1 {
+ 1: i32 one
+ 2: i32 two // omit
+ 3: i32 three
+}
+
+struct test2 {
+ 1: i32 one
+ 2: test2 two // omit
+ 3: i32 three
+}
+
+struct test3 {
+ 1: i32 one
+ 2: list<test1> two // omit
+}
+
+struct test4 {
+ 1: i32 one
+ 2: map<i32,test1> two // omit
+}
+
diff --git a/lib/erl/test/test_omit.erl b/lib/erl/test/test_omit.erl
new file mode 100644
index 0000000..80841e2
--- /dev/null
+++ b/lib/erl/test/test_omit.erl
@@ -0,0 +1,79 @@
+-module(test_omit).
+
+-include("gen-erl/thrift_omit_with_types.hrl").
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+omit_struct1_test() ->
+ %% In this test, the field that is deleted is a basic type (an i32).
+ A = #test1{one = 1, three = 3},
+ B = #test1{one = 1, two = 2, three = 3},
+ {ok, Transport} = thrift_membuffer_transport:new(),
+ {ok, P0} = thrift_binary_protocol:new(Transport),
+
+ {P1, ok} = thrift_protocol:write(P0, {{struct, {thrift_omit_with_types, element(1, A)}}, A}),
+ {P2, {ok, O0}} = thrift_protocol:read(P1, {struct, {thrift_omit_without_types, element(1, A)}}),
+ ?assertEqual(element(1, A), element(1, O0)),
+ ?assertEqual(element(2, A), element(2, O0)),
+ ?assertEqual(element(4, A), element(3, O0)),
+
+ {P3, ok} = thrift_protocol:write(P2, {{struct, {thrift_omit_with_types, element(1, B)}}, B}),
+ {_P4, {ok, O1}} = thrift_protocol:read(P3, {struct, {thrift_omit_without_types, element(1, A)}}),
+ ?assertEqual(element(1, A), element(1, O1)),
+ ?assertEqual(element(2, A), element(2, O1)),
+ ?assertEqual(element(4, A), element(3, O1)),
+
+ ok.
+
+omit_struct2_test() ->
+ %% In this test, the field that is deleted is a struct.
+ A = #test2{one = 1, two = #test2{one = 10, three = 30}, three = 3},
+ B = #test2{one = 1, two = #test2{one = 10, two = #test2{one = 100}, three = 30}, three = 3},
+
+ {ok, Transport} = thrift_membuffer_transport:new(),
+ {ok, P0} = thrift_binary_protocol:new(Transport),
+
+ {P1, ok} = thrift_protocol:write(P0, {{struct, {thrift_omit_with_types, element(1, A)}}, A}),
+ {P2, {ok, O0}} = thrift_protocol:read(P1, {struct, {thrift_omit_without_types, element(1, A)}}),
+ ?assertEqual(element(1, A), element(1, O0)),
+ ?assertEqual(element(2, A), element(2, O0)),
+ ?assertEqual(element(4, A), element(3, O0)),
+
+ {P3, ok} = thrift_protocol:write(P2, {{struct, {thrift_omit_with_types, element(1, B)}}, B}),
+ {_P4, {ok, O1}} = thrift_protocol:read(P3, {struct, {thrift_omit_without_types, element(1, A)}}),
+ ?assertEqual(element(1, A), element(1, O1)),
+ ?assertEqual(element(2, A), element(2, O1)),
+ ?assertEqual(element(4, A), element(3, O1)),
+
+ ok.
+
+omit_list_test() ->
+ %% In this test, the field that is deleted is a list.
+ A = #test1{one = 1, two = 2, three = 3},
+ B = #test3{one = 1, two = [ A ]},
+
+ {ok, Transport} = thrift_membuffer_transport:new(),
+ {ok, P0} = thrift_binary_protocol:new(Transport),
+
+ {P1, ok} = thrift_protocol:write(P0, {{struct, {thrift_omit_with_types, element(1, B)}}, B}),
+ {_P2, {ok, O0}} = thrift_protocol:read(P1, {struct, {thrift_omit_without_types, element(1, B)}}),
+ ?assertEqual(element(2, B), element(2, O0)),
+
+ ok.
+
+omit_map_test() ->
+ %% In this test, the field that is deleted is a map.
+ A = #test1{one = 1, two = 2, three = 3},
+ B = #test4{one = 1, two = dict:from_list([ {2, A} ])},
+
+ {ok, Transport} = thrift_membuffer_transport:new(),
+ {ok, P0} = thrift_binary_protocol:new(Transport),
+
+ {P1, ok} = thrift_protocol:write(P0, {{struct, {thrift_omit_with_types, element(1, B)}}, B}),
+ {_P2, {ok, O0}} = thrift_protocol:read(P1, {struct, {thrift_omit_without_types, element(1, B)}}),
+ ?assertEqual(element(2, B), element(2, O0)),
+
+ ok.
+
+-endif. %% TEST