Fix issue with finishing handshake in ssl driver#30580
Merged
Tim-Brooks merged 2 commits intoelastic:masterfrom May 15, 2018
Merged
Fix issue with finishing handshake in ssl driver#30580Tim-Brooks merged 2 commits intoelastic:masterfrom
Tim-Brooks merged 2 commits intoelastic:masterfrom
Conversation
This is fixing an issue that has come up in some builds. In some scenarios I see an assertion failure that we are trying to move to application mode when we are not in handshake mode. What I think is happening is that we are in handshake mode and have received the completed handshake message AND an application message. While reading in handshake mode we switch to application mode. However, there is still data to be consumed so we attempt to continue to read in handshake mode. This leads to us attempting to move to application mode again throwing an assertion. This commit fixes this by immediatly exiting the handshake mode read method if we are not longer in handshake mode. Additionally if we swap modes during a read we attempt to read with the new mode to see if there is data that needs to be handled.
Collaborator
|
Pinging @elastic/es-security |
Contributor
Author
|
This might be the core issue causing this build failure (#30013). |
s1monw
approved these changes
May 14, 2018
| public void read(InboundChannelBuffer buffer) throws SSLException { | ||
| currentMode.read(buffer); | ||
| boolean continueReading = true; | ||
| while (continueReading) { |
Contributor
There was a problem hiding this comment.
maybe use do { } while (modePriorToRead != currentMode)?
jaymode
approved these changes
May 14, 2018
Member
jaymode
left a comment
jaymode
left a comment
There was a problem hiding this comment.
LGTM 2. I also had the same thought about using a do while
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
May 15, 2018
…ngs-to-true * elastic/master: [DOCS] Restores 7.0.0 release notes and highlights Remove assert statements from field caps documentation. (elastic#30601) Repository GCS plugin new client library (elastic#30168) HLRestClient: Follow-up for put index template api (elastic#30592) Unmute IndexUpgradeIT tests [DOCS] Remove references to changelog and to highlights Side-step pending deletes check (elastic#30571) [DOCS] Remove references to removed changelog Revert "Mute ML upgrade test (elastic#30458)" [ML] Adjust BWC version following backport of elastic#30125 [Docs] Improve section detailing translog usage (elastic#30573) [Tests] Relax allowed delta in extended_stats aggregation (elastic#30569) [ML] Reverse engineer Grok patterns from categorization results (elastic#30125) Update build file due to doc file rename Remove the changelog (elastic#30593) Fix issue with finishing handshake in ssl driver (elastic#30580) Fail if reading from closed KeyStoreWrapper (elastic#30394) Silence IndexUpgradeIT test failures. (elastic#30430)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This is fixing an issue that has come up in some builds. In some
scenarios I see an assertion failure that we are trying to move to
application mode when we are not in handshake mode. What I think is
happening is that we are in handshake mode and have received the
completed handshake message AND an application message. While reading in
handshake mode we switch to application mode. However, there is still
data to be consumed so we attempt to continue to read in handshake mode.
This leads to us attempting to move to application mode again throwing
an assertion.
This commit fixes this by immediatly exiting the handshake mode read
method if we are not longer in handshake mode. Additionally if we swap
modes during a read we attempt to read with the new mode to see if there
is data that needs to be handled.