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

Really fix ftp loop #2436

Closed
wants to merge 2 commits into from
Closed

Conversation

richcarl
Copy link
Contributor

@richcarl richcarl commented Oct 29, 2019

The previous fix in 9a1c36c was not sufficient, and the code was still looping, eventually consuming all available memory. This new patch has been verified in production running many thousands of ftp transfers with no problems.

This test code can be used to reproduce a case where this happens, with a mimic of a broken control sequence:

{ok, P} = gen_tcp:listen(0,[]).
rr(ftp).
ftp:handle_info({tcp, P, <<$\r>>}, #state{csock={tcp,P}, ctrl_data={<<$\r, $\n, 0,0,2>>,[],{2,0,0}}} ).

The previous fix was insufficient, not using the updated state for the
recursive call.
Fix variable bindings, so as not to compare the whole ctrl_data tuple to
the binary data only.
@HansN HansN self-assigned this Oct 29, 2019
@HansN HansN added fix team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Oct 29, 2019
@HansN
Copy link
Contributor

HansN commented Oct 30, 2019

Thanks for the PR! I've rebased and merged it to maint and master so it will appear in 22.2 and 23.0.
It will also be released in 21.3.8.11

@HansN HansN closed this Oct 30, 2019
@richcarl richcarl deleted the really-fix-ftp-loop branch October 30, 2019 14:36
@richcarl
Copy link
Contributor Author

This patch is in 21 and master but never made it to maint-22, so it's not yet been included in any OTP 22 release.

@HansN
Copy link
Contributor

HansN commented Nov 29, 2019

It is available also in maint (as all maint-* patches) and will therefor be released in OTP-22.2, planned to be released 2019-12-11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants