[erlang-questions] Intended behaviour in ssl code?

[Essien Essien]

On Mon, May 25, 2009 at 11:54 AM, Dan Gudmundsson <dgud@REDACTED> wrote:
> new_ssl worked really bad in R12B, please try R13B,
> I have some additional pending bug-fixes which will come in the next
> patch-release.

Ok. thnx for clarifying.
FWIW, R13B implementation looks clean and that particular problem is not there.

cheers,
Essien

>
> /Dan
>
> Essien Essien wrote:
>>
>> Hi all,
>>
>> I have found some questionable code, which gives me undesirable
>> behaviour when using new_ssl with ssl-3.9 that comes with R12B3.
>>
>>  If I ask for 10 bytes and a {recv, 10} is sent, to the gen_fsm in
>> ssl_connection.erl and BytesToRead in the #state{} record will be
>> properly set to 10. Supposing the next time that data arrives, only 6
>> bytes come in, so I still need 4 bytes to get my 10 bytes. The code
>> will currently, buffer the 6 bytes that it has already read, and set
>> BytesToRead to 4. This will cause a problem the next time that data
>> comes in, b/cos suddently, all I'm looking for is 4 bytes, which will
>> be returned.
>>
>> The actual culprit is the second case clause in the second clause of
>> deliver_application:
>>
>> deliver_application_data(Pid, Buffer, Active, _, 0, _, Mode, _)
>>  when Active =/= false ->
>>    send_user(Pid, user_data(Active, Buffer, Mode)),
>>    {<<>>, 0, reply};
>>
>> deliver_application_data(Pid, Buffer, Active, NewDataSize, BytesToRead,
>> From,
>>                         Mode, BytesToStrip) ->
>>    case Buffer of
>>         % This is where BytesToRead comes back to byte us
>>         % It only starts hurting the second time around, when
>>         % the wrong value for BytesToRead becomes available
>>        <<Read:BytesToRead/binary, Rest/binary>> ->
>>            <<_:BytesToStrip/binary, Data/binary>> = Read,
>>            send_or_reply(Active, Pid, From, user_data(Active, Data,
>> Mode)),
>>            {Rest, 0, reply};
>>        _ ->
>>            % Here BytesToRead - NewDataSize is problematic
>>            {Buffer, BytesToRead - NewDataSize, no_reply}
>>    end.
>>
>> This function is called in application_data:
>>
>> application_data(Data, #state{user_application = Pid,
>>                              socket_options = SocketOptions,
>>                              bytes_to_read = BytesToRead0,
>>                              from = From,
>>                              user_data_buffer = Buffer0} = State0) ->
>>    #socket_options{active = Active, packet = Packet} = SocketOptions,
>>    Mode = get_mode(SocketOptions),
>>    Buffer1 = <<Buffer0/binary, Data/binary>>,
>>    {BytesToRead1, BytesToStrip} =
>>        check_packet(Packet,  Buffer1, BytesToRead0),
>>    BytesToRead2 = check_passive_0(Active, BytesToRead1, size(Buffer1)),
>>
>>    % Here deliver_applicatoin_data is called and the wrong value
>>    % for BytesToRead is stored in the state, from where it will
>>    % come back for us.
>>    {Buffer, BytesToRead, Replied} =
>>        deliver_application_data(Pid, Buffer1, Active, size(Data),
>>                                 BytesToRead2, From, Mode, BytesToStrip),
>>    State = State0#state{user_data_buffer = Buffer,
>>                         bytes_to_read = BytesToRead},
>>    case {Replied, Active, Buffer} of
>>        {no_reply, _, _} ->  % no reply, we need more data
>>            next_record(State);
>>        {reply, once, _} ->  % reply, once, we set active false
>>            State#state{socket_options =
>>                        SocketOptions#socket_options{active = false}};
>>        {reply, false, _} ->  % reply and passive, nothing more needed
>>            State#state{from = undefined};
>>        {reply, true, <<>>} -> % reply and empty buffer, we need more data
>>            next_record(State);
>>        {reply, true, _} -> % reply and data left in buffer continue
>> processing
>>            application_data(<<>>, State)
>>    end.
>>
>> To solve this, I have changed value returned by the second clause of
>> the case in deliver_application_data to: {Buffer, BytesToRead,
>> no_reply}
>>
>> Which ensures that BytesToRead is always set to what the caller
>> intends. I have tested this with various scenarios and they all work
>> as expected.
>>
>> Is this really a bug?
>>
>> cheers,
>> Essien
>> _______________________________________________
>> erlang-questions mailing list
>> erlang-questions@REDACTED
>> http://www.erlang.org/mailman/listinfo/erlang-questions
>>
>