[erlang-questions] Intended behaviour in ssl code?

[Essien Essien]

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