Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERL-1319: SSL Flow Control Always True Predicate #4379

Closed
OTP-Maintainer opened this issue Jul 28, 2020 · 1 comment
Closed

ERL-1319: SSL Flow Control Always True Predicate #4379

OTP-Maintainer opened this issue Jul 28, 2020 · 1 comment
Labels
bug Issue is reported as a bug priority:low team:PS Assigned to OTP team PS
Milestone

Comments

@OTP-Maintainer
Copy link

Original reporter: JIRAUSER16003
Affected version: OTP-22.3.4.4
Fixed in version: OTP-23.1
Component: ssl
Migrated from: https://bugs.erlang.org/browse/ERL-1319


 

 

 
{code:java}
flow_ctrl(#state{user_data_buffer = {_,Size,_},
                 socket_options = #socket_options{active = false,
                                                  packet = Packet},
                 bytes_to_read = undefined} = State)  when ((Packet =/= 0) orelse (Packet =/= raw))
                                                           andalso Size =/= 0 ->
    %% We need more data to complete the packet.
    activate_socket(State);
flow_ctrl(#state{user_data_buffer = {_,Size,_},
                 socket_options = #socket_options{active = false,
                                                  packet = Packet},
                 bytes_to_read = BytesToRead} = State)  when ((Packet =/= 0) orelse (Packet =/= raw)) ->
    case (Size >= BytesToRead andalso Size =/= 0) of
        true -> %% There is enough data bufferd
            {no_record, State};
        false -> %% We need more data to complete the packet of <BytesToRead> size
            activate_socket(State)
    end;{code}
 

lib/ssl/src/tls_connection.erl

 

I'm not sure if this is a bug or not but the clause {{((Packet =/= 0) orelse (Packet =/= raw)) looks like it is always true and I'm not sure if that is what is intended.}}

{{ie:}}

1> Packet = raw, ((Packet =/= 0) orelse (Packet =/= raw)).
true

3> Packet = 0, ((Packet =/= 0) orelse (Packet =/= raw)).
true

5> Packet = 5, ((Packet =/= 0) orelse (Packet =/= raw)).
true

 Having a guess I think we would want:

(Packet =/= 0) andalso (Packet =/= raw)

 

 

 

 
@OTP-Maintainer
Copy link
Author

ingela said:

Thank you for reporting this. I think you are correct it should be andalso. I am testing it in our builds now and I am hoping that this actually solves a problem that only shows up on one very special machine.  Seems this corner case is not that common at the moment but timing could change and make it a bigger problem for people using ssl:recv/*

@OTP-Maintainer OTP-Maintainer added bug Issue is reported as a bug team:PS Assigned to OTP team PS priority:low labels Feb 10, 2021
@OTP-Maintainer OTP-Maintainer added this to the OTP-23.1 milestone Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug priority:low team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

1 participant