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

fix response for the command that is too long #562

Merged
merged 1 commit into from
Sep 9, 2019
Merged

Conversation

ysmolski
Copy link
Member

@ysmolski ysmolski commented Sep 6, 2019

Before the patch, if a client sent too long command, server would
respond by BAD_FORMAT, followed by UNKNOWN_COMMAND. This resulted
in desync between client and server.

This patch adds another state STATE_WANT_ENDLINE,
whose purpose is to drop command line until the EOL is found.
Client can send as big command as he want and server will be able
to skip it and return only one error message: BAD_FORMAT.

Some STATE_* constants were renamed to improve readability.

Fixes #337

@codecov
Copy link

codecov bot commented Sep 6, 2019

Codecov Report

Merging #562 into master will decrease coverage by 0.01%.
The diff coverage is 86.84%.

@@            Coverage Diff             @@
##           master     #562      +/-   ##
==========================================
- Coverage   74.25%   74.23%   -0.02%     
==========================================
  Files          13       13              
  Lines        2288     2302      +14     
==========================================
+ Hits         1699     1709      +10     
- Misses        589      593       +4

@sergeyklay
Copy link
Contributor

@ysmolsky Should we update docs as well?

@ysmolski
Copy link
Member Author

ysmolski commented Sep 6, 2019

@ysmolsky Should we update docs as well?

Docs mention that already, but expanding it a little would not hurt I guess.
https://github.com/beanstalkd/beanstalkd/blob/master/doc/protocol.txt#L44

Before the patch, if a client sent too long command, server would
respond by BAD_FORMAT, followed by UNKNOWN_COMMAND. This resulted
in desync between client and server.

This patch adds another state STATE_WANT_ENDLINE,
whose purpose is to drop command line until the EOL is found.
Client can send as big command as he want and server will be able
to skip it and return only one error message: BAD_FORMAT.

Some STATE_* constants were renamed to improve readability.

Fixes #337
@ysmolski
Copy link
Member Author

ysmolski commented Sep 7, 2019

I have fixed the protocol regarding why BAD_FORMAT may be returned.

@thoro
Copy link

thoro commented Apr 16, 2020

@ysmolsky Is it on purpose you added a printf for each command that is executed?

https://github.com/beanstalkd/beanstalkd/pull/562/files#diff-5b9219f6cd269a2c85dc83291ca314a4R1288

@ysmolski
Copy link
Member Author

Oh wow! Thanks for spotting this. I am not sure how it got there, but it was not an intention.

ysmolski pushed a commit that referenced this pull request Apr 16, 2020
@tomponline
Copy link
Member

@ysmolsky maybe we should do another release with all of the fixes you've added since the last one?

@ysmolski
Copy link
Member Author

Yes, that would be awesome. Last release was almost 1 year ago...

ysmolski pushed a commit that referenced this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

a command without parameters should not yield UNKNOWN_COMMAND
4 participants