Fixed issues with leafnode compression negotiation #4230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a server would send an asynchronous INFO to a remote server it would incorrectly contain compression information that could cause issues with one side thinking that the connection should be compressed while the other side was not. It also caused the authentication timer to be incorrectly set which would cause a disconnect. Signed-off-by: Ivan Kozlovic <ijkozlovic@gmail.com>
| info.Compression = opts.LeafNode.Compression.Mode | ||
| // For tests that want to simulate old servers, do not set the compression | ||
| // on the INFO protocol if configured with CompressionNotSupported. | ||
| if cm := opts.LeafNode.Compression.Mode; cm != CompressionNotSupported { |
There was a problem hiding this comment.
Is there any value in checking that the proposed compression mode is one we understand?
There was a problem hiding this comment.
Not sure I understand the question. If it is "are we sure that opts.LeafNode.Compression.Mode is a valid value here?", then yes, it has been already validated on startup (or reload). The point of this test here, is that if we configure as "CompressionNotSupported" (likely in tests), then we don't include the Compression field in the INFO protocol, which makes this server behaves as an old server (protocol wise), since this field won't be set/present in old servers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a server would send an asynchronous INFO to a remote server it would incorrectly contain compression information that could cause issues with one side thinking that the connection should be compressed while the other side was not.
It also caused the authentication timer to be incorrectly set which would cause a disconnect.
Signed-off-by: Ivan Kozlovic ijkozlovic@gmail.com