Skip to content

Commit c83a97a

Browse files
committed
Fix bug in replica chunk acceptance when filters are used.
If osiris_replica:parse_chunk ended up parsing an chunk iolist where the first element in the list is exactly 48 bytes (the size of the osiris chunk header) it would end up crashing the call to osiris_log:accept_chunk as this assumed the filter data, when included, would always be in the first list element.
1 parent 1240e63 commit c83a97a

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

src/osiris_log.erl

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -778,13 +778,12 @@ accept_chunk([<<?MAGIC:4/unsigned,
778778
_TrailerSize:32/unsigned,
779779
FilterSize:8/unsigned,
780780
_Reserved:24,
781-
_Filter:FilterSize/binary,
782781
Data/binary>>
783782
| DataParts] =
784783
Chunk,
785784
#?MODULE{cfg = #cfg{}, mode = #write{tail_info = {Next, _}}} =
786785
State0) ->
787-
DataAndTrailer = [Data | DataParts],
786+
DataAndTrailer = skip(FilterSize, [Data | DataParts]),
788787
validate_crc(Next, Crc, part(DataSize, DataAndTrailer)),
789788
%% assertion
790789
% true = iolist_size(DataAndTrailer) == (DataSize + TrailerSize),
@@ -2788,6 +2787,17 @@ part(Len, [B | L]) when Len > 0 ->
27882787
[binary:part(B, {0, Len})]
27892788
end.
27902789

2790+
skip(0, L) ->
2791+
L;
2792+
skip(Len, [B | L]) when Len > 0 ->
2793+
S = byte_size(B),
2794+
case Len < S of
2795+
true ->
2796+
[binary:part(B, {S, Len - S}) | L];
2797+
false ->
2798+
skip(Len - S, L)
2799+
end.
2800+
27912801
-spec recover_tracking(state()) ->
27922802
osiris_tracking:state().
27932803
recover_tracking(#?MODULE{cfg = #cfg{directory = Dir,
@@ -3232,6 +3242,7 @@ list_dir(Dir) ->
32323242
end.
32333243

32343244
-ifdef(TEST).
3245+
-include_lib("eunit/include/eunit.hrl").
32353246

32363247

32373248
part_test() ->
@@ -3240,4 +3251,12 @@ part_test() ->
32403251
[<<"AB">>, <<"CDEF">>] = part(6, [<<"AB">>, <<"CDEF">>]),
32413252
ok.
32423253

3254+
skip_test() ->
3255+
[<<"EF">>] = skip(4, [<<"ABCDEF">>]),
3256+
[<<"B">>, <<"CDEF">>] = skip(1, [<<"AB">>, <<"CDEF">>]),
3257+
[<<"CDEF">>] = skip(2, [<<"AB">>, <<"CDEF">>]),
3258+
[<<"DEF">>] = skip(3, [<<"AB">>, <<"CDEF">>]),
3259+
[<<"AB">>, <<"CDEF">>] = skip(0, [<<"AB">>, <<"CDEF">>]),
3260+
ok.
3261+
32433262
-endif.

test/osiris_log_SUITE.erl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ all_tests() ->
7272
% truncate_multi_segment,
7373
accept_chunk,
7474
accept_chunk_inital_offset,
75+
accept_chunk_iolist_header_in_first_element,
7576
init_acceptor_truncates_tail,
7677
accept_chunk_truncates_tail,
7778
accept_chunk_does_not_truncate_tail_in_same_epoch,
@@ -1086,6 +1087,19 @@ accept_chunk_inital_offset(Config) ->
10861087

10871088
ok.
10881089

1090+
accept_chunk_iolist_header_in_first_element(Config) ->
1091+
ok = logger:set_primary_config(level, all),
1092+
Conf = ?config(osiris_conf, Config),
1093+
1094+
1095+
<<Head:48/binary, Res/binary>> = fake_chunk([{<<"filter">>, <<"blob1">>}],
1096+
?LINE, 1, 100),
1097+
Ch1 = [Head, Res],
1098+
F0 = osiris_log:init(Conf#{initial_offset => 100}, acceptor),
1099+
%% this should not crash
1100+
_F1 = osiris_log:accept_chunk(Ch1, F0),
1101+
ok.
1102+
10891103
accept_chunk(Config) ->
10901104
ok = logger:set_primary_config(level, all),
10911105
Conf = ?config(osiris_conf, Config),

0 commit comments

Comments
 (0)