This is an automated email from the ASF dual-hosted git repository.
upthewaterspout pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 5063aca GEODE-4007: Authentication/Handshake errors should close the
socket
5063aca is described below
commit 5063aca4b2b1a1ff43d448cd7b24f17355373a50
Author: Galen O'Sullivan <[email protected]>
AuthorDate: Tue Nov 21 17:38:06 2017 -0800
GEODE-4007: Authentication/Handshake errors should close the socket
This will cause the connection to be closed whenever a handshake or
authentication message fails.
The connection will also be broken if we ever receive an unexpected
handshake or
authenticantication message.
Signed-off-by: Galen O'Sullivan <[email protected]>
---
.../protocol/operations/OperationHandler.java | 7 ++-
.../ConnectionShiroAuthorizingStateProcessor.java | 3 +-
.../protocol/state/ConnectionStateProcessor.java | 9 ++++
.../ConnectionTerminatingStateProcessor.java} | 21 ++++++--
.../OperationNotAuthorizedException.java} | 10 ++--
.../client/protocol/ClientProtocolProcessor.java | 5 ++
.../sockets/GenericProtocolServerConnection.java | 4 ++
.../protobuf/v1/ProtobufCachePipeline.java | 5 ++
.../protobuf/v1/ProtobufLocatorPipeline.java | 6 +++
.../protocol/protobuf/v1/ProtobufOpsProcessor.java | 11 +++-
.../HandshakeRequestOperationHandler.java | 14 ++---
.../AuthenticationRequestOperationHandler.java | 38 ++++---------
.../protobuf/v1/AuthenticationIntegrationTest.java | 17 ++++++
.../protobuf/v1/HandshakeIntegrationTest.java | 62 ++++++++++++++++++++++
.../CacheConnectionTimeoutJUnitTest.java | 15 +++++-
.../GetAllRequestOperationHandlerJUnitTest.java | 7 +--
...tAvailableServersOperationHandlerJUnitTest.java | 7 ++-
...egionNamesRequestOperationHandlerJUnitTest.java | 2 +-
.../HandshakeRequestOperationHandlerJUnitTest.java | 56 ++++++++++++-------
19 files changed, 221 insertions(+), 78 deletions(-)
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/operations/OperationHandler.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/operations/OperationHandler.java
index 841976d..dc1fe2e 100644
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/operations/OperationHandler.java
+++
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/operations/OperationHandler.java
@@ -19,6 +19,7 @@ import
org.apache.geode.internal.exception.InvalidExecutionContextException;
import org.apache.geode.internal.protocol.MessageExecutionContext;
import org.apache.geode.internal.protocol.Result;
import org.apache.geode.internal.protocol.serialization.SerializationService;
+import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
/**
* This interface is implemented by a object capable of handling request types
'Req' and returning
@@ -30,7 +31,11 @@ public interface OperationHandler<Req, Resp, ErrorResp> {
/**
* Decode the message, deserialize contained values using the serialization
service, do the work
* indicated on the provided cache, and return a response.
+ *
+ * @throws ConnectionStateException if the connection is in an invalid state
for the operation in
+ * question.
*/
Result<Resp, ErrorResp> process(SerializationService serializationService,
Req request,
- MessageExecutionContext messageExecutionContext) throws
InvalidExecutionContextException;
+ MessageExecutionContext messageExecutionContext)
+ throws InvalidExecutionContextException, ConnectionStateException;
}
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionShiroAuthorizingStateProcessor.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionShiroAuthorizingStateProcessor.java
index cd2c6cc..c7c71aa 100644
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionShiroAuthorizingStateProcessor.java
+++
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionShiroAuthorizingStateProcessor.java
@@ -21,6 +21,7 @@ import
org.apache.geode.internal.protocol.MessageExecutionContext;
import org.apache.geode.internal.protocol.OperationContext;
import org.apache.geode.internal.protocol.ProtocolErrorCode;
import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
+import
org.apache.geode.internal.protocol.state.exception.OperationNotAuthorizedException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.security.NotAuthorizedException;
@@ -42,7 +43,7 @@ public class ConnectionShiroAuthorizingStateProcessor
implements ConnectionState
securityService.authorize(operationContext.getAccessPermissionRequired());
} catch (NotAuthorizedException e) {
messageContext.getStatistics().incAuthorizationViolations();
- throw new
ConnectionStateException(ProtocolErrorCode.AUTHORIZATION_FAILED,
+ throw new
OperationNotAuthorizedException(ProtocolErrorCode.AUTHORIZATION_FAILED,
"The user is not authorized to complete this operation");
} finally {
threadState.restore();
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionStateProcessor.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionStateProcessor.java
index 321120d..e0d18b3 100644
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionStateProcessor.java
+++
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionStateProcessor.java
@@ -58,4 +58,13 @@ public interface ConnectionStateProcessor {
throw new ConnectionStateException(ProtocolErrorCode.UNSUPPORTED_OPERATION,
"Requested operation not allowed at this time");
}
+
+ /**
+ * This indicates whether this state is capable of receiving any more
messages
+ *
+ * @return True if the socket should be closed
+ */
+ default boolean socketProcessingIsFinished() {
+ return false;
+ }
}
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionTerminatingStateProcessor.java
similarity index 50%
copy from
geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
copy to
geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionTerminatingStateProcessor.java
index e799522..d1b47ec 100644
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
+++
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/ConnectionTerminatingStateProcessor.java
@@ -12,12 +12,23 @@
* or implied. See the License for the specific language governing permissions
and limitations under
* the License.
*/
-package org.apache.geode.internal.protocol.security.exception;
+package org.apache.geode.internal.protocol.state;
-import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.internal.protocol.MessageExecutionContext;
+import org.apache.geode.internal.protocol.OperationContext;
+import org.apache.geode.internal.protocol.ProtocolErrorCode;
+import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
-public class IncompatibleAuthenticationMechanismsException extends
AuthenticationFailedException {
- public IncompatibleAuthenticationMechanismsException(String message) {
- super(message);
+public class ConnectionTerminatingStateProcessor implements
ConnectionStateProcessor {
+ @Override
+ public void validateOperation(MessageExecutionContext messageContext,
+ OperationContext operationContext) throws ConnectionStateException {
+ throw new ConnectionStateException(ProtocolErrorCode.GENERIC_FAILURE,
+ "This connection has been marked as terminating.");
+ }
+
+ @Override
+ public boolean socketProcessingIsFinished() {
+ return true;
}
}
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/exception/OperationNotAuthorizedException.java
similarity index 70%
rename from
geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
rename to
geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/exception/OperationNotAuthorizedException.java
index e799522..a06002b 100644
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
+++
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/state/exception/OperationNotAuthorizedException.java
@@ -12,12 +12,12 @@
* or implied. See the License for the specific language governing permissions
and limitations under
* the License.
*/
-package org.apache.geode.internal.protocol.security.exception;
+package org.apache.geode.internal.protocol.state.exception;
-import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.internal.protocol.ProtocolErrorCode;
-public class IncompatibleAuthenticationMechanismsException extends
AuthenticationFailedException {
- public IncompatibleAuthenticationMechanismsException(String message) {
- super(message);
+public class OperationNotAuthorizedException extends ConnectionStateException {
+ public OperationNotAuthorizedException(ProtocolErrorCode errorCode, String
errorMessage) {
+ super(errorCode, errorMessage);
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/client/protocol/ClientProtocolProcessor.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/client/protocol/ClientProtocolProcessor.java
index e49f16f..2631ed5 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/client/protocol/ClientProtocolProcessor.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/client/protocol/ClientProtocolProcessor.java
@@ -42,4 +42,9 @@ public interface ClientProtocolProcessor extends
AutoCloseable {
*/
@Override
void close();
+
+ /**
+ * Indicates that the associated connection should be closed
+ */
+ boolean socketProcessingIsFinished();
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
index 8063bf0..736c7ad 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/GenericProtocolServerConnection.java
@@ -68,6 +68,10 @@ public class GenericProtocolServerConnection extends
ServerConnection {
OutputStream outputStream = socket.getOutputStream();
protocolProcessor.processMessage(inputStream, outputStream);
+
+ if (protocolProcessor.socketProcessingIsFinished()) {
+ this.setFlagProcessMessagesAsFalse();
+ }
} catch (EOFException e) {
this.setFlagProcessMessagesAsFalse();
setClientDisconnectedException(e);
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufCachePipeline.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufCachePipeline.java
index 647e13e..4b88ec4 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufCachePipeline.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufCachePipeline.java
@@ -57,4 +57,9 @@ public final class ProtobufCachePipeline implements
ClientProtocolProcessor {
public void close() {
this.statistics.clientDisconnected();
}
+
+ @Override
+ public boolean socketProcessingIsFinished() {
+ return
messageExecutionContext.getConnectionStateProcessor().socketProcessingIsFinished();
+ }
}
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufLocatorPipeline.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufLocatorPipeline.java
index 3129d59..d67897f 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufLocatorPipeline.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufLocatorPipeline.java
@@ -55,4 +55,10 @@ public final class ProtobufLocatorPipeline implements
ClientProtocolProcessor {
public void close() {
this.statistics.clientDisconnected();
}
+
+ @Override
+ public boolean socketProcessingIsFinished() {
+ // All locator connections are closed after one message, so this is not
used
+ return false;
+ }
}
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
index ef64027..9437c3a 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/ProtobufOpsProcessor.java
@@ -28,7 +28,9 @@ import org.apache.geode.internal.protocol.Result;
import
org.apache.geode.internal.protocol.protobuf.v1.registry.ProtobufOperationContextRegistry;
import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponseUtilities;
import org.apache.geode.internal.protocol.serialization.SerializationService;
+import
org.apache.geode.internal.protocol.state.ConnectionTerminatingStateProcessor;
import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
+import
org.apache.geode.internal.protocol.state.exception.OperationNotAuthorizedException;
/**
* This handles protobuf requests by determining the operation type of the
request and dispatching
@@ -59,8 +61,14 @@ public class ProtobufOpsProcessor {
messageExecutionContext.getConnectionStateProcessor()
.validateOperation(messageExecutionContext, operationContext);
result = processOperation(request, messageExecutionContext, requestType,
operationContext);
+ } catch (OperationNotAuthorizedException e) {
+ // Don't move to a terminating state for authorization state failures
+ logger.warn(e.getMessage());
+ result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(e));
} catch (ConnectionStateException e) {
logger.warn(e.getMessage());
+ messageExecutionContext
+ .setConnectionStateProcessor(new
ConnectionTerminatingStateProcessor());
result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(e));
}
@@ -69,7 +77,8 @@ public class ProtobufOpsProcessor {
}
private Result processOperation(ClientProtocol.Request request,
MessageExecutionContext context,
- ClientProtocol.Request.RequestAPICase requestType, OperationContext
operationContext) {
+ ClientProtocol.Request.RequestAPICase requestType, OperationContext
operationContext)
+ throws ConnectionStateException {
try {
return
operationContext.getOperationHandler().process(serializationService,
operationContext.getFromRequest().apply(request), context);
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandler.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandler.java
index 1521fc0..97338e6 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandler.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandler.java
@@ -29,6 +29,7 @@ import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponse
import org.apache.geode.internal.protocol.serialization.SerializationService;
import
org.apache.geode.internal.protocol.state.ConnectionHandshakingStateProcessor;
import org.apache.geode.internal.protocol.state.ConnectionStateProcessor;
+import
org.apache.geode.internal.protocol.state.ConnectionTerminatingStateProcessor;
import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
public class HandshakeRequestOperationHandler implements
@@ -39,20 +40,21 @@ public class HandshakeRequestOperationHandler implements
@Override
public Result<ConnectionAPI.HandshakeResponse, ClientProtocol.ErrorResponse>
process(
SerializationService serializationService,
ConnectionAPI.HandshakeRequest request,
- MessageExecutionContext messageExecutionContext) throws
InvalidExecutionContextException {
+ MessageExecutionContext messageExecutionContext)
+ throws InvalidExecutionContextException, ConnectionStateException {
ConnectionHandshakingStateProcessor stateProcessor;
- try {
- stateProcessor =
messageExecutionContext.getConnectionStateProcessor().allowHandshake();
- } catch (ConnectionStateException e) {
- return Failure.of(ProtobufResponseUtilities.makeErrorResponse(e));
- }
+ // If handshake not allowed by this state this will throw a
ConnectionStateException
+ stateProcessor =
messageExecutionContext.getConnectionStateProcessor().allowHandshake();
final boolean handshakeSucceeded =
validator.isValid(request.getMajorVersion(),
request.getMinorVersion());
if (handshakeSucceeded) {
ConnectionStateProcessor nextStateProcessor =
stateProcessor.handshakeSucceeded();
messageExecutionContext.setConnectionStateProcessor(nextStateProcessor);
+ } else {
+ messageExecutionContext
+ .setConnectionStateProcessor(new
ConnectionTerminatingStateProcessor());
}
return Success.of(ConnectionAPI.HandshakeResponse.newBuilder()
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
index 3decb49..727a693 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
@@ -20,19 +20,16 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.geode.internal.exception.InvalidExecutionContextException;
-import org.apache.geode.internal.protocol.Failure;
import org.apache.geode.internal.protocol.MessageExecutionContext;
-import org.apache.geode.internal.protocol.ProtocolErrorCode;
import org.apache.geode.internal.protocol.Result;
import org.apache.geode.internal.protocol.Success;
import org.apache.geode.internal.protocol.operations.OperationHandler;
-import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
-import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponseUtilities;
-import
org.apache.geode.internal.protocol.security.exception.IncompatibleAuthenticationMechanismsException;
import org.apache.geode.internal.protocol.serialization.SerializationService;
import
org.apache.geode.internal.protocol.state.ConnectionAuthenticatingStateProcessor;
+import org.apache.geode.internal.protocol.state.ConnectionStateProcessor;
+import
org.apache.geode.internal.protocol.state.ConnectionTerminatingStateProcessor;
import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
import org.apache.geode.security.AuthenticationFailedException;
@@ -43,41 +40,26 @@ public class AuthenticationRequestOperationHandler
implements
@Override
public Result<ConnectionAPI.AuthenticationResponse,
ClientProtocol.ErrorResponse> process(
SerializationService serializationService,
ConnectionAPI.AuthenticationRequest request,
- MessageExecutionContext messageExecutionContext) throws
InvalidExecutionContextException {
+ MessageExecutionContext messageExecutionContext)
+ throws InvalidExecutionContextException, ConnectionStateException {
ConnectionAuthenticatingStateProcessor stateProcessor;
- try {
- stateProcessor =
messageExecutionContext.getConnectionStateProcessor().allowAuthentication();
- } catch (ConnectionStateException e) {
- return Failure.of(ProtobufResponseUtilities.makeErrorResponse(e));
- }
+ // If authentication not allowed by this state this will throw a
ConnectionStateException
+ stateProcessor =
messageExecutionContext.getConnectionStateProcessor().allowAuthentication();
Properties properties = new Properties();
properties.putAll(request.getCredentialsMap());
try {
-
messageExecutionContext.setConnectionStateProcessor(stateProcessor.authenticate(properties));
+ ConnectionStateProcessor nextState =
stateProcessor.authenticate(properties);
+ messageExecutionContext.setConnectionStateProcessor(nextState);
return Success
.of(ConnectionAPI.AuthenticationResponse.newBuilder().setAuthenticated(true).build());
- } catch (IncompatibleAuthenticationMechanismsException e) {
- return Failure.of(ClientProtocol.ErrorResponse.newBuilder().setError(
- buildAndLogError(ProtocolErrorCode.UNSUPPORTED_AUTHENTICATION_MODE,
e.getMessage(), e))
- .build());
} catch (AuthenticationFailedException e) {
+ messageExecutionContext
+ .setConnectionStateProcessor(new
ConnectionTerminatingStateProcessor());
return Success
.of(ConnectionAPI.AuthenticationResponse.newBuilder().setAuthenticated(false).build());
}
}
-
- private BasicTypes.Error buildAndLogError(ProtocolErrorCode errorCode,
String message,
- Exception ex) {
- if (ex == null) {
- logger.warn(message);
- } else {
- logger.warn(message, ex);
- }
-
- return
BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(message)
- .build();
- }
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
index e10573a..c3b6c73 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
@@ -200,6 +200,7 @@ public class AuthenticationIntegrationTest {
errorResponse.getResponse().getResponseAPICase());
assertEquals(AUTHENTICATION_FAILED.codeValue,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
+ verifyConnectionClosed();
}
@Test
@@ -246,6 +247,7 @@ public class AuthenticationIntegrationTest {
ConnectionAPI.AuthenticationResponse authenticationResponse =
parseSimpleAuthenticationResponseFromInput();
assertFalse(authenticationResponse.getAuthenticated());
+ verifyConnectionClosed();
}
@Test
@@ -265,6 +267,8 @@ public class AuthenticationIntegrationTest {
ConnectionAPI.AuthenticationResponse authenticationResponse =
parseSimpleAuthenticationResponseFromInput();
assertFalse(authenticationResponse.getAuthenticated());
+
+ verifyConnectionClosed();
}
@Test
@@ -296,6 +300,7 @@ public class AuthenticationIntegrationTest {
errorResponse.getResponse().getResponseAPICase());
assertEquals(UNSUPPORTED_AUTHENTICATION_MODE.codeValue,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
+ verifyConnectionClosed();
}
@Test
@@ -316,6 +321,18 @@ public class AuthenticationIntegrationTest {
errorResponse.getResponse().getResponseAPICase());
assertEquals(UNSUPPORTED_AUTHENTICATION_MODE.codeValue,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
+ verifyConnectionClosed();
+ }
+
+ private void verifyConnectionClosed() {
+ Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> {
+ try {
+ assertEquals(-1, socket.getInputStream().read()); // EOF implies
disconnected.
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ });
}
private void createLegacyAuthCache(String authenticationProperty) {
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/HandshakeIntegrationTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/HandshakeIntegrationTest.java
index b52ed0c..de3038f 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/HandshakeIntegrationTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/HandshakeIntegrationTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.internal.protocol.protobuf.v1;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
@@ -40,6 +41,7 @@ import org.apache.geode.cache.server.CacheServer;
import org.apache.geode.distributed.ConfigurationProperties;
import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.cache.tier.CommunicationMode;
+import org.apache.geode.internal.protocol.ProtocolErrorCode;
import
org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
import org.apache.geode.test.junit.categories.IntegrationTest;
@@ -122,4 +124,64 @@ public class HandshakeIntegrationTest {
}
});
}
+
+ @Test
+ public void testInvalidMinorVersionBreaksConnectionAfterResponse() throws
Exception {
+
outputStream.write(CommunicationMode.ProtobufClientServerProtocol.getModeNumber());
+
outputStream.write(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE);
+
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder()
+ .setHandshakeRequest(ConnectionAPI.HandshakeRequest.newBuilder()
+
.setMajorVersion(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE)
+
.setMinorVersion(ConnectionAPI.MinorVersions.INVALID_MINOR_VERSION_VALUE)))
+ .build().writeDelimitedTo(outputStream);
+ ClientProtocol.Message handshakeResponse =
protobufProtocolSerializer.deserialize(inputStream);
+
assertFalse(handshakeResponse.getResponse().getHandshakeResponse().getHandshakePassed());
+
+ // Verify that connection is closed
+ Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> {
+ try {
+ assertEquals(-1, socket.getInputStream().read()); // EOF implies
disconnected.
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ @Test
+ public void testUnexpectedHandshakeFailsAndClosesConnection() throws
Exception {
+
outputStream.write(CommunicationMode.ProtobufClientServerProtocol.getModeNumber());
+
outputStream.write(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE);
+
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder()
+ .setHandshakeRequest(ConnectionAPI.HandshakeRequest.newBuilder()
+
.setMajorVersion(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE)
+
.setMinorVersion(ConnectionAPI.MinorVersions.CURRENT_MINOR_VERSION_VALUE)))
+ .build().writeDelimitedTo(outputStream);
+ ClientProtocol.Message handshakeResponse =
protobufProtocolSerializer.deserialize(inputStream);
+
assertTrue(handshakeResponse.getResponse().getHandshakeResponse().getHandshakePassed());
+
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder()
+ .setHandshakeRequest(ConnectionAPI.HandshakeRequest.newBuilder()
+
.setMajorVersion(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE)
+
.setMinorVersion(ConnectionAPI.MinorVersions.CURRENT_MINOR_VERSION_VALUE)))
+ .build().writeDelimitedTo(outputStream);
+ ClientProtocol.Message failingHandshake =
protobufProtocolSerializer.deserialize(inputStream);
+ assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
+
failingHandshake.getResponse().getErrorResponse().getError().getErrorCode());
+
+ // Verify that connection is closed
+ Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> {
+ try {
+ assertEquals(-1, socket.getInputStream().read()); // EOF implies
disconnected.
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionTimeoutJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionTimeoutJUnitTest.java
index b8e4417..b14ceb2 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionTimeoutJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/CacheConnectionTimeoutJUnitTest.java
@@ -17,6 +17,7 @@ package
org.apache.geode.internal.protocol.protobuf.v1.acceptance;
import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
import java.io.IOException;
import java.io.InputStream;
@@ -27,6 +28,7 @@ import java.util.concurrent.TimeUnit;
import org.awaitility.Awaitility;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -41,9 +43,12 @@ import org.apache.geode.cache.Scope;
import org.apache.geode.cache.server.CacheServer;
import org.apache.geode.distributed.ConfigurationProperties;
import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.tier.CommunicationMode;
import org.apache.geode.internal.cache.tier.sockets.ClientHealthMonitor;
import org.apache.geode.internal.net.SocketCreatorFactory;
+import org.apache.geode.internal.protocol.MessageExecutionContext;
+import org.apache.geode.internal.protocol.Result;
import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
import org.apache.geode.internal.protocol.protobuf.v1.MessageUtil;
@@ -83,7 +88,6 @@ public class CacheConnectionTimeoutJUnitTest {
cacheFactory.set(ConfigurationProperties.MCAST_PORT, "0");
cacheFactory.set(ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION,
"false");
cacheFactory.set(ConfigurationProperties.USE_CLUSTER_CONFIGURATION,
"false");
- cacheFactory.setSecurityManager(null);
cache = cacheFactory.create();
@@ -150,6 +154,15 @@ public class CacheConnectionTimeoutJUnitTest {
@Test
public void testResponsiveClientsStaysConnected() throws Exception {
ProtobufProtocolSerializer protobufProtocolSerializer = new
ProtobufProtocolSerializer();
+
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder()
+ .setHandshakeRequest(ConnectionAPI.HandshakeRequest.newBuilder()
+
.setMajorVersion(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE)
+
.setMinorVersion(ConnectionAPI.MinorVersions.CURRENT_MINOR_VERSION_VALUE)))
+ .build().writeDelimitedTo(outputStream);
+ protobufProtocolSerializer.deserialize(socket.getInputStream());
+
ClientProtocol.Message putMessage =
MessageUtil.makePutRequestMessage(serializationService, TEST_KEY,
TEST_VALUE, TEST_REGION);
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
index c758438..592e8df 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandlerJUnitTest.java
@@ -33,7 +33,6 @@ import org.junit.experimental.categories.Category;
import org.apache.geode.cache.CacheLoaderException;
import org.apache.geode.cache.Region;
-import org.apache.geode.internal.exception.InvalidExecutionContextException;
import org.apache.geode.internal.protocol.Result;
import org.apache.geode.internal.protocol.Success;
import org.apache.geode.internal.protocol.TestExecutionContext;
@@ -95,8 +94,7 @@ public class GetAllRequestOperationHandlerJUnitTest extends
OperationHandlerJUni
}
@Test
- public void processReturnsNoEntriesForNoKeysRequested() throws
UnsupportedEncodingTypeException,
- CodecNotRegisteredForTypeException, InvalidExecutionContextException {
+ public void processReturnsNoEntriesForNoKeysRequested() throws Exception {
Result result =
operationHandler.process(serializationServiceStub,
generateTestRequest(false, false),
TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
@@ -129,8 +127,7 @@ public class GetAllRequestOperationHandlerJUnitTest extends
OperationHandlerJUni
}
@Test
- public void multipleKeysWhereOneThrows() throws
UnsupportedEncodingTypeException,
- CodecNotRegisteredForTypeException, InvalidExecutionContextException {
+ public void multipleKeysWhereOneThrows() throws Exception {
Result result =
operationHandler.process(serializationServiceStub,
generateTestRequest(true, true),
TestExecutionContext.getNoAuthCacheExecutionContext(cacheStub));
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java
index a43c5fa..61f72d1 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAvailableServersOperationHandlerJUnitTest.java
@@ -38,6 +38,7 @@ import
org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI;
import
org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI.GetAvailableServersResponse;
import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufRequestUtilities;
+import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
import org.apache.geode.test.junit.categories.UnitTest;
@Category(UnitTest.class)
@@ -81,8 +82,7 @@ public class GetAvailableServersOperationHandlerJUnitTest
extends OperationHandl
}
@Test
- public void testWhenServersFromSnapshotAreNullReturnsEmtpy()
- throws InvalidExecutionContextException {
+ public void testWhenServersFromSnapshotAreNullReturnsEmtpy() throws
Exception {
when(locatorLoadSnapshot.getServers(any())).thenReturn(null);
LocatorAPI.GetAvailableServersRequest getAvailableServersRequest =
@@ -95,8 +95,7 @@ public class GetAvailableServersOperationHandlerJUnitTest
extends OperationHandl
}
private Result getOperationHandlerResult(
- LocatorAPI.GetAvailableServersRequest getAvailableServersRequest)
- throws InvalidExecutionContextException {
+ LocatorAPI.GetAvailableServersRequest getAvailableServersRequest) throws
Exception {
return operationHandler.process(serializationServiceStub,
getAvailableServersRequest,
TestExecutionContext.getLocatorExecutionContext(internalLocatorMock));
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionNamesRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionNamesRequestOperationHandlerJUnitTest.java
index 0deb3f6..4913e4b 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionNamesRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionNamesRequestOperationHandlerJUnitTest.java
@@ -61,7 +61,7 @@ public class GetRegionNamesRequestOperationHandlerJUnitTest
extends OperationHan
}
@Test
- public void processReturnsCacheRegions() throws
InvalidExecutionContextException {
+ public void processReturnsCacheRegions() throws Exception {
Result result = operationHandler.process(serializationServiceStub,
ProtobufRequestUtilities.createGetRegionNamesRequest(),
getNoAuthCacheExecutionContext(cacheStub));
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandlerJUnitTest.java
index 0641e5d..0baf9bb 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/HandshakeRequestOperationHandlerJUnitTest.java
@@ -4,6 +4,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import org.apache.shiro.subject.Subject;
@@ -13,6 +14,7 @@ import org.junit.experimental.categories.Category;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.exception.InvalidExecutionContextException;
+import org.apache.geode.internal.protocol.Failure;
import org.apache.geode.internal.protocol.MessageExecutionContext;
import org.apache.geode.internal.protocol.ProtocolErrorCode;
import org.apache.geode.internal.protocol.Result;
@@ -21,9 +23,11 @@ import
org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
import
org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
import
org.apache.geode.internal.protocol.protobuf.v1.state.ConnectionShiroAuthenticatingStateProcessor;
import
org.apache.geode.internal.protocol.protobuf.v1.state.ProtobufConnectionHandshakeStateProcessor;
+import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponseUtilities;
import org.apache.geode.internal.protocol.serialization.SerializationService;
import
org.apache.geode.internal.protocol.state.ConnectionShiroAuthorizingStateProcessor;
import
org.apache.geode.internal.protocol.state.NoSecurityConnectionStateProcessor;
+import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.test.junit.categories.UnitTest;
@@ -85,16 +89,20 @@ public class HandshakeRequestOperationHandlerJUnitTest {
new MessageExecutionContext(mock(InternalCache.class), null,
handshakeStateProcessor);
verifyHandshakeFails(handshakeRequest, messageExecutionContext);
+ }
- // Also validate the protobuf INVALID_MAJOR_VERSION_VALUE constant fails
- handshakeRequest =
+ @Test
+ public void testInvalidMajorVersionProtocolConstantFails() throws Exception {
+ ConnectionAPI.HandshakeRequest handshakeRequest =
generateHandshakeRequest(ConnectionAPI.MajorVersions.INVALID_MAJOR_VERSION_VALUE,
ConnectionAPI.MinorVersions.CURRENT_MINOR_VERSION_VALUE);
+ MessageExecutionContext messageExecutionContext =
+ new MessageExecutionContext(mock(InternalCache.class), null,
handshakeStateProcessor);
verifyHandshakeFails(handshakeRequest, messageExecutionContext);
}
private void verifyHandshakeFails(ConnectionAPI.HandshakeRequest
handshakeRequest,
- MessageExecutionContext messageExecutionContext) throws
InvalidExecutionContextException {
+ MessageExecutionContext messageExecutionContext) throws Exception {
Result<ConnectionAPI.HandshakeResponse, ClientProtocol.ErrorResponse>
result =
handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
ConnectionAPI.HandshakeResponse handshakeResponse = result.getMessage();
@@ -111,11 +119,16 @@ public class HandshakeRequestOperationHandlerJUnitTest {
new MessageExecutionContext(mock(InternalCache.class), null,
handshakeStateProcessor);
verifyHandshakeFails(handshakeRequest, messageExecutionContext);
+ }
- // Also validate the protobuf INVALID_MINOR_VERSION_VALUE constant fails
- handshakeRequest =
+ @Test
+ public void testInvalidMinorVersionProtocolConstantFails() throws Exception {
+ ConnectionAPI.HandshakeRequest handshakeRequest =
generateHandshakeRequest(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE,
ConnectionAPI.MinorVersions.INVALID_MINOR_VERSION_VALUE);
+ MessageExecutionContext messageExecutionContext =
+ new MessageExecutionContext(mock(InternalCache.class), null,
handshakeStateProcessor);
+
verifyHandshakeFails(handshakeRequest, messageExecutionContext);
}
@@ -127,11 +140,12 @@ public class HandshakeRequestOperationHandlerJUnitTest {
MessageExecutionContext messageExecutionContext = new
MessageExecutionContext(
mock(InternalCache.class), null, new
NoSecurityConnectionStateProcessor());
- Result<ConnectionAPI.HandshakeResponse, ClientProtocol.ErrorResponse>
result =
- handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
- ClientProtocol.ErrorResponse errorMessage = result.getErrorMessage();
- assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
- errorMessage.getError().getErrorCode());
+ try {
+ handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
+ fail("Handshake in non-handshake state should throw exception");
+ } catch (ConnectionStateException ex) {
+ assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION, ex.getErrorCode());
+ }
}
@Test
@@ -143,11 +157,12 @@ public class HandshakeRequestOperationHandlerJUnitTest {
new MessageExecutionContext(mock(InternalCache.class), null,
new
ConnectionShiroAuthenticatingStateProcessor(mock(SecurityService.class)));
- Result<ConnectionAPI.HandshakeResponse, ClientProtocol.ErrorResponse>
result =
- handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
- ClientProtocol.ErrorResponse errorMessage = result.getErrorMessage();
- assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
- errorMessage.getError().getErrorCode());
+ try {
+ handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
+ fail("Handshake in non-handshake state should throw exception");
+ } catch (ConnectionStateException ex) {
+ assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION, ex.getErrorCode());
+ }
}
@Test
@@ -160,11 +175,12 @@ public class HandshakeRequestOperationHandlerJUnitTest {
new
ConnectionShiroAuthorizingStateProcessor(mock(SecurityService.class),
mock(Subject.class)));
- Result<ConnectionAPI.HandshakeResponse, ClientProtocol.ErrorResponse>
result =
- handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
- ClientProtocol.ErrorResponse errorMessage = result.getErrorMessage();
- assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
- errorMessage.getError().getErrorCode());
+ try {
+ handshakeHandler.process(serializationService, handshakeRequest,
messageExecutionContext);
+ fail("Handshake in non-handshake state should throw exception");
+ } catch (ConnectionStateException ex) {
+ assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION, ex.getErrorCode());
+ }
}
private ConnectionAPI.HandshakeRequest generateHandshakeRequest(int
majorVersion,
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
